Canonicalize constant classes.
Bug: 129381548
Change-Id: I99e83466ef35a9890e03b5ee95cce36313816bf3
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 a7a7bbc..a643c9a 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
@@ -1047,7 +1047,7 @@
// TODO(mkroghj) Test if shorten live ranges is worth it.
if (!options.isGeneratingClassFiles()) {
- ConstantCanonicalizer.canonicalize(code);
+ ConstantCanonicalizer.canonicalize(appView, code);
codeRewriter.useDedicatedConstantForLitInstruction(code);
codeRewriter.shortenLiveRanges(code);
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java b/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
index 53da564..4a2e847 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
@@ -3,7 +3,10 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.ir.optimize;
+import com.android.tools.r8.graph.AppInfo;
+import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.ir.code.BasicBlock;
+import com.android.tools.r8.ir.code.ConstClass;
import com.android.tools.r8.ir.code.ConstInstruction;
import com.android.tools.r8.ir.code.ConstNumber;
import com.android.tools.r8.ir.code.ConstString;
@@ -27,18 +30,23 @@
private static final int MAX_CANONICALIZED_CONSTANT = 15;
- public static void canonicalize(IRCode code) {
+ public static void canonicalize(AppView<? extends AppInfo> appView, IRCode code) {
Object2ObjectLinkedOpenCustomHashMap<ConstInstruction, List<Value>> valuesDefinedByConstant =
new Object2ObjectLinkedOpenCustomHashMap<>(
new Strategy<ConstInstruction>() {
@Override
public int hashCode(ConstInstruction constInstruction) {
- assert constInstruction.isConstNumber() || constInstruction.isConstString();
+ assert constInstruction.isConstNumber()
+ || constInstruction.isConstString()
+ || constInstruction.isConstClass();
if (constInstruction.isConstNumber()) {
return Long.hashCode(constInstruction.asConstNumber().getRawValue())
+ 13 * constInstruction.outType().hashCode();
}
- return constInstruction.asConstString().getValue().hashCode();
+ if (constInstruction.isConstString()) {
+ return constInstruction.asConstString().getValue().hashCode();
+ }
+ return constInstruction.asConstClass().getValue().hashCode();
}
@Override
@@ -56,8 +64,14 @@
InstructionListIterator it = block.listIterator();
while (it.hasNext()) {
Instruction current = it.next();
- // Interested in ConstNumber and ConstString.
- if (!current.isConstNumber() && !current.isConstString()) {
+ // Interested in ConstNumber, ConstString, and ConstClass
+ if (!current.isConstNumber() && !current.isConstString() && !current.isConstClass()) {
+ continue;
+ }
+ // Do not canonicalize ConstClass that may have side effects. Its original instructions
+ // will not be removed by dead code remover due to the side effects.
+ if (current.isConstClass()
+ && current.instructionMayHaveSideEffects(appView, code.method.method.holder)) {
continue;
}
// Do not canonicalize ConstString instructions if there are monitor operations in the code.
@@ -102,14 +116,19 @@
.limit(MAX_CANONICALIZED_CONSTANT)
.forEach((entry) -> {
ConstInstruction canonicalizedConstant = entry.getKey().asConstInstruction();
- assert canonicalizedConstant.isConstNumber() || canonicalizedConstant.isConstString();
+ assert canonicalizedConstant.isConstNumber()
+ || canonicalizedConstant.isConstString()
+ || canonicalizedConstant.isConstClass();
ConstInstruction newConst;
if (canonicalizedConstant.isConstNumber()) {
ConstNumber canonicalizedConstantNumber = canonicalizedConstant.asConstNumber();
newConst = ConstNumber.copyOf(code, canonicalizedConstantNumber);
- } else {
+ } else if (canonicalizedConstant.isConstString()) {
ConstString canonicalizedConstantString = canonicalizedConstant.asConstString();
newConst = ConstString.copyOf(code, canonicalizedConstantString);
+ } else {
+ ConstClass canonicalizedConstClass = canonicalizedConstant.asConstClass();
+ newConst = ConstClass.copyOf(code, canonicalizedConstClass);
}
newConst.setPosition(firstNonNonePosition);
insertCanonicalizedConstant(code, newConst);
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/ConstClassCanonicalizationTest.java b/src/test/java/com/android/tools/r8/ir/optimize/ConstClassCanonicalizationTest.java
new file mode 100644
index 0000000..4e8fdc8
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/ConstClassCanonicalizationTest.java
@@ -0,0 +1,219 @@
+// 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;
+
+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.D8TestRunResult;
+import com.android.tools.r8.DexIndexedConsumer.ArchiveConsumer;
+import com.android.tools.r8.R8TestRunResult;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.TestRunResult;
+import com.android.tools.r8.ir.optimize.ConstClassMain.Outer;
+import com.android.tools.r8.ir.optimize.ConstClassMain.Outer.Inner;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.InstructionSubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import com.google.common.collect.Streams;
+import java.nio.file.Path;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+class IncrementalA {
+}
+
+class IncrementalMain {
+ public static void main(String... args) {
+ System.out.println(IncrementalA.class.getSimpleName());
+ System.out.println(IncrementalA.class.getSimpleName());
+ System.out.println(IncrementalA.class.getSimpleName());
+ }
+}
+
+class ConstClassMain {
+
+ static class Outer {
+ Outer() {
+ System.out.println("Outer::<init>");
+ }
+
+ static class Inner {
+ Inner() {
+ System.out.println("Inner::<init>");
+ }
+ }
+ }
+
+ public static void main(String... args) {
+ Class outer = Inner.class.getDeclaringClass();
+ System.out.println(outer.getSimpleName());
+ for (Class<?> inner : outer.getDeclaredClasses()) {
+ System.out.println(inner.getSimpleName());
+ }
+ System.out.println(ConstClassMain.class.getSimpleName());
+ System.out.println(Outer.class.getSimpleName());
+ System.out.println(Inner.class.getSimpleName());
+ System.out.println(ConstClassMain.class.getSimpleName().length());
+ System.out.println(Outer.class.getSimpleName().length());
+ System.out.println(Inner.class.getSimpleName().length());
+ }
+}
+
+@RunWith(Parameterized.class)
+public class ConstClassCanonicalizationTest extends TestBase {
+ private static final String INCREMENTAL_OUTPUT = StringUtils.lines(
+ "IncrementalA",
+ "IncrementalA",
+ "IncrementalA"
+ );
+
+ private static final Class<?> MAIN = ConstClassMain.class;
+ private static final String JAVA_OUTPUT = StringUtils.lines(
+ "Outer",
+ "Inner",
+ "ConstClassMain",
+ "Outer",
+ "Inner",
+ "14",
+ "5",
+ "5"
+ );
+ private static final int ORIGINAL_MAIN_COUNT = 2;
+ private static final int ORIGINAL_OUTER_COUNT = 2;
+ private static final int ORIGINAL_INNER_COUNT = 3;
+ private static final int CANONICALIZED_MAIN_COUNT = 1;
+ private static final int CANONICALIZED_OUTER_COUNT = 1;
+ private static final int CANONICALIZED_INNER_COUNT = 1;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimes().build();
+ }
+
+ private final TestParameters parameters;
+
+ public ConstClassCanonicalizationTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testJVMOutput() throws Exception {
+ assumeTrue(
+ "Only run JVM reference once (for CF backend)",
+ parameters.getBackend() == Backend.CF);
+ testForJvm()
+ .addTestClasspath()
+ .run(parameters.getRuntime(), MAIN)
+ .assertSuccessWithOutput(JAVA_OUTPUT);
+ }
+
+ @Test
+ public void testD8_incremental() throws Exception {
+ assumeTrue("Only run D8 for Dex backend", parameters.getBackend() == Backend.DEX);
+
+ Path zipA = temp.newFile("a.zip").toPath().toAbsolutePath();
+ testForD8()
+ .release()
+ .addProgramClasses(IncrementalA.class)
+ .setMinApi(parameters.getRuntime())
+ .setProgramConsumer(new ArchiveConsumer(zipA))
+ .compile();
+ testForD8()
+ .release()
+ .addProgramClasses(IncrementalMain.class)
+ .setMinApi(parameters.getRuntime())
+ .compile()
+ .addRunClasspathFiles(zipA)
+ .run(parameters.getRuntime(), IncrementalMain.class)
+ .assertSuccessWithOutput(INCREMENTAL_OUTPUT)
+ .inspect(inspector -> {
+ ClassSubject main = inspector.clazz(IncrementalMain.class);
+ assertThat(main, isPresent());
+ MethodSubject mainMethod = main.mainMethod();
+ assertThat(mainMethod, isPresent());
+ assertEquals(
+ 3,
+ Streams.stream(
+ mainMethod.iterateInstructions(InstructionSubject::isConstClass))
+ .count());
+ });
+ }
+
+ private void test(
+ TestRunResult result, int mainCount, int outerCount, int innerCount) throws Exception {
+ CodeInspector codeInspector = result.inspector();
+ ClassSubject mainClass = codeInspector.clazz(MAIN);
+ MethodSubject mainMethod = mainClass.mainMethod();
+ assertThat(mainMethod, isPresent());
+ assertEquals(
+ mainCount,
+ Streams.stream(
+ mainMethod.iterateInstructions(i -> i.isConstClass(MAIN.getTypeName())))
+ .count());
+ assertEquals(
+ outerCount,
+ Streams.stream(
+ mainMethod.iterateInstructions(i -> i.isConstClass(Outer.class.getTypeName())))
+ .count());
+ assertEquals(
+ innerCount,
+ Streams.stream(
+ mainMethod.iterateInstructions(i -> i.isConstClass(Inner.class.getTypeName())))
+ .count());
+ }
+
+ @Test
+ public void testD8() throws Exception {
+ assumeTrue("Only run D8 for Dex backend", parameters.getBackend() == Backend.DEX);
+
+ D8TestRunResult result =
+ testForD8()
+ .debug()
+ .addProgramClassesAndInnerClasses(MAIN)
+ .setMinApi(parameters.getRuntime())
+ .run(parameters.getRuntime(), MAIN)
+ .assertSuccessWithOutput(JAVA_OUTPUT);
+ test(result, CANONICALIZED_MAIN_COUNT, ORIGINAL_OUTER_COUNT, ORIGINAL_INNER_COUNT);
+
+ result =
+ testForD8()
+ .release()
+ .addProgramClassesAndInnerClasses(MAIN)
+ .setMinApi(parameters.getRuntime())
+ .run(parameters.getRuntime(), MAIN)
+ .assertSuccessWithOutput(JAVA_OUTPUT);
+ test(result, CANONICALIZED_MAIN_COUNT, ORIGINAL_OUTER_COUNT, ORIGINAL_INNER_COUNT);
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ R8TestRunResult result =
+ testForR8(parameters.getBackend())
+ .addProgramClassesAndInnerClasses(MAIN)
+ .addKeepMainRule(MAIN)
+ .addKeepAllAttributes()
+ .noMinification()
+ .setMinApi(parameters.getRuntime())
+ .run(parameters.getRuntime(), MAIN)
+ .assertSuccessWithOutput(JAVA_OUTPUT);
+ // The number of expected const-class instructions differs because constant canonicalization is
+ // only enabled for the DEX backend.
+ int expectedMainCount =
+ parameters.getBackend() == Backend.CF ? ORIGINAL_MAIN_COUNT : CANONICALIZED_MAIN_COUNT;
+ int expectedOuterCount =
+ parameters.getBackend() == Backend.CF ? ORIGINAL_OUTER_COUNT : CANONICALIZED_OUTER_COUNT;
+ int expectedInnerCount =
+ parameters.getBackend() == Backend.CF ? ORIGINAL_INNER_COUNT : CANONICALIZED_INNER_COUNT;
+ test(result, expectedMainCount, expectedOuterCount, expectedInnerCount);
+ }
+
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/IdempotentFunctionCallCanonicalizationTest.java b/src/test/java/com/android/tools/r8/ir/optimize/IdempotentFunctionCallCanonicalizationTest.java
index 27cd963..7bb6c66 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/IdempotentFunctionCallCanonicalizationTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/IdempotentFunctionCallCanonicalizationTest.java
@@ -111,7 +111,10 @@
assumeTrue(
"Only run JVM reference once (for CF backend)",
parameters.getBackend() == Backend.CF);
- testForJvm().addTestClasspath().run(MAIN).assertSuccessWithOutput(JAVA_OUTPUT);
+ testForJvm()
+ .addTestClasspath()
+ .run(parameters.getRuntime(), MAIN)
+ .assertSuccessWithOutput(JAVA_OUTPUT);
}
private static boolean isValueOf(DexMethod method, String descriptor) {
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/reflection/GetClassTest.java b/src/test/java/com/android/tools/r8/ir/optimize/reflection/GetClassTest.java
index 8854018..c78f4b9 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/reflection/GetClassTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/reflection/GetClassTest.java
@@ -232,6 +232,10 @@
.run(parameters.getRuntime(), MAIN);
test(result, 5, 1, 1, 0);
+ // The number of expected const-class instructions differs because constant canonicalization is
+ // only enabled for the DEX backend.
+ int expectedConstClassCount = parameters.getBackend() == Backend.CF ? 7 : 5;
+
// R8 release, no minification.
result =
testForR8(parameters.getBackend())
@@ -243,7 +247,7 @@
.setMinApi(parameters.getRuntime())
.run(parameters.getRuntime(), MAIN)
.assertSuccessWithOutput(JAVA_OUTPUT);
- test(result, 0, 7, 0, 1);
+ test(result, 0, expectedConstClassCount, 0, 1);
// R8 release, minification.
result =
@@ -255,7 +259,7 @@
.setMinApi(parameters.getRuntime())
// We are not checking output because it can't be matched due to minification. Just run.
.run(parameters.getRuntime(), MAIN);
- test(result, 0, 7, 0, 1);
+ test(result, 0, expectedConstClassCount, 0, 1);
}
}
