Fix removal of unused materialized strings
Change-Id: I6ef39bd330f1cee6091eba10ca4ffebe915fe910
diff --git a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
index c0aaa95..8fe1c7e 100644
--- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
+++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -2172,6 +2172,10 @@
assert false : "Unexpected invoke targeting `" + invokedMethod.toSourceString() + "`";
return false;
}
+
+ public boolean isAppendCharArrayMethod(DexMethod method) {
+ return method == appendCharArray || method == appendSubCharArray;
+ }
}
public class SupplierMembers extends LibraryMembers {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderAppendOptimizer.java b/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderAppendOptimizer.java
index 2606826..a2a90b4 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderAppendOptimizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderAppendOptimizer.java
@@ -284,7 +284,7 @@
} else if (oracle.isAppend(instruction)) {
AppendNode appendNode = createAppendNode(instruction.asInvokeVirtual());
appendNode.setConstantArgument(oracle.getConstantArgument(instruction));
- Value arg = invoke.getOperand(1).getAliasedValue();
+ Value arg = invoke.getFirstNonReceiverArgument().getAliasedValue();
if (oracle.hasStringBuilderType(arg)) {
insertImplicitToStringNode(
arg, instruction, appendNode, escapeState, nodeConsumer);
@@ -508,7 +508,7 @@
escaping.add(root);
}
if (next.isToStringNode() || next.isImplicitToStringNode()) {
- materializingInstructions.add(root);
+ materializingInstructions.add(next);
}
if (next.isInspectingNode()) {
inspectingCapacity.add(root);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderNodeMuncher.java b/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderNodeMuncher.java
index 01be10c..74b654d 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderNodeMuncher.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderNodeMuncher.java
@@ -167,7 +167,6 @@
ToStringNode toStringNode = currentNode.asToStringNode();
munchingState.actions.put(
toStringNode.getInstruction(), new ReplaceByConstantString(constantArgument));
- munchingState.materializingInstructions.get(originalRoot).remove(currentNode);
String oldValue =
munchingState.optimizedStrings.put(
toStringNode.getInstruction().outValue(), constantArgument);
@@ -180,6 +179,7 @@
munchingState.actions.put(
initOrAppend.getInstruction(), new AppendWithNewConstantString(constantArgument));
}
+ munchingState.materializingInstructions.get(originalRoot).remove(currentNode);
currentNode.removeNode();
return true;
}
@@ -261,7 +261,7 @@
&& currentNode.getSuccessors().isEmpty();
if (canRemoveIfNoInspectionOrMaterializing
&& canRemoveIfLastAndNoLoop
- && munchingState.oracle.canObserveStringBuilderCall(
+ && !munchingState.oracle.canObserveStringBuilderCall(
currentNode.asAppendNode().getInstruction())) {
munchingState.actions.put(
appendNode.getInstruction(), RemoveStringBuilderAction.getInstance());
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderOracle.java b/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderOracle.java
index d7d7f59..063f791 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderOracle.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderOracle.java
@@ -12,6 +12,7 @@
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InvokeDirect;
+import com.android.tools.r8.ir.code.InvokeMethodWithReceiver;
import com.android.tools.r8.ir.code.InvokeVirtual;
import com.android.tools.r8.ir.code.Value;
import java.util.List;
@@ -109,7 +110,8 @@
if (!instruction.isInvokeMethodWithReceiver()) {
return null;
}
- if (isAppend(instruction)) {
+ if (isAppend(instruction)
+ && !isAppendWithSubArray(instruction.asInvokeMethodWithReceiver())) {
return getConstantStringForAppend(instruction.asInvokeVirtual());
} else if (isInit(instruction)) {
return getConstantStringForInit(instruction.asInvokeDirect());
@@ -165,14 +167,16 @@
return false;
}
DexMethod invokedMethod = instruction.asInvokeMethod().getInvokedMethod();
- if (factory.stringBuilderMethods.isAppendSubArrayMethod(invokedMethod)
- || factory.stringBufferMethods.isAppendSubArrayMethod(invokedMethod)) {
- return false;
- }
return factory.stringBuilderMethods.isAppendMethod(invokedMethod)
|| factory.stringBufferMethods.isAppendMethod(invokedMethod);
}
+ public boolean isAppendWithSubArray(InvokeMethodWithReceiver instruction) {
+ DexMethod invokedMethod = instruction.asInvokeMethod().getInvokedMethod();
+ return factory.stringBuilderMethods.isAppendSubArrayMethod(invokedMethod)
+ || factory.stringBufferMethods.isAppendSubArrayMethod(invokedMethod);
+ }
+
@Override
public boolean canObserveStringBuilderCall(Instruction instruction) {
if (!instruction.isInvokeMethod()) {
@@ -185,7 +189,11 @@
|| factory.stringBuilderMethods.charSequenceConstructor == invokedMethod
|| factory.stringBufferMethods.charSequenceConstructor == invokedMethod) {
assert instruction.inValues().size() == 2;
- return instruction.inValues().get(1).getType().isStringType(factory);
+ return !instruction.inValues().get(1).getType().isStringType(factory);
+ }
+ if (factory.stringBuilderMethods.isAppendCharArrayMethod(invokedMethod)
+ || factory.stringBufferMethods.isAppendCharArrayMethod(invokedMethod)) {
+ return instruction.asInvokeVirtual().getFirstNonReceiverArgument().isMaybeNull();
}
return false;
}
