Enable new stringbuilder optimizer
Bug: 190489514
Bug: 219455761
Bug: 113859361
Bug: 114002137
Bug: 222437581
Bug: 154899065
Change-Id: I5d87e7ebf64ad615cd1e57571180b6fd60efc155
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 5c8046f..fbd4b5a 100644
--- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
+++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -2127,6 +2127,10 @@
return appendPrimitiveMethods.contains(method);
}
+ public boolean isAppendSubArrayMethod(DexMethod method) {
+ return appendSubCharArray == method || appendSubCharSequence == method;
+ }
+
public boolean isAppendStringMethod(DexMethod method) {
return method == appendString;
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index 226cb11..2a97c16 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -80,6 +80,7 @@
import com.android.tools.r8.ir.optimize.membervaluepropagation.MemberValuePropagation;
import com.android.tools.r8.ir.optimize.membervaluepropagation.R8MemberValuePropagation;
import com.android.tools.r8.ir.optimize.outliner.Outliner;
+import com.android.tools.r8.ir.optimize.string.StringBuilderAppendOptimizer;
import com.android.tools.r8.ir.optimize.string.StringBuilderOptimizer;
import com.android.tools.r8.ir.optimize.string.StringOptimizer;
import com.android.tools.r8.logging.Log;
@@ -1315,15 +1316,14 @@
timing.begin("Rewrite move result");
codeRewriter.rewriteMoveResult(code);
timing.end();
- // TODO(b/114002137): for now, string concatenation depends on rewriteMoveResult.
+ // TODO(b/114002137): Also run for CF
if (options.enableStringConcatenationOptimization
&& !isDebugMode
&& options.isGeneratingDex()) {
timing.begin("Rewrite string concat");
- stringBuilderOptimizer.computeTrivialStringConcatenation(code);
+ StringBuilderAppendOptimizer.run(appView, code);
timing.end();
}
-
timing.begin("Split range invokes");
codeRewriter.splitRangeInvokeConstants(code);
timing.end();
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderAction.java b/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderAction.java
index 5ecb6bb..05a91e5 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderAction.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderAction.java
@@ -38,7 +38,11 @@
InstructionListIterator iterator,
Instruction instruction,
StringBuilderOracle oracle) {
- assert oracle.isModeledStringBuilderInstruction(instruction);
+ assert oracle.isModeledStringBuilderInstruction(
+ instruction,
+ value ->
+ value.getType().isClassType()
+ && oracle.isStringBuilderType(value.getType().asClassType().getClassType()));
if (oracle.isAppend(instruction) && instruction.outValue() != null) {
// Append will return the string builder instance. Before removing, ensure that
// all users of the output values uses the receiver.
@@ -70,7 +74,7 @@
InstructionListIterator iterator,
Instruction instruction,
StringBuilderOracle oracle) {
- assert oracle.isToString(instruction);
+ assert oracle.isToString(instruction, instruction.getFirstOperand());
iterator.replaceCurrentInstructionWithConstString(appView, code, replacement);
}
}
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 caf8432..702cf76 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
@@ -229,7 +229,8 @@
if (instruction.isAssume()) {
return;
}
- if (oracle.isModeledStringBuilderInstruction(instruction)) {
+ if (oracle.isModeledStringBuilderInstruction(
+ instruction, escapeState::isLiveStringBuilder)) {
createNodesForStringBuilderInstruction(instruction, escapeState, nodeConsumer);
} else {
for (Value newEscapedValue : escapeState.getNewlyEscaped()) {
@@ -291,7 +292,7 @@
escapeState,
actual -> nodeConsumer.accept(actual, appendNode),
escaped -> nodeConsumer.accept(escaped, createMutateNode()));
- } else if (oracle.isToString(instruction)) {
+ } else if (oracle.isToString(instruction, receiver)) {
visitStringBuilderValues(
receiver,
escapeState,
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 ff4a62a..2ea5a08 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
@@ -53,16 +53,30 @@
public TransferFunctionResult<StringBuilderEscapeState> apply(
Instruction instruction, StringBuilderEscapeState state) {
StringBuilderEscapeState.Builder builder = state.builder();
- boolean isStringBuilderInstruction = oracle.isModeledStringBuilderInstruction(instruction);
+ boolean isStringBuilderInstruction =
+ oracle.isModeledStringBuilderInstruction(instruction, state::isLiveStringBuilder);
if (!isStringBuilderInstruction && isEscapingInstructionForInValues(instruction)) {
for (Value inValue : instruction.inValues()) {
if (isLiveStringBuilder(builder, inValue)) {
- // TODO(b/232377424): Account for calls to Object (such as Object.toString()).
builder.addEscaping(inValue);
}
}
}
+ if (isStringBuilderInstruction) {
+ if (instruction.isInvokeMethodWithReceiver()) {
+ 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.isConstZero();
+ builder.addLiveStringBuilder(firstOperand);
+ }
+ } else {
+ assert instruction.isNewInstance();
+ }
+ }
assert !isStringBuilderInstruction
+ || instruction.isNewInstance()
|| builder.getLiveStringBuilders().contains(instruction.getFirstOperand());
Value outValue = instruction.outValue();
if (outValue != null) {
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 000ca52..01be10c 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
@@ -246,16 +246,12 @@
boolean removeNode;
boolean isEscaping = munchingState.escaping.contains(root);
while (currentNode != null) {
- removeNode =
- currentNode.isSplitReferenceNode()
- && (currentNode.getSuccessors().isEmpty() || currentNode.hasSinglePredecessor());
// Remove appends if the string builder do not escape, is not inspected or materialized
- if (removeNode) {
- // TODO(b/190489514): See if this larger pattern is necessary.
- assert false : "Check when this happens";
- }
// and if it is not part of a loop.
- if (currentNode.isAppendNode() && !isEscaping) {
+ removeNode = false;
+ if (currentNode.isSplitReferenceNode()) {
+ removeNode = currentNode.getSuccessors().isEmpty() || currentNode.hasSinglePredecessor();
+ } else if (currentNode.isAppendNode() && !isEscaping) {
AppendNode appendNode = currentNode.asAppendNode();
boolean canRemoveIfNoInspectionOrMaterializing =
!munchingState.inspectingCapacity.contains(root)
@@ -271,8 +267,7 @@
appendNode.getInstruction(), RemoveStringBuilderAction.getInstance());
removeNode = true;
}
- }
- if (currentNode.isInitNode()
+ } else if (currentNode.isInitNode()
&& currentNode.asInitNode().hasConstantArgument()
&& currentNode.hasSinglePredecessor()
&& currentNode.getSinglePredecessor().isNewInstanceNode()
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 b6c0b4f..dd49ae4 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,10 +12,10 @@
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;
+import java.util.function.Predicate;
/**
* The {@link StringBuilderOracle} can answer if an instruction is of particular interest to the
@@ -23,13 +23,14 @@
*/
interface StringBuilderOracle {
- boolean isModeledStringBuilderInstruction(Instruction instruction);
+ boolean isModeledStringBuilderInstruction(
+ Instruction instruction, Predicate<Value> isLiveStringBuilder);
boolean hasStringBuilderType(Value value);
boolean isStringBuilderType(DexType type);
- boolean isToString(Instruction instruction);
+ boolean isToString(Instruction instruction, Value value);
String getConstantArgument(Instruction instruction);
@@ -50,13 +51,18 @@
}
@Override
- public boolean isModeledStringBuilderInstruction(Instruction instruction) {
+ public boolean isModeledStringBuilderInstruction(
+ Instruction instruction, Predicate<Value> isLiveStringBuilder) {
if (instruction.isNewInstance()) {
return isStringBuilderType(instruction.asNewInstance().getType());
} else if (instruction.isInvokeMethod()) {
DexMethod invokedMethod = instruction.asInvokeMethod().getInvokedMethod();
- return isStringBuildingMethod(factory.stringBuilderMethods, invokedMethod)
- || isStringBuildingMethod(factory.stringBufferMethods, invokedMethod);
+ if (isStringBuildingMethod(factory.stringBuilderMethods, invokedMethod)
+ || isStringBuildingMethod(factory.stringBufferMethods, invokedMethod)) {
+ return true;
+ }
+ return invokedMethod == factory.objectMembers.toString
+ && isLiveStringBuilder.test(instruction.getFirstOperand());
}
return false;
}
@@ -80,14 +86,18 @@
}
@Override
- public boolean isToString(Instruction instruction) {
- if (!instruction.isInvokeMethodWithReceiver()) {
+ public boolean isToString(Instruction instruction, Value value) {
+ if (!instruction.isInvokeVirtual()) {
return false;
}
- InvokeMethodWithReceiver invoke = instruction.asInvokeMethodWithReceiver();
+ InvokeVirtual invoke = instruction.asInvokeVirtual();
+ if (invoke.getReceiver() != value) {
+ return false;
+ }
DexMethod invokedMethod = invoke.getInvokedMethod();
return factory.stringBuilderMethods.toString == invokedMethod
- || factory.stringBufferMethods.toString == invokedMethod;
+ || factory.stringBufferMethods.toString == invokedMethod
+ || factory.objectMembers.toString == invokedMethod;
}
@Override
@@ -151,13 +161,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);
}
@Override
public boolean canObserveStringBuilderCall(Instruction instruction) {
- assert isModeledStringBuilderInstruction(instruction);
if (!instruction.isInvokeMethod()) {
assert false : "Expecting a call to string builder";
return true;
diff --git a/src/test/examples/shaking1/print-mapping-dex.ref b/src/test/examples/shaking1/print-mapping-dex.ref
index 5a9504a..3635115 100644
--- a/src/test/examples/shaking1/print-mapping-dex.ref
+++ b/src/test/examples/shaking1/print-mapping-dex.ref
@@ -4,5 +4,5 @@
3:5:void <init>(java.lang.String):13:13 -> <init>
0:6:void main(java.lang.String[]):8:8 -> main
7:21:void main(java.lang.String[]):9:9 -> main
- 0:19:java.lang.String method():17:17 -> a
+ 0:16:java.lang.String method():17:17 -> a
java.lang.String name -> a
diff --git a/src/test/java/com/android/tools/r8/R8RunExamplesAndroidOTest.java b/src/test/java/com/android/tools/r8/R8RunExamplesAndroidOTest.java
index a7759e3..7b5d60b 100644
--- a/src/test/java/com/android/tools/r8/R8RunExamplesAndroidOTest.java
+++ b/src/test/java/com/android/tools/r8/R8RunExamplesAndroidOTest.java
@@ -131,7 +131,7 @@
.withMinApiLevel(ToolHelper.getMinApiLevelForDexVmNoHigherThan(AndroidApiLevel.K))
.withBuilderTransformation(
b -> b.addProguardConfiguration(PROGUARD_OPTIONS, Origin.unknown()))
- .withDexCheck(inspector -> checkLambdaCount(inspector, 4, "lambdadesugaring"))
+ .withDexCheck(inspector -> checkLambdaCount(inspector, 3, "lambdadesugaring"))
.run();
}
@@ -170,7 +170,7 @@
.withMinApiLevel(AndroidApiLevel.N)
.withBuilderTransformation(
b -> b.addProguardConfiguration(PROGUARD_OPTIONS, Origin.unknown()))
- .withDexCheck(inspector -> checkLambdaCount(inspector, 4, "lambdadesugaring"))
+ .withDexCheck(inspector -> checkLambdaCount(inspector, 3, "lambdadesugaring"))
.run();
}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/canonicalization/StringCanonicalizationTest.java b/src/test/java/com/android/tools/r8/ir/optimize/canonicalization/StringCanonicalizationTest.java
index a8b1557..3aa8509 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/canonicalization/StringCanonicalizationTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/canonicalization/StringCanonicalizationTest.java
@@ -6,7 +6,6 @@
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assume.assumeTrue;
import com.android.tools.r8.D8TestCompileResult;
import com.android.tools.r8.NeverInline;
@@ -159,7 +158,7 @@
@Parameterized.Parameters(name = "{0}")
public static TestParametersCollection data() {
// CF should not canonicalize strings or lower them. See (r8g/30163) and (r8g/30320).
- return getTestParameters().withDexRuntimes().build();
+ return getTestParameters().withDexRuntimes().withAllApiLevels().build();
}
private final TestParameters parameters;
@@ -202,24 +201,25 @@
}
@Test
- public void testD8() throws Exception {
- assumeTrue("Only run D8 for Dex backend", parameters.isDexRuntime());
+ public void testR8Debug() throws Exception {
+ D8TestCompileResult result =
+ testForD8()
+ .debug()
+ .addProgramClassesAndInnerClasses(MAIN)
+ .setMinApi(parameters.getApiLevel())
+ .compile();
+ test(result, 2, 1, 1, 1, 1);
+ }
+ @Test
+ public void testD8Release() throws Exception {
D8TestCompileResult result =
testForD8()
.release()
.addProgramClassesAndInnerClasses(MAIN)
- .setMinApi(parameters.getRuntime())
+ .setMinApi(parameters.getApiLevel())
.compile();
- test(result, 1, 1, 1, 1, 1);
-
- result =
- testForD8()
- .debug()
- .addProgramClassesAndInnerClasses(MAIN)
- .setMinApi(parameters.getRuntime())
- .compile();
- test(result, 2, 1, 1, 1, 1);
+ test(result, 1, 1, 1, 1, 0);
}
@Test
@@ -230,9 +230,8 @@
.enableProguardTestOptions()
.enableInliningAnnotations()
.addKeepMainRule(MessageLoader.class)
- .setMinApi(parameters.getRuntime())
+ .setMinApi(parameters.getApiLevel())
.compile();
- test(result, 1, 1, 1, 1, 1);
+ test(result, 1, 1, 1, 1, 0);
}
-
}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/outliner/b111893131/B111893131.java b/src/test/java/com/android/tools/r8/ir/optimize/outliner/b111893131/B111893131.java
index 7ba5906..7873c68 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/outliner/b111893131/B111893131.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/outliner/b111893131/B111893131.java
@@ -81,6 +81,7 @@
// To trigger outliner, set # of expected outline candidate as threshold.
options.outline.threshold = 2;
options.inlinerOptions().enableInlining = false;
+ options.enableStringConcatenationOptimization = false;
});
ProcessResult result = runOnArtRaw(app, TestClass.class);
assertEquals(result.toString(), 0, result.exitCode);
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
deleted file mode 100644
index cc582ea..0000000
--- a/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderOptimizerAnalysisSmaliTest.java
+++ /dev/null
@@ -1,190 +0,0 @@
-// Copyright (c) 2019, 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 static com.android.tools.r8.ir.optimize.string.StringBuilderOptimizerAnalysisTest.checkBuilderState;
-import static com.android.tools.r8.ir.optimize.string.StringBuilderOptimizerAnalysisTest.checkOptimizerStates;
-import static org.junit.Assert.assertEquals;
-
-import com.android.tools.r8.TestParameters;
-import com.android.tools.r8.TestParametersCollection;
-import com.android.tools.r8.ToolHelper;
-import com.android.tools.r8.ir.analysis.AnalysisTestBase;
-import com.android.tools.r8.ir.code.Instruction;
-import com.android.tools.r8.ir.code.Value;
-import com.android.tools.r8.ir.optimize.string.StringBuilderOptimizer.BuilderState;
-import com.android.tools.r8.origin.Origin;
-import com.android.tools.r8.smali.SmaliBuilder;
-import com.android.tools.r8.utils.AndroidApp;
-import com.android.tools.r8.utils.StringUtils;
-import com.google.common.collect.ImmutableList;
-import java.util.Map;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-
-@RunWith(Parameterized.class)
-public class StringBuilderOptimizerAnalysisSmaliTest extends AnalysisTestBase {
- @Parameterized.Parameters(name = "{0}")
- public static TestParametersCollection data() {
- return getTestParameters().withAllRuntimesAndApiLevels().build();
- }
-
- public StringBuilderOptimizerAnalysisSmaliTest(TestParameters parameters) throws Exception {
- super(parameters, buildApp(), "TestClass");
- }
-
- private static AndroidApp buildApp() throws Exception {
- SmaliBuilder smaliBuilder = new SmaliBuilder("TestClass");
-
- {
- // IR of StringConcatenationTestClass#phiAtInit at DVM older than or equal to 5.1.1
- String code =
- StringUtils.lines(
- "invoke-static {}, Ljava/lang/System;->currentTimeMillis()J",
- "move-result-wide v0",
- "const-wide/16 v2, 0",
- "cmp-long v4, v0, v2",
- // new-instance is hoisted, but <init> calls are still at each branch
- "new-instance v0, Ljava/lang/StringBuilder;",
- "if-lez v4, :cond",
- "const-string v1, \"Hello\"",
- "invoke-direct {v0, v1}, Ljava/lang/StringBuilder;-><init>(Ljava/lang/String;)V",
- "goto :merge",
- ":cond",
- "const-string v1, \"Hi\"",
- "invoke-direct {v0, v1}, Ljava/lang/StringBuilder;-><init>(Ljava/lang/String;)V",
- "goto :merge",
- ":merge",
- "const-string v1, \", R8\"",
- "invoke-virtual {v0, v1}, Ljava/lang/StringBuilder;->append"
- + "(Ljava/lang/String;)Ljava/lang/StringBuilder;",
- "sget-object v2, Ljava/lang/System;->out:Ljava/io/PrintStream;",
- "invoke-virtual {v0}, Ljava/lang/StringBuilder;->toString()Ljava/lang/String;",
- "move-result-object v1",
- "invoke-virtual {v2, v1}, Ljava/io/PrintStream;->println(Ljava/lang/String;)V",
- "return-void");
- smaliBuilder.addStaticMethod(
- "void", "phiAtInit_5_1_1", ImmutableList.of(), 5, code);
- }
-
- {
- // Compiled StringConcatenationTestClass#phiAtInit and modified two new-instance instructions
- // are flown to the same <init> call.
- String code =
- StringUtils.lines(
- "invoke-static {}, Ljava/lang/System;->currentTimeMillis()J",
- "move-result-wide v0",
- "const-wide/16 v2, 0",
- "cmp-long v4, v0, v2",
- "if-lez v4, :cond",
- "new-instance v0, Ljava/lang/StringBuilder;",
- "const-string v1, \"Hello\"",
- "goto :merge",
- ":cond",
- "new-instance v0, Ljava/lang/StringBuilder;",
- "const-string v1, \"Hi\"",
- "goto :merge",
- ":merge",
- // Two separate new-instance instructions are flown to the same <init>.
- "invoke-direct {v0, v1}, Ljava/lang/StringBuilder;-><init>(Ljava/lang/String;)V",
- "const-string v1, \", R8\"",
- "invoke-virtual {v0, v1}, Ljava/lang/StringBuilder;->append"
- + "(Ljava/lang/String;)Ljava/lang/StringBuilder;",
- "sget-object v2, Ljava/lang/System;->out:Ljava/io/PrintStream;",
- "invoke-virtual {v0}, Ljava/lang/StringBuilder;->toString()Ljava/lang/String;",
- "move-result-object v1",
- "invoke-virtual {v2, v1}, Ljava/io/PrintStream;->println(Ljava/lang/String;)V",
- "return-void");
- smaliBuilder.addStaticMethod(
- "void", "phiWithDifferentNewInstance", ImmutableList.of(), 5, code);
- }
-
- {
- // IR of StringConcatenationTestClass#phiAtInit at DVM/ART newer than 5.1.1
- String code =
- StringUtils.lines(
- "invoke-static {}, Ljava/lang/System;->currentTimeMillis()J",
- "move-result-wide v0",
- "const-wide/16 v2, 0",
- "cmp-long v4, v0, v2",
- // new-instance and <init> are in each branch.
- "if-lez v4, :cond",
- "new-instance v0, Ljava/lang/StringBuilder;",
- "const-string v1, \"Hello\"",
- "invoke-direct {v0, v1}, Ljava/lang/StringBuilder;-><init>(Ljava/lang/String;)V",
- "goto :merge",
- ":cond",
- "new-instance v0, Ljava/lang/StringBuilder;",
- "const-string v1, \"Hi\"",
- "invoke-direct {v0, v1}, Ljava/lang/StringBuilder;-><init>(Ljava/lang/String;)V",
- "goto :merge",
- ":merge",
- "const-string v1, \", R8\"",
- "invoke-virtual {v0, v1}, Ljava/lang/StringBuilder;->append"
- + "(Ljava/lang/String;)Ljava/lang/StringBuilder;",
- "sget-object v2, Ljava/lang/System;->out:Ljava/io/PrintStream;",
- "invoke-virtual {v0}, Ljava/lang/StringBuilder;->toString()Ljava/lang/String;",
- "move-result-object v1",
- "invoke-virtual {v2, v1}, Ljava/io/PrintStream;->println(Ljava/lang/String;)V",
- "return-void");
- smaliBuilder.addStaticMethod(
- "void", "phiAtInit", ImmutableList.of(), 5, code);
- }
-
- return AndroidApp.builder()
- .addDexProgramData(smaliBuilder.compile(), Origin.unknown())
- .addLibraryFile(ToolHelper.getMostRecentAndroidJar())
- .build();
- }
-
- @Test
- public void testPhiAtInit_5_1_1() {
- buildAndCheckIR(
- "phiAtInit_5_1_1",
- checkOptimizerStates(appView, optimizer -> {
- assertEquals(1, optimizer.analysis.builderStates.size());
- for (Value builder : optimizer.analysis.builderStates.keySet()) {
- Map<Instruction, BuilderState> perBuilderState =
- optimizer.analysis.builderStates.get(builder);
- checkBuilderState(optimizer, perBuilderState, null, true);
- }
- assertEquals(0, optimizer.analysis.simplifiedBuilders.size());
- }));
- }
-
- @Test
- public void testPhiWithDifferentNewInstance() {
- buildAndCheckIR(
- "phiWithDifferentNewInstance",
- 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(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
deleted file mode 100644
index 2a81ef0..0000000
--- a/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderOptimizerAnalysisTest.java
+++ /dev/null
@@ -1,401 +0,0 @@
-// Copyright (c) 2019, 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 static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-
-import com.android.tools.r8.TestParameters;
-import com.android.tools.r8.TestParametersCollection;
-import com.android.tools.r8.graph.AppView;
-import com.android.tools.r8.ir.analysis.AnalysisTestBase;
-import com.android.tools.r8.ir.code.IRCode;
-import com.android.tools.r8.ir.code.Instruction;
-import com.android.tools.r8.ir.code.InvokeMethod;
-import com.android.tools.r8.ir.code.Value;
-import com.android.tools.r8.ir.optimize.string.StringBuilderOptimizer.BuilderState;
-import com.android.tools.r8.utils.AndroidApiLevel;
-import com.android.tools.r8.utils.InternalOptions;
-import java.util.HashSet;
-import java.util.Map;
-import java.util.Set;
-import java.util.function.Consumer;
-import org.junit.Ignore;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-
-@RunWith(Parameterized.class)
-public class StringBuilderOptimizerAnalysisTest extends AnalysisTestBase {
-
- @Parameterized.Parameters(name = "{0}")
- public static TestParametersCollection data() {
- return getTestParameters().withDexRuntimes().withAllApiLevels().build();
- }
-
- public StringBuilderOptimizerAnalysisTest(TestParameters parameters) throws Exception {
- super(
- parameters,
- StringConcatenationTestClass.class.getTypeName(),
- StringConcatenationTestClass.class);
- }
-
- @Override
- public void configure(InternalOptions options) {}
-
- @Test
- public void testUnusedBuilder() {
- buildAndCheckIR(
- "unusedBuilder",
- code -> assertTrue(code.streamInstructions().allMatch(Instruction::isReturn)));
- }
-
- @Test
- public void testTrivialSequence() {
- buildAndCheckIR(
- "trivialSequence",
- checkOptimizerStates(appView, optimizer -> {
- assertEquals(1, optimizer.analysis.builderStates.size());
- for (Value builder : optimizer.analysis.builderStates.keySet()) {
- Map<Instruction, BuilderState> perBuilderState =
- optimizer.analysis.builderStates.get(builder);
- checkBuilderState(optimizer, perBuilderState, "xyz", true);
- }
- assertEquals(1, optimizer.analysis.simplifiedBuilders.size());
- }));
- }
-
- @Test
- public void testBuilderWithInitialValue() {
- buildAndCheckIR(
- "builderWithInitialValue",
- checkOptimizerStates(appView, optimizer -> {
- assertEquals(1, optimizer.analysis.builderStates.size());
- for (Value builder : optimizer.analysis.builderStates.keySet()) {
- Map<Instruction, BuilderState> perBuilderState =
- optimizer.analysis.builderStates.get(builder);
- checkBuilderState(optimizer, perBuilderState, "Hello,R8", true);
- }
- assertEquals(1, optimizer.analysis.simplifiedBuilders.size());
- }));
- }
-
- @Test
- public void testBuilderWithCapacity() {
- buildAndCheckIR(
- "builderWithCapacity",
- checkOptimizerStates(appView, optimizer -> {
- assertEquals(1, optimizer.analysis.builderStates.size());
- for (Value builder : optimizer.analysis.builderStates.keySet()) {
- Map<Instruction, BuilderState> perBuilderState =
- optimizer.analysis.builderStates.get(builder);
- checkBuilderState(optimizer, perBuilderState, "42", true);
- }
- assertEquals(1, optimizer.analysis.simplifiedBuilders.size());
- }));
- }
-
- @Test
- public void testNonStringArgs() {
- buildAndCheckIR(
- "nonStringArgs",
- checkOptimizerStates(appView, optimizer -> {
- assertEquals(1, optimizer.analysis.builderStates.size());
- for (Value builder : optimizer.analysis.builderStates.keySet()) {
- Map<Instruction, BuilderState> perBuilderState =
- optimizer.analysis.builderStates.get(builder);
- checkBuilderState(optimizer, perBuilderState, "42", true);
- }
- assertEquals(1, optimizer.analysis.simplifiedBuilders.size());
- }));
- }
-
- @Test
- public void testTypeConversion() {
- buildAndCheckIR(
- "typeConversion",
- checkOptimizerStates(appView, optimizer -> {
- assertEquals(1, optimizer.analysis.builderStates.size());
- for (Value builder : optimizer.analysis.builderStates.keySet()) {
- Map<Instruction, BuilderState> perBuilderState =
- optimizer.analysis.builderStates.get(builder);
- checkBuilderState(optimizer, perBuilderState, "0.14 0 false null", true);
- }
- assertEquals(1, optimizer.analysis.simplifiedBuilders.size());
- }));
- }
-
- @Test
- public void testTypeConversion_withPhis() {
- buildAndCheckIR(
- "typeConversion_withPhis",
- checkOptimizerStates(appView, optimizer -> {
- assertEquals(1, optimizer.analysis.builderStates.size());
- for (Value builder : optimizer.analysis.builderStates.keySet()) {
- Map<Instruction, BuilderState> perBuilderState =
- optimizer.analysis.builderStates.get(builder);
- checkBuilderState(optimizer, perBuilderState, null, true);
- }
- assertEquals(0, optimizer.analysis.simplifiedBuilders.size());
- }));
- }
-
- @Ignore("TODO(b/113859361): passed to another builder should be an eligible case.")
- @Test
- public void testNestedBuilders_appendBuilderItself() {
- buildAndCheckIR(
- "nestedBuilders_appendBuilderItself",
- checkOptimizerStates(appView, optimizer -> {
- assertEquals(1, optimizer.analysis.builderStates.size());
- for (Value builder : optimizer.analysis.builderStates.keySet()) {
- Map<Instruction, BuilderState> perBuilderState =
- optimizer.analysis.builderStates.get(builder);
- checkBuilderState(optimizer, perBuilderState, "Hello,R8", true);
- }
- assertEquals(1, optimizer.analysis.simplifiedBuilders.size());
- // TODO(b/113859361): check # of merged builders.
- // assertEquals(2, optimizer.analysis.mergedBuilders.size());
- }));
- }
-
- @Ignore("TODO(b/113859361): merge builder.")
- @Test
- public void testNestedBuilders_appendBuilderResult() {
- buildAndCheckIR(
- "nestedBuilders_appendBuilderResult",
- checkOptimizerStates(appView, optimizer -> {
- assertEquals(1, optimizer.analysis.builderStates.size());
- for (Value builder : optimizer.analysis.builderStates.keySet()) {
- Map<Instruction, BuilderState> perBuilderState =
- optimizer.analysis.builderStates.get(builder);
- checkBuilderState(optimizer, perBuilderState, "Hello,R8", true);
- }
- assertEquals(1, optimizer.analysis.simplifiedBuilders.size());
- // TODO(b/113859361): check # of merged builders.
- // assertEquals(2, optimizer.analysis.mergedBuilders.size());
- }));
- }
-
- @Ignore("TODO(b/113859361): merge builder.")
- @Test
- public void testNestedBuilders_conditional() {
- buildAndCheckIR(
- "nestedBuilders_conditional",
- checkOptimizerStates(appView, optimizer -> {
- assertEquals(1, optimizer.analysis.builderStates.size());
- for (Value builder : optimizer.analysis.builderStates.keySet()) {
- Map<Instruction, BuilderState> perBuilderState =
- optimizer.analysis.builderStates.get(builder);
- checkBuilderState(optimizer, perBuilderState, null, true);
- }
- assertEquals(0, optimizer.analysis.simplifiedBuilders.size());
- // TODO(b/113859361): check # of merged builders.
- // assertEquals(3, optimizer.analysis.mergedBuilders.size());
- }));
- }
-
- @Ignore("TODO(b/113859361): merge builder.")
- @Test
- public void testConcatenatedBuilders_init() {
- buildAndCheckIR(
- "concatenatedBuilders_init",
- checkOptimizerStates(appView, optimizer -> {
- assertEquals(1, optimizer.analysis.builderStates.size());
- for (Value builder : optimizer.analysis.builderStates.keySet()) {
- Map<Instruction, BuilderState> perBuilderState =
- optimizer.analysis.builderStates.get(builder);
- checkBuilderState(optimizer, perBuilderState, "Hello,R8", true);
- }
- assertEquals(1, optimizer.analysis.simplifiedBuilders.size());
- // TODO(b/113859361): check # of merged builders.
- // assertEquals(2, optimizer.analysis.mergedBuilders.size());
- }));
- }
-
- @Ignore("TODO(b/113859361): merge builder.")
- @Test
- public void testConcatenatedBuilders_append() {
- buildAndCheckIR(
- "concatenatedBuilders_append",
- checkOptimizerStates(appView, optimizer -> {
- assertEquals(1, optimizer.analysis.builderStates.size());
- for (Value builder : optimizer.analysis.builderStates.keySet()) {
- Map<Instruction, BuilderState> perBuilderState =
- optimizer.analysis.builderStates.get(builder);
- checkBuilderState(optimizer, perBuilderState, "Hello,R8", true);
- }
- assertEquals(1, optimizer.analysis.simplifiedBuilders.size());
- // TODO(b/113859361): check # of merged builders.
- // assertEquals(2, optimizer.analysis.mergedBuilders.size());
- }));
- }
-
- @Ignore("TODO(b/113859361): merge builder.")
- @Test
- public void testConcatenatedBuilders_conditional() {
- final Set<String> expectedConstStrings = new HashSet<>();
- expectedConstStrings.add("Hello,R8");
- expectedConstStrings.add("D8");
- buildAndCheckIR(
- "concatenatedBuilders_conditional",
- 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);
- checkBuilderStates(optimizer, perBuilderState, expectedConstStrings, true);
- }
- assertEquals(2, optimizer.analysis.simplifiedBuilders.size());
- // TODO(b/113859361): check # of merged builders.
- // assertEquals(2, optimizer.analysis.mergedBuilders.size());
- }));
- }
-
- @Test
- public void testSimplePhi() {
- buildAndCheckIR(
- "simplePhi",
- checkOptimizerStates(appView, optimizer -> {
- assertEquals(0, optimizer.analysis.builderStates.size());
- assertEquals(0, optimizer.analysis.simplifiedBuilders.size());
- }));
- }
-
- @Test
- public void testPhiAtInit() {
- int expectedNumOfNewBuilder = 0;
- boolean expectToMeetToString = false;
- if (parameters.isDexRuntime() && parameters.getApiLevel().isLessThan(AndroidApiLevel.M)) {
- expectedNumOfNewBuilder = 1;
- expectToMeetToString = true;
- }
- final int finalExpectedNumOfNewBuilder = expectedNumOfNewBuilder;
- final boolean finalExpectToMeetToString = expectToMeetToString;
- buildAndCheckIR(
- "phiAtInit",
- checkOptimizerStates(appView, optimizer -> {
- assertEquals(finalExpectedNumOfNewBuilder, optimizer.analysis.builderStates.size());
- for (Value builder : optimizer.analysis.builderStates.keySet()) {
- Map<Instruction, BuilderState> perBuilderState =
- optimizer.analysis.builderStates.get(builder);
- checkBuilderState(optimizer, perBuilderState, null, finalExpectToMeetToString);
- }
- assertEquals(0, optimizer.analysis.simplifiedBuilders.size());
- }));
- }
-
- @Test
- public void testPhiWithDifferentInits() {
- buildAndCheckIR(
- "phiWithDifferentInits",
- 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 testConditionalPhiWithoutAppend() {
- buildAndCheckIR(
- "conditionalPhiWithoutAppend",
- checkOptimizerStates(
- appView,
- optimizer -> {
- assertEquals(1, optimizer.analysis.builderStates.size());
- for (Value builder : optimizer.analysis.builderStates.keySet()) {
- Map<Instruction, BuilderState> perBuilderState =
- optimizer.analysis.builderStates.get(builder);
- checkBuilderState(optimizer, perBuilderState, "initial:suffix", true);
- }
- assertEquals(1, optimizer.analysis.simplifiedBuilders.size());
- }));
- }
-
- @Test
- public void testLoop() {
- buildAndCheckIR(
- "loop",
- 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, true);
- }
- assertEquals(0, optimizer.analysis.simplifiedBuilders.size());
- }));
- }
-
- @Test
- public void testLoopWithBuilder() {
- buildAndCheckIR(
- "loopWithBuilder",
- checkOptimizerStates(appView, optimizer -> {
- assertEquals(1, optimizer.analysis.builderStates.size());
- for (Value builder : optimizer.analysis.builderStates.keySet()) {
- Map<Instruction, BuilderState> perBuilderState =
- optimizer.analysis.builderStates.get(builder);
- checkBuilderState(optimizer, perBuilderState, null, true);
- }
- assertEquals(0, optimizer.analysis.simplifiedBuilders.size());
- }));
- }
-
- static Consumer<IRCode> checkOptimizerStates(
- AppView<?> appView, Consumer<StringBuilderOptimizer> optimizerConsumer) {
- return code -> {
- StringBuilderOptimizer optimizer = new StringBuilderOptimizer(appView);
- optimizer.computeTrivialStringConcatenation(code);
- optimizerConsumer.accept(optimizer);
- };
- }
-
- static void checkBuilderState(
- StringBuilderOptimizer optimizer,
- Map<Instruction, BuilderState> perBuilderState,
- String expectedConstString,
- boolean expectToSeeMethodToString) {
- boolean seenMethodToString = false;
- for (Map.Entry<Instruction, BuilderState> entry : perBuilderState.entrySet()) {
- InvokeMethod invoke = entry.getKey().asInvokeMethod();
- if (invoke != null
- && optimizer.optimizationConfiguration.isToStringMethod(invoke.getInvokedMethod())) {
- seenMethodToString = true;
- Value builder = invoke.getArgument(0).getAliasedValue();
- assertEquals(
- expectedConstString, optimizer.analysis.toCompileTimeString(builder, entry.getValue()));
- }
- }
- assertEquals(expectToSeeMethodToString, seenMethodToString);
- }
-
- static void checkBuilderStates(
- StringBuilderOptimizer optimizer,
- Map<Instruction, BuilderState> perBuilderState,
- Set<String> expectedConstStrings,
- boolean expectToSeeMethodToString) {
- boolean seenMethodToString = false;
- for (Map.Entry<Instruction, BuilderState> entry : perBuilderState.entrySet()) {
- InvokeMethod invoke = entry.getKey().asInvokeMethod();
- if (invoke != null
- && optimizer.optimizationConfiguration.isToStringMethod(invoke.getInvokedMethod())) {
- seenMethodToString = true;
- Value builder = invoke.getArgument(0).getAliasedValue();
- String computedString = optimizer.analysis.toCompileTimeString(builder, entry.getValue());
- assertNotNull(computedString);
- assertTrue(expectedConstStrings.contains(computedString));
- expectedConstStrings.remove(computedString);
- }
- }
- assertEquals(expectToSeeMethodToString, seenMethodToString);
- }
-}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderTests.java b/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderTests.java
index f908b95..365ced3 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderTests.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderTests.java
@@ -37,7 +37,7 @@
@Parameters(name = "{0}, configuration: {1}")
public static List<Object[]> data() {
return buildParameters(
- getTestParameters().withAllRuntimesAndApiLevels().build(), getTestExpectations());
+ getTestParameters().withDexRuntimes().withAllApiLevels().build(), getTestExpectations());
}
private static class StringBuilderResult {
@@ -94,14 +94,14 @@
StringBuilderResult.create(
Main.class.getMethod("materializingWithAdditionalAppend"),
StringUtils.lines("Hello World", "Hello WorldObservable"),
- 1,
- 3,
- 2),
+ 0,
+ 0,
+ 0),
StringBuilderResult.create(
Main.class.getMethod("appendWithNonConstant"),
StringUtils.lines("Hello World, Hello World"),
1,
- 3,
+ 2,
1),
StringBuilderResult.create(
Main.class.getMethod("simpleLoopTest"),
@@ -109,13 +109,12 @@
1,
1,
1),
- // TODO(b/222437581): Should not remove StringBuilder
StringBuilderResult.create(
Main.class.getMethod("simpleLoopTest2"),
StringUtils.lines("Hello World", "Hello WorldHello World"),
- 0,
- 0,
- 0),
+ 1,
+ 1,
+ 1),
StringBuilderResult.create(
Main.class.getMethod("simpleLoopWithStringBuilderInBodyTest"),
StringUtils.lines("Hello World"),
@@ -129,27 +128,27 @@
0,
0),
StringBuilderResult.create(
- Main.class.getMethod("diamondWithUseTest"), StringUtils.lines("Hello World"), 1, 3, 1),
+ Main.class.getMethod("diamondWithUseTest"), StringUtils.lines("Hello World"), 1, 2, 1),
StringBuilderResult.create(
Main.class.getMethod("diamondsWithSingleUseTest"),
StringUtils.lines("Hello World"),
1,
- 3,
+ 2,
1),
StringBuilderResult.create(
Main.class.getMethod("escapeTest"), StringUtils.lines("Hello World"), 2, 2, 1),
StringBuilderResult.create(
Main.class.getMethod("intoPhiTest"), StringUtils.lines("Hello World"), 2, 2, 1),
StringBuilderResult.create(
- Main.class.getMethod("optimizePartial"), StringUtils.lines("Hello World.."), 1, 4, 1),
+ Main.class.getMethod("optimizePartial"), StringUtils.lines("Hello World.."), 1, 2, 1),
StringBuilderResult.create(
Main.class.getMethod("multipleToStrings"),
StringUtils.lines("Hello World", "Hello World.."),
- 1,
- 4,
- 2),
+ 0,
+ 0,
+ 0),
StringBuilderResult.create(
- Main.class.getMethod("changeAppendType"), StringUtils.lines("1 World"), 1, 3, 1),
+ Main.class.getMethod("changeAppendType"), StringUtils.lines("1 World"), 1, 2, 1),
StringBuilderResult.create(
Main.class.getMethod("checkCapacity"), StringUtils.lines("true"), 2, 1, 0),
StringBuilderResult.create(
@@ -157,29 +156,29 @@
StringBuilderResult.create(
Main.class.getMethod("stringBuilderWithStringBuilderToString"),
StringUtils.lines("Hello World"),
- 1,
- 1,
- 1),
+ 0,
+ 0,
+ 0),
StringBuilderResult.create(
Main.class.getMethod("stringBuilderWithStringBuilder"),
StringUtils.lines("Hello World"),
- 2,
- 2,
- 1),
+ 0,
+ 0,
+ 0),
StringBuilderResult.create(
Main.class.getMethod("stringBuilderInStringBuilderConstructor"),
StringUtils.lines("Hello World"),
- 2,
- 1,
- 1),
+ 0,
+ 0,
+ 0),
StringBuilderResult.create(
Main.class.getMethod("interDependencyTest"),
StringUtils.lines("World Hello World "),
- 2,
- 2,
- 1),
+ 0,
+ 0,
+ 0),
StringBuilderResult.create(
- Main.class.getMethod("stringBuilderSelfReference"), StringUtils.lines(""), 1, 1, 1),
+ Main.class.getMethod("stringBuilderSelfReference"), StringUtils.lines(""), 0, 0, 0),
StringBuilderResult.create(
Main.class.getMethod("unknownStringBuilderInstruction"),
StringUtils.lines("Hello World"),
@@ -214,8 +213,6 @@
@Test
public void testR8() throws Exception {
- boolean hasError =
- stringBuilderTest.getMethodName().equals("simpleLoopTest2") && parameters.isDexRuntime();
compilationResults
.apply(parameters)
.inspect(
@@ -235,9 +232,7 @@
stringBuilderTest.toStrings, countStringBuilderToStrings(foundMethodSubject));
})
.run(parameters.getRuntime(), Main.class, stringBuilderTest.getMethodName())
- // TODO(b/222437581): Incorrect result for string builder inside loop.
- .assertSuccessWithOutputLinesIf(hasError, "Hello World", "Hello World")
- .assertSuccessWithOutputIf(!hasError, stringBuilderTest.expected);
+ .assertSuccessWithOutput(stringBuilderTest.expected);
}
private long countStringBuilderInits(FoundMethodSubject method) {
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderWithIndirectMutationThroughPhiTest.java b/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderWithIndirectMutationThroughPhiTest.java
index bd6fffd..88f0cf8 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderWithIndirectMutationThroughPhiTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderWithIndirectMutationThroughPhiTest.java
@@ -41,10 +41,8 @@
.inspect(
inspector -> {
MethodSubject mainMethodSubject = inspector.clazz(Main.class).mainMethod();
- // TODO(b/114002137): This should be optimized into having only 3 append calls,
- // since append("baz").append("qux") can be optimized into append("bazqux").
assertEquals(
- 4,
+ parameters.isCfRuntime() ? 4 : 3,
mainMethodSubject
.streamInstructions()
.filter(isInvokeStringBuilderAppendWithString())
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderWithIndirectPhiMutationThroughPhiOperandTest.java b/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderWithIndirectPhiMutationThroughPhiOperandTest.java
index b553e2d..81fa83b 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderWithIndirectPhiMutationThroughPhiOperandTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/string/StringBuilderWithIndirectPhiMutationThroughPhiOperandTest.java
@@ -41,10 +41,9 @@
.inspect(
inspector -> {
MethodSubject mainMethodSubject = inspector.clazz(Main.class).mainMethod();
- // TODO(b/114002137): This should be optimized into having only 3 append calls,
- // since append("baz").append("qux") can be optimized into append("bazqux").
+ // TODO(b/114002137): Also run for CF
assertEquals(
- 4,
+ parameters.isCfRuntime() ? 4 : 3,
mainMethodSubject
.streamInstructions()
.filter(isInvokeStringBuilderAppendWithString())
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/string/StringConcatenationTest.java b/src/test/java/com/android/tools/r8/ir/optimize/string/StringConcatenationTest.java
index ef8217a..617fd63 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/string/StringConcatenationTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/string/StringConcatenationTest.java
@@ -9,7 +9,6 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assume.assumeTrue;
-import com.android.tools.r8.D8TestRunResult;
import com.android.tools.r8.R8TestRunResult;
import com.android.tools.r8.SingleTestRunResult;
import com.android.tools.r8.TestBase;
@@ -113,27 +112,27 @@
method = mainClass.uniqueMethodWithName("trivialSequence");
assertThat(method, isPresent());
- expectedCount = isR8 ? 1 : 3;
+ expectedCount = isReleaseMode ? 1 : 3;
assertEquals(expectedCount, countConstString(method));
method = mainClass.uniqueMethodWithName("builderWithInitialValue");
assertThat(method, isPresent());
- expectedCount = isR8 ? 1 : 3;
+ expectedCount = isReleaseMode ? 1 : 3;
assertEquals(expectedCount, countConstString(method));
method = mainClass.uniqueMethodWithName("builderWithCapacity");
assertThat(method, isPresent());
- expectedCount = isR8 ? 1 : 0;
+ expectedCount = isReleaseMode ? 1 : 0;
assertEquals(expectedCount, countConstString(method));
method = mainClass.uniqueMethodWithName("nonStringArgs");
assertThat(method, isPresent());
- expectedCount = isR8 ? 1 : 0;
+ expectedCount = isReleaseMode ? 1 : 0;
assertEquals(expectedCount, countConstString(method));
method = mainClass.uniqueMethodWithName("typeConversion");
assertThat(method, isPresent());
- expectedCount = isR8 ? 1 : 0;
+ expectedCount = isReleaseMode ? 1 : 0;
assertEquals(expectedCount, countConstString(method));
method = mainClass.uniqueMethodWithName("typeConversion_withPhis");
@@ -142,41 +141,31 @@
method = mainClass.uniqueMethodWithName("nestedBuilders_appendBuilderItself");
assertThat(method, isPresent());
- // TODO(b/113859361): merge builders
- expectedCount = 3;
- assertEquals(expectedCount, countConstString(method));
+ assertEquals(isReleaseMode ? 1 : 3, countConstString(method));
method = mainClass.uniqueMethodWithName("nestedBuilders_appendBuilderResult");
assertThat(method, isPresent());
- // TODO(b/113859361): merge builders
- expectedCount = 3;
- assertEquals(expectedCount, countConstString(method));
+ assertEquals(isReleaseMode ? 1 : 3, countConstString(method));
method = mainClass.uniqueMethodWithName("nestedBuilders_conditional");
assertThat(method, isPresent());
- assertEquals(4, countConstString(method));
+ assertEquals(isReleaseMode ? 3 : 4, countConstString(method));
method = mainClass.uniqueMethodWithName("concatenatedBuilders_init");
assertThat(method, isPresent());
- // TODO(b/113859361): merge builders
- expectedCount = 2;
- assertEquals(expectedCount, countConstString(method));
+ assertEquals(isReleaseMode ? 1 : 2, countConstString(method));
method = mainClass.uniqueMethodWithName("concatenatedBuilders_append");
assertThat(method, isPresent());
- // TODO(b/113859361): merge builders
- expectedCount = 2;
- assertEquals(expectedCount, countConstString(method));
+ assertEquals(isReleaseMode ? 1 : 2, countConstString(method));
method = mainClass.uniqueMethodWithName("concatenatedBuilders_conditional");
assertThat(method, isPresent());
- // TODO(b/113859361): merge builders
- expectedCount = 4;
- assertEquals(expectedCount, countConstString(method));
+ assertEquals(isReleaseMode ? 2 : 4, countConstString(method));
method = mainClass.uniqueMethodWithName("simplePhi");
assertThat(method, isPresent());
- assertEquals(4, countConstString(method));
+ assertEquals(isReleaseMode ? 3 : 4, countConstString(method));
method = mainClass.uniqueMethodWithName("phiAtInit");
assertThat(method, isPresent());
@@ -188,7 +177,7 @@
method = mainClass.uniqueMethodWithName("conditionalPhiWithoutAppend");
assertThat(method, isPresent());
- assertEquals(3, countConstString(method));
+ assertEquals(isReleaseMode ? 2 : 3, countConstString(method));
method = mainClass.uniqueMethodWithName("loop");
assertThat(method, isPresent());
@@ -204,26 +193,31 @@
}
@Test
- public void testD8() throws Exception {
+ public void testD8Debug() throws Exception {
assumeTrue("Only run D8 for Dex backend", parameters.isDexRuntime());
-
- D8TestRunResult result =
+ test(
testForD8()
.debug()
.addProgramClasses(MAIN)
.setMinApi(parameters.getApiLevel())
.run(parameters.getRuntime(), MAIN)
- .assertSuccessWithOutput(JAVA_OUTPUT);
- test(result, false, false);
+ .assertSuccessWithOutput(JAVA_OUTPUT),
+ false,
+ false);
+ }
- result =
+ @Test
+ public void testD8Release() throws Exception {
+ assumeTrue("Only run D8 for Dex backend", parameters.isDexRuntime());
+ test(
testForD8()
.release()
.addProgramClasses(MAIN)
.setMinApi(parameters.getApiLevel())
.run(parameters.getRuntime(), MAIN)
- .assertSuccessWithOutput(JAVA_OUTPUT);
- test(result, false, true);
+ .assertSuccessWithOutput(JAVA_OUTPUT),
+ false,
+ true);
}
@Test
diff --git a/src/test/java/com/android/tools/r8/kotlin/stringplus/StringPlusTest.java b/src/test/java/com/android/tools/r8/kotlin/stringplus/StringPlusTest.java
index 170595a..37b20bd 100644
--- a/src/test/java/com/android/tools/r8/kotlin/stringplus/StringPlusTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/stringplus/StringPlusTest.java
@@ -8,6 +8,7 @@
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
import com.android.tools.r8.CompilationFailedException;
import com.android.tools.r8.KotlinCompilerTool;
@@ -100,11 +101,18 @@
// TODO(b/190489514): We should be able to optimize constant stringPlus calls.
assertThat(methodSubject, CodeMatchers.invokesMethodWithName("stringPlus"));
}
- // TODO(b/219455761): StringBuilderOptimizer fails to remove constant input.
- assertThat(
- methodSubject,
- CodeMatchers.invokesMethodWithHolderAndName(
- typeName(StringBuilder.class), "append"));
+ // We cannot remove the <init> -> <append> call since that changes the capacity
+ // and the string builder is escaping into System.out.
+ assertEquals(
+ 1,
+ methodSubject
+ .streamInstructions()
+ .filter(
+ instructionSubject ->
+ CodeMatchers.isInvokeWithTarget(
+ typeName(StringBuilder.class), "append")
+ .test(instructionSubject))
+ .count());
})
.assertAllWarningMessagesMatch(equalTo("Resource 'META-INF/MANIFEST.MF' already exists."))
.run(parameters.getRuntime(), MAIN)
diff --git a/src/test/java/com/android/tools/r8/smali/JumboStringTest.java b/src/test/java/com/android/tools/r8/smali/JumboStringTest.java
index 20fda04..e7b5fe6 100644
--- a/src/test/java/com/android/tools/r8/smali/JumboStringTest.java
+++ b/src/test/java/com/android/tools/r8/smali/JumboStringTest.java
@@ -20,11 +20,19 @@
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
@RunWith(Parameterized.class)
public class JumboStringTest extends SmaliTestBase {
private static Pair<StringBuilder, StringBuilder> builders;
- private final TestParameters parameters;
+
+ @Parameter() public TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withDexRuntimes().withOnlyDexRuntimeApiLevel().build();
+ }
@BeforeClass
public static void createBuilders() {
@@ -49,15 +57,6 @@
builders = new Pair<>(builder, expectedBuilder);
}
- @Parameterized.Parameters(name = "{0}")
- public static TestParametersCollection data() {
- return getTestParameters().withDexRuntimes().build();
- }
-
- public JumboStringTest(TestParameters parameters) {
- this.parameters = parameters;
- }
-
@Test
public void test() throws Exception {
SmaliBuilder smaliBuilder = new SmaliBuilder(DEFAULT_CLASS_NAME);
@@ -82,16 +81,21 @@
testForR8(parameters.getBackend())
.addProgramDexFileData(smaliBuilder.compile())
.addKeepMainRule(DEFAULT_CLASS_NAME)
- .setMinApi(parameters.getRuntime())
+ .setMinApi(parameters.getApiLevel())
+ .addOptionsModification(
+ options -> {
+ options.enableStringConcatenationOptimization = false;
+ })
.run(parameters.getRuntime(), DEFAULT_CLASS_NAME)
.assertSuccessWithOutput(builders.getSecond().toString())
- .inspect(inspector -> {
- ClassSubject main = inspector.clazz(DEFAULT_CLASS_NAME);
- assertThat(main, isPresent());
- MethodSubject method = main.uniqueMethodWithName(DEFAULT_METHOD_NAME);
- assertThat(method, isPresent());
- assertTrue(method.streamInstructions().anyMatch(InstructionSubject::isJumboString));
- });
+ .inspect(
+ inspector -> {
+ ClassSubject main = inspector.clazz(DEFAULT_CLASS_NAME);
+ assertThat(main, isPresent());
+ MethodSubject method = main.uniqueMethodWithName(DEFAULT_METHOD_NAME);
+ assertThat(method, isPresent());
+ assertTrue(method.streamInstructions().anyMatch(InstructionSubject::isJumboString));
+ });
}
@Test
@@ -128,18 +132,23 @@
.addProgramDexFileData(smaliBuilder.compile())
.addKeepMainRule(DEFAULT_CLASS_NAME)
.addKeepRules("-addconfigurationdebugging")
- .setMinApi(parameters.getRuntime())
+ .setMinApi(parameters.getApiLevel())
+ .addOptionsModification(
+ options -> {
+ options.enableStringConcatenationOptimization = false;
+ })
.run(parameters.getRuntime(), DEFAULT_CLASS_NAME)
.assertSuccessWithOutput(builders.getSecond().toString())
- .inspect(inspector -> {
- ClassSubject main = inspector.clazz(DEFAULT_CLASS_NAME);
- assertThat(main, isPresent());
- MethodSubject method = main.uniqueMethodWithName(DEFAULT_METHOD_NAME);
- assertThat(method, isPresent());
- assertTrue(method.streamInstructions().anyMatch(InstructionSubject::isJumboString));
- MethodSubject method2 = main.uniqueMethodWithName(DEFAULT_METHOD_NAME + "2");
- assertThat(method2, isPresent());
- assertTrue(method2.streamInstructions().anyMatch(InstructionSubject::isJumboString));
- });
+ .inspect(
+ inspector -> {
+ ClassSubject main = inspector.clazz(DEFAULT_CLASS_NAME);
+ assertThat(main, isPresent());
+ MethodSubject method = main.uniqueMethodWithName(DEFAULT_METHOD_NAME);
+ assertThat(method, isPresent());
+ assertTrue(method.streamInstructions().anyMatch(InstructionSubject::isJumboString));
+ MethodSubject method2 = main.uniqueMethodWithName(DEFAULT_METHOD_NAME + "2");
+ assertThat(method2, isPresent());
+ assertTrue(method2.streamInstructions().anyMatch(InstructionSubject::isJumboString));
+ });
}
}
