Revert "Limit StringBuilder tracking to local instances to avoid O(N^2) behavior"
This reverts commit fa95f01cd4b36076811652c9162813be25106c38.
Reason for revert VerifyError in app
Bug: b/531533396
Bug: b/370217723
Bug: b/529584330
Change-Id: If2b07a942c310e65753e75cdfed1d528515e3289
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 8866814..4ed744d 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
@@ -325,7 +325,7 @@
}
if (invoke.arguments().size() == 2) {
Value arg = invoke.getOperand(1);
- if (oracle.hasStringBuilderType(arg) && escapeState.isLiveStringBuilder(arg)) {
+ if (oracle.hasStringBuilderType(arg)) {
insertImplicitToStringNode(
arg, instruction, initNode, escapeState, nodeConsumer);
}
@@ -345,8 +345,7 @@
&& arg.isNeverNull()) {
appendNode.setNonConstantArgument(arg);
}
- if (oracle.hasStringBuilderType(arg.getAliasedValue())
- && escapeState.isLiveStringBuilder(arg.getAliasedValue())) {
+ if (oracle.hasStringBuilderType(arg.getAliasedValue())) {
insertImplicitToStringNode(
arg, instruction, appendNode, escapeState, nodeConsumer);
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderEscapeTransferFunction.java b/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderEscapeTransferFunction.java
index d9bb7db..9c0932b 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderEscapeTransferFunction.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderEscapeTransferFunction.java
@@ -40,11 +40,12 @@
.forEach(
phi -> {
if (oracle.hasStringBuilderType(phi)) {
- for (Value operand : phi.getOperands()) {
- if (isLiveStringBuilder(builder, operand)) {
- builder.addLiveStringBuilder(phi);
- builder.addAlias(phi, operand);
- }
+ builder.addLiveStringBuilder(phi);
+ }
+ for (Value operand : phi.getOperands()) {
+ if (isLiveStringBuilder(builder, operand)) {
+ builder.addLiveStringBuilder(phi);
+ builder.addAlias(phi, operand);
}
}
});
@@ -70,7 +71,13 @@
if (isStringBuilderInstruction) {
if (instruction.isInvokeMethod()) {
assert !instruction.inValues().isEmpty();
- assert builder.getLiveStringBuilders().contains(instruction.getFirstOperand());
+ Value firstOperand = instruction.getFirstOperand();
+ if (!builder.getLiveStringBuilders().contains(firstOperand)) {
+ // We can have constant NULL being the first operand, which we have not marked as
+ // a live string builder.
+ assert firstOperand.getAliasedValue().isConstZero();
+ builder.addLiveStringBuilder(firstOperand);
+ }
} else {
assert instruction.isNewInstance();
}
@@ -85,9 +92,7 @@
builder.addLiveStringBuilder(outValue);
builder.addAlias(outValue, instruction.getFirstOperand());
} else if (oracle.hasStringBuilderType(outValue)) {
- if (instruction.isNewInstance() || instruction.isArgument()) {
- builder.addLiveStringBuilder(outValue);
- }
+ builder.addLiveStringBuilder(outValue);
}
if (!isStringBuilderInstruction
&& isLiveStringBuilder(builder, instruction.outValue())
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 2e29695..32b4a5b 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
@@ -71,12 +71,13 @@
return isStringBuilderType(instruction.asNewInstance().getType());
} else if (instruction.isInvokeMethod()) {
DexMethod invokedMethod = instruction.asInvokeMethod().getInvokedMethod();
- boolean isStringBuilding =
- isStringBuildingMethod(factory.stringBuilderMethods, invokedMethod)
- || isStringBuildingMethod(factory.stringBufferMethods, invokedMethod)
- || invokedMethod == factory.objectMembers.toString
- || invokedMethod == factory.objectsMethods.toStringWithObject;
- return isStringBuilding && isLiveStringBuilder.test(instruction.getFirstOperand());
+ if (isStringBuildingMethod(factory.stringBuilderMethods, invokedMethod)
+ || isStringBuildingMethod(factory.stringBufferMethods, invokedMethod)) {
+ return true;
+ }
+ return (invokedMethod == factory.objectMembers.toString
+ || invokedMethod == factory.objectsMethods.toStringWithObject)
+ && isLiveStringBuilder.test(instruction.getFirstOperand());
}
return false;
}
diff --git a/src/test/java8/ir/com/android/tools/r8/ir/optimize/string/B370217723Test.java b/src/test/java8/ir/com/android/tools/r8/ir/optimize/string/B370217723Test.java
index 431ceb4..194b074 100644
--- a/src/test/java8/ir/com/android/tools/r8/ir/optimize/string/B370217723Test.java
+++ b/src/test/java8/ir/com/android/tools/r8/ir/optimize/string/B370217723Test.java
@@ -49,7 +49,8 @@
.addKeepMainRule(TestClass.class)
.setMinApi(parameters)
.run(parameters.getRuntime(), TestClass.class)
- .assertFailureWithErrorThatThrows(NullPointerException.class);
+ // TODO(b/370217723): Should throw NullPointerException.
+ .assertSuccess();
}
static class TestClass {
