Fix removal of used StringBuilder
Bug: 213369062
Change-Id: I4bf0fcd6bab119a61531f91d8ad4833669cd9b4b
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderOptimizer.java b/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderOptimizer.java
index 33f4ba5..dca8e31 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderOptimizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderOptimizer.java
@@ -356,6 +356,10 @@
if (!candidateBuilders.contains(builder)) {
continue;
}
+ if (builder.hasPhiUsers()) {
+ candidateBuilders.remove(builder);
+ continue;
+ }
Map<Instruction, BuilderState> perInstrState = createBuilderState(builder);
perInstrState.put(instr, BuilderState.createRoot());
continue;
@@ -396,6 +400,10 @@
if (!candidateBuilders.contains(builder)) {
continue;
}
+ if (invoke.hasUsedOutValue()) {
+ candidateBuilders.remove(builder);
+ continue;
+ }
Value arg = invoke.inValues().get(1).getAliasedValue();
DexType argType = invoke.getInvokedMethod().proto.parameters.values[0];
String addition = extractConstantArgument(arg, argType);
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderOptimizerAnalysisSmaliTest.java b/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderOptimizerAnalysisSmaliTest.java
index 8552e48..cc582ea 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderOptimizerAnalysisSmaliTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderOptimizerAnalysisSmaliTest.java
@@ -158,29 +158,33 @@
public void testPhiWithDifferentNewInstance() {
buildAndCheckIR(
"phiWithDifferentNewInstance",
- checkOptimizerStates(appView, optimizer -> {
- assertEquals(2, optimizer.analysis.builderStates.size());
- for (Value builder : optimizer.analysis.builderStates.keySet()) {
- Map<Instruction, BuilderState> perBuilderState =
- optimizer.analysis.builderStates.get(builder);
- checkBuilderState(optimizer, perBuilderState, null, false);
- }
- assertEquals(0, optimizer.analysis.simplifiedBuilders.size());
- }));
+ checkOptimizerStates(
+ appView,
+ optimizer -> {
+ assertEquals(0, optimizer.analysis.builderStates.size());
+ for (Value builder : optimizer.analysis.builderStates.keySet()) {
+ Map<Instruction, BuilderState> perBuilderState =
+ optimizer.analysis.builderStates.get(builder);
+ checkBuilderState(optimizer, perBuilderState, null, false);
+ }
+ assertEquals(0, optimizer.analysis.simplifiedBuilders.size());
+ }));
}
@Test
public void testPhiAtInit() {
buildAndCheckIR(
"phiAtInit",
- checkOptimizerStates(appView, optimizer -> {
- assertEquals(2, optimizer.analysis.builderStates.size());
- for (Value builder : optimizer.analysis.builderStates.keySet()) {
- Map<Instruction, BuilderState> perBuilderState =
- optimizer.analysis.builderStates.get(builder);
- checkBuilderState(optimizer, perBuilderState, null, false);
- }
- assertEquals(0, optimizer.analysis.simplifiedBuilders.size());
- }));
+ checkOptimizerStates(
+ appView,
+ optimizer -> {
+ assertEquals(0, optimizer.analysis.builderStates.size());
+ for (Value builder : optimizer.analysis.builderStates.keySet()) {
+ Map<Instruction, BuilderState> perBuilderState =
+ optimizer.analysis.builderStates.get(builder);
+ checkBuilderState(optimizer, perBuilderState, null, false);
+ }
+ assertEquals(0, optimizer.analysis.simplifiedBuilders.size());
+ }));
}
}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderOptimizerAnalysisTest.java b/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderOptimizerAnalysisTest.java
index e18c8ea..72b5887 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderOptimizerAnalysisTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderOptimizerAnalysisTest.java
@@ -290,15 +290,17 @@
public void testPhiWithDifferentInits() {
buildAndCheckIR(
"phiWithDifferentInits",
- checkOptimizerStates(appView, optimizer -> {
- assertEquals(2, optimizer.analysis.builderStates.size());
- for (Value builder : optimizer.analysis.builderStates.keySet()) {
- Map<Instruction, BuilderState> perBuilderState =
- optimizer.analysis.builderStates.get(builder);
- checkBuilderState(optimizer, perBuilderState, null, false);
- }
- assertEquals(0, optimizer.analysis.simplifiedBuilders.size());
- }));
+ checkOptimizerStates(
+ appView,
+ optimizer -> {
+ assertEquals(0, optimizer.analysis.builderStates.size());
+ for (Value builder : optimizer.analysis.builderStates.keySet()) {
+ Map<Instruction, BuilderState> perBuilderState =
+ optimizer.analysis.builderStates.get(builder);
+ checkBuilderState(optimizer, perBuilderState, null, false);
+ }
+ assertEquals(0, optimizer.analysis.simplifiedBuilders.size());
+ }));
}
@Test
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderWithLoopTest.java b/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderWithLoopTest.java
index 05899f9..8cc98c4 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderWithLoopTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderWithLoopTest.java
@@ -7,7 +7,6 @@
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
-import com.android.tools.r8.utils.codeinspector.AssertUtils;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -27,16 +26,13 @@
@Test
public void test() throws Exception {
- AssertUtils.assertFailsCompilationIf(
- parameters.isDexRuntime(),
- () ->
- testForR8(parameters.getBackend())
- .addInnerClasses(getClass())
- .addKeepMainRule(Main.class)
- .addKeepClassAndMembersRules(Utils.class)
- .setMinApi(parameters.getApiLevel())
- .run(parameters.getRuntime(), Main.class)
- .assertSuccessWithOutputLines("1", "2"));
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(Main.class)
+ .addKeepClassAndMembersRules(Utils.class)
+ .setMinApi(parameters.getApiLevel())
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("1", "2");
}
static class Main {
