Handle if-instructions in string builder optimization
Change-Id: I56fb479bcb18956f3e0696dfb857264aba0bd03d
diff --git a/src/main/java/com/android/tools/r8/ir/code/If.java b/src/main/java/com/android/tools/r8/ir/code/If.java
index 706d470..62cb5a2 100644
--- a/src/main/java/com/android/tools/r8/ir/code/If.java
+++ b/src/main/java/com/android/tools/r8/ir/code/If.java
@@ -224,6 +224,12 @@
return targetFromCondition(1);
}
+ public BasicBlock targetFromNullObject() {
+ assert isZeroTest();
+ assert inValues.get(0).outType().isObject();
+ return targetFromCondition(0);
+ }
+
public BasicBlock targetFromCondition(int cond) {
assert Integer.signum(cond) == cond;
switch (type) {
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 4dc7926..3864fb8 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
@@ -23,7 +23,9 @@
import com.android.tools.r8.ir.code.ConstString;
import com.android.tools.r8.ir.code.DominatorTree;
import com.android.tools.r8.ir.code.DominatorTree.Assumption;
+import com.android.tools.r8.ir.code.Goto;
import com.android.tools.r8.ir.code.IRCode;
+import com.android.tools.r8.ir.code.If;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InstructionListIterator;
import com.android.tools.r8.ir.code.InvokeDirect;
@@ -35,6 +37,7 @@
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.code.ValueType;
import com.android.tools.r8.logging.Log;
+import com.android.tools.r8.utils.SetUtils;
import com.android.tools.r8.utils.StringUtils;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableSet;
@@ -731,13 +734,34 @@
if (deadBuilders.isEmpty() && simplifiedBuilders.isEmpty()) {
return;
}
- Set<Value> buildersToRemove = Sets.union(deadBuilders, simplifiedBuilders);
+ Set<Value> affectedValues = Sets.newIdentityHashSet();
+ Set<Value> buildersToRemove = SetUtils.newIdentityHashSet(deadBuilders, simplifiedBuilders);
+ Set<Value> buildersUsedInIf = Sets.newIdentityHashSet();
// All instructions that refer to dead/simplified builders are dead.
// Here, we remove toString() calls, append(...) calls, <init>, and new-instance in order.
InstructionListIterator it = code.instructionListIterator();
+ boolean shouldRemoveUnreachableBlocks = false;
while (it.hasNext()) {
Instruction instr = it.next();
- if (instr.isInvokeMethod()) {
+ if (instr.isIf()) {
+ If theIf = instr.asIf();
+ Value lhs = theIf.lhs().getAliasedValue();
+ if (theIf.isZeroTest()) {
+ if (buildersToRemove.contains(lhs)) {
+ theIf.targetFromNullObject().unlinkSinglePredecessorSiblingsAllowed();
+ it.replaceCurrentInstruction(new Goto());
+ shouldRemoveUnreachableBlocks = true;
+ }
+ } else {
+ Value rhs = theIf.rhs().getAliasedValue();
+ if (buildersToRemove.contains(lhs)) {
+ buildersUsedInIf.add(lhs);
+ }
+ if (buildersToRemove.contains(rhs)) {
+ buildersUsedInIf.add(rhs);
+ }
+ }
+ } else if (instr.isInvokeMethod()) {
InvokeMethod invoke = instr.asInvokeMethod();
DexMethod invokedMethod = invoke.getInvokedMethod();
if (optimizationConfiguration.isToStringMethod(invokedMethod)
@@ -746,6 +770,10 @@
}
}
}
+ if (shouldRemoveUnreachableBlocks) {
+ affectedValues.addAll(code.removeUnreachableBlocks());
+ }
+ buildersToRemove.removeAll(buildersUsedInIf);
// append(...) and <init> don't have out values, so removing them won't bother each other.
it = code.instructionListIterator();
while (it.hasNext()) {
@@ -787,6 +815,9 @@
it.removeOrReplaceByDebugLocalRead();
}
}
+ if (!affectedValues.isEmpty()) {
+ new TypeAnalysis(appView).narrowing(affectedValues);
+ }
assert code.isConsistentSSA();
}
}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderWithIfNullUserTest.java b/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderWithIfNullUserTest.java
new file mode 100644
index 0000000..2d4b414
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderWithIfNullUserTest.java
@@ -0,0 +1,49 @@
+// Copyright (c) 2021, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.ir.optimize.string;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class StringBuilderWithIfNullUserTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public StringBuilderWithIfNullUserTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(StringBuilderWithIfNullUserTest.class)
+ .addKeepMainRule(Main.class)
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("Hello world!");
+ }
+
+ static class Main {
+
+ public static void main(String[] args) {
+ StringBuilder builder = new StringBuilder();
+ builder.append("Hello world!");
+ if (builder != null) {
+ System.out.println(builder.toString());
+ }
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderWithIfUserTest.java b/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderWithIfUserTest.java
index 0b733bd..14ba197 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderWithIfUserTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderWithIfUserTest.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;
@@ -28,17 +27,13 @@
@Test
public void test() throws Exception {
- AssertUtils.assertFailsCompilationIf(
- parameters.isDexRuntime(),
- () -> {
- testForR8(parameters.getBackend())
- .addInnerClasses(StringBuilderWithIfUserTest.class)
- .addKeepMainRule(Main.class)
- .setMinApi(parameters.getApiLevel())
- .compile()
- .run(parameters.getRuntime(), Main.class)
- .assertSuccessWithOutputLines("Hello world!");
- });
+ testForR8(parameters.getBackend())
+ .addInnerClasses(StringBuilderWithIfUserTest.class)
+ .addKeepMainRule(Main.class)
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("Hello world!");
}
static class Main {
