Move the materialization of constants for NewArrayEmpty rewriting
When NewArrayEmpty is rewritten to a series of aput instructions move
the marerialization of constant values to just before the aput if
possible.
Bug: b/297500960
Bug: b/297497196
Change-Id: I0ef9175b5b94781b8ac519e9bd5b7a0e70c1588f
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/passes/FilledNewArrayRewriter.java b/src/main/java/com/android/tools/r8/ir/conversion/passes/FilledNewArrayRewriter.java
index 4076241..9764e66 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/passes/FilledNewArrayRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/passes/FilledNewArrayRewriter.java
@@ -14,9 +14,12 @@
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.BasicBlockInstructionListIterator;
import com.android.tools.r8.ir.code.BasicBlockIterator;
+import com.android.tools.r8.ir.code.ConstClass;
import com.android.tools.r8.ir.code.ConstNumber;
+import com.android.tools.r8.ir.code.ConstString;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.Instruction;
+import com.android.tools.r8.ir.code.InstructionListIterator;
import com.android.tools.r8.ir.code.MemberType;
import com.android.tools.r8.ir.code.NewArrayEmpty;
import com.android.tools.r8.ir.code.NewArrayFilled;
@@ -25,13 +28,18 @@
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.conversion.passes.result.CodeRewriterResult;
import com.android.tools.r8.utils.InternalOptions.RewriteArrayOptions;
+import com.android.tools.r8.utils.SetUtils;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
+import java.util.Set;
public class FilledNewArrayRewriter extends CodeRewriterPass<AppInfo> {
private final RewriteArrayOptions rewriteArrayOptions;
+ private static final Set<Instruction> NOTHING = ImmutableSet.of();
private boolean mayHaveRedundantBlocks;
+ Set<Instruction> toRemove = NOTHING;
public FilledNewArrayRewriter(AppView<?> appView) {
super(appView);
@@ -45,6 +53,8 @@
@Override
protected CodeRewriterResult rewriteCode(IRCode code) {
+ assert !mayHaveRedundantBlocks;
+ assert toRemove == NOTHING;
BasicBlockIterator blockIterator = code.listIterator();
CodeRewriterResult result = noChange();
while (blockIterator.hasNext()) {
@@ -59,6 +69,15 @@
}
}
}
+ if (!toRemove.isEmpty()) {
+ InstructionListIterator it = code.instructionListIterator();
+ while (it.hasNext()) {
+ if (toRemove.contains(it.next())) {
+ it.remove();
+ mayHaveRedundantBlocks = true;
+ }
+ }
+ }
if (mayHaveRedundantBlocks) {
code.removeRedundantBlocks();
}
@@ -287,16 +306,58 @@
BasicBlock splitBlock =
instructionIterator.splitCopyCatchHandlers(code, blockIterator, options);
instructionIterator = splitBlock.listIterator(code);
- addArrayPut(code, instructionIterator, newArrayEmpty, index, elementValue);
+ Value putValue = getPutValue(code, instructionIterator, newArrayEmpty, elementValue);
+ blockIterator.positionAfterPreviousBlock(splitBlock);
+ splitBlock = instructionIterator.splitCopyCatchHandlers(code, blockIterator, options);
+ instructionIterator = splitBlock.listIterator(code);
+ addArrayPut(code, instructionIterator, newArrayEmpty, index, putValue);
blockIterator.positionAfterPreviousBlock(splitBlock);
mayHaveRedundantBlocks = true;
} else {
- addArrayPut(code, instructionIterator, newArrayEmpty, index, elementValue);
+ Value putValue = getPutValue(code, instructionIterator, newArrayEmpty, elementValue);
+ addArrayPut(code, instructionIterator, newArrayEmpty, index, putValue);
}
index++;
}
}
+ private Value getPutValue(
+ IRCode code,
+ BasicBlockInstructionListIterator instructionIterator,
+ NewArrayEmpty newArrayEmpty,
+ Value elementValue) {
+ // If the value was only used by the NewArrayFilled instruction it now has no normal users.
+ if (elementValue.hasAnyUsers()
+ || !(elementValue.isConstString()
+ || elementValue.isConstNumber()
+ || elementValue.isConstClass())) {
+ return elementValue;
+ }
+ Instruction copy;
+ if (elementValue.isConstNumber()) {
+ copy = ConstNumber.copyOf(code, elementValue.definition.asConstNumber());
+ } else if (elementValue.isConstString()) {
+ copy = ConstString.copyOf(code, elementValue.definition.asConstString());
+ } else if (elementValue.isConstClass()) {
+ copy = ConstClass.copyOf(code, elementValue.definition.asConstClass());
+ } else {
+ assert false;
+ return elementValue;
+ }
+ copy.setBlock(instructionIterator.getBlock());
+ copy.setPosition(newArrayEmpty.getPosition());
+ instructionIterator.add(copy);
+ addToRemove(elementValue.definition);
+ return copy.outValue();
+ }
+
+ private void addToRemove(Instruction instruction) {
+ if (toRemove == NOTHING) {
+ toRemove = SetUtils.newIdentityHashSet();
+ }
+ toRemove.add(instruction);
+ }
+
private void addArrayPut(
IRCode code,
BasicBlockInstructionListIterator instructionIterator,
diff --git a/src/test/java/com/android/tools/r8/compatproguard/GetMembersTest.java b/src/test/java/com/android/tools/r8/compatproguard/GetMembersTest.java
index 89af3cd..3449aa1 100644
--- a/src/test/java/com/android/tools/r8/compatproguard/GetMembersTest.java
+++ b/src/test/java/com/android/tools/r8/compatproguard/GetMembersTest.java
@@ -85,9 +85,9 @@
assertTrue(code.instructions[5] instanceof DexInvokeVirtual);
assertTrue(code.instructions[6] instanceof DexReturnVoid);
} else {
- assertTrue(code.instructions[0] instanceof DexConstClass);
- assertTrue(code.instructions[1] instanceof DexConst4);
- assertTrue(code.instructions[2] instanceof DexNewArray);
+ assertTrue(code.instructions[0] instanceof DexConst4);
+ assertTrue(code.instructions[1] instanceof DexNewArray);
+ assertTrue(code.instructions[2] instanceof DexConstClass);
assertTrue(code.instructions[3] instanceof DexConst4);
assertTrue(code.instructions[4] instanceof DexAputObject);
assertTrue(code.instructions[5] instanceof DexConstClass);
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/ConstClassArrayTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/ConstClassArrayTest.java
new file mode 100644
index 0000000..180c412
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/rewrite/arrays/ConstClassArrayTest.java
@@ -0,0 +1,163 @@
+// Copyright (c) 2023, 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.rewrite.arrays;
+
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.CompilationMode;
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.StringUtils;
+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 java.util.Iterator;
+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 ConstClassArrayTest extends TestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameter(1)
+ public CompilationMode compilationMode;
+
+ @Parameters(name = "{0}, mode = {1}")
+ public static Iterable<?> data() {
+ return buildParameters(
+ getTestParameters().withDefaultCfRuntime().withDexRuntimesAndAllApiLevels().build(),
+ CompilationMode.values());
+ }
+
+ private static final String EXPECTED_OUTPUT =
+ StringUtils.lines("[A, B, C, D, E]", "[E, D, C, B, A]");
+
+ public boolean canUseFilledNewArrayOfNonStringObjects(TestParameters parameters) {
+ return parameters.isDexRuntime()
+ && parameters.getApiLevel().isGreaterThanOrEqualTo(AndroidApiLevel.N);
+ }
+
+ private enum State {
+ EXPECTING_CONSTCLASS,
+ EXPECTING_APUTOBJECT
+ }
+
+ private void inspect(MethodSubject method, boolean insideCatchHandler) {
+ boolean expectingFilledNewArray =
+ canUseFilledNewArrayOfNonStringObjects(parameters) && !insideCatchHandler;
+ assertEquals(
+ expectingFilledNewArray ? 0 : 5,
+ method.streamInstructions().filter(InstructionSubject::isArrayPut).count());
+ assertEquals(
+ expectingFilledNewArray ? 1 : 0,
+ method.streamInstructions().filter(InstructionSubject::isFilledNewArray).count());
+ if (!expectingFilledNewArray) {
+ // Test that const-class and aput instructions are interleaved by the lowering of
+ // filled-new-array.
+ int aputCount = 0;
+ State state = State.EXPECTING_CONSTCLASS;
+ Iterator<InstructionSubject> iterator = method.iterateInstructions();
+ while (iterator.hasNext()) {
+ InstructionSubject instruction = iterator.next();
+ if (instruction.isConstClass()) {
+ assertEquals(State.EXPECTING_CONSTCLASS, state);
+ state = State.EXPECTING_APUTOBJECT;
+ } else if (instruction.isArrayPut()) {
+ assertEquals(State.EXPECTING_APUTOBJECT, state);
+ aputCount++;
+ state = State.EXPECTING_CONSTCLASS;
+ }
+ }
+ assertEquals(State.EXPECTING_CONSTCLASS, state);
+ assertEquals(5, aputCount);
+ }
+ }
+
+ private void inspect(CodeInspector inspector) {
+ inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m1"), false);
+ inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m2"), true);
+ }
+
+ @Test
+ public void testD8() throws Exception {
+ parameters.assumeDexRuntime();
+ testForD8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .setMinApi(parameters)
+ .run(parameters.getRuntime(), TestClass.class)
+ .inspect(this::inspect)
+ .assertSuccessWithOutput(EXPECTED_OUTPUT);
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(TestClass.class)
+ .setMinApi(parameters)
+ .enableInliningAnnotations()
+ .addDontObfuscate()
+ .run(parameters.getRuntime(), TestClass.class)
+ .inspect(this::inspect)
+ .assertSuccessWithOutput(EXPECTED_OUTPUT);
+ }
+
+ public static final class TestClass {
+
+ @NeverInline
+ public static void m1() {
+ Class<?>[] classArray = {A.class, B.class, C.class, D.class, E.class};
+ printArray(classArray);
+ }
+
+ @NeverInline
+ public static void m2() {
+ try {
+ Class<?>[] classArray = {E.class, D.class, C.class, B.class, A.class};
+ printArray(classArray);
+ } catch (Exception e) {
+ throw new RuntimeException();
+ }
+ }
+
+ @NeverInline
+ public static void printArray(Class<?>[] classArray) {
+ System.out.print("[");
+ for (Class<?> clazz : classArray) {
+ if (clazz != classArray[0]) {
+ System.out.print(", ");
+ }
+ String simpleName = clazz.getName();
+ if (simpleName.lastIndexOf("$") > 0) {
+ simpleName = simpleName.substring(simpleName.lastIndexOf("$") + 1);
+ }
+ System.out.print(simpleName);
+ }
+ System.out.println("]");
+ }
+
+ public static void main(String[] args) {
+ m1();
+ m2();
+ }
+ }
+
+ class A {}
+
+ class B {}
+
+ class C {}
+
+ class D {}
+
+ class E {}
+}
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/IntegerArrayTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/IntegerArrayTest.java
new file mode 100644
index 0000000..54abbcd
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/rewrite/arrays/IntegerArrayTest.java
@@ -0,0 +1,114 @@
+// Copyright (c) 2023, 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.rewrite.arrays;
+
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.CompilationMode;
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.utils.StringUtils;
+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 java.util.Arrays;
+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 IntegerArrayTest extends TestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameter(1)
+ public CompilationMode compilationMode;
+
+ @Parameters(name = "{0}, mode = {1}")
+ public static Iterable<?> data() {
+ return buildParameters(
+ getTestParameters().withDefaultCfRuntime().withDexRuntimesAndAllApiLevels().build(),
+ CompilationMode.values());
+ }
+
+ private static final String EXPECTED_OUTPUT =
+ StringUtils.lines(
+ "[-2147483648, -1, 0, 1, 2147483647]", "[-2147483647, -2, 0, 2, 2147483646]");
+
+ public boolean canUseFilledNewArrayOfInteger(TestParameters parameters) {
+ return parameters.isDexRuntime();
+ }
+
+ private void inspect(MethodSubject main) {
+ assertEquals(
+ canUseFilledNewArrayOfInteger(parameters) ? 0 : 5,
+ main.streamInstructions().filter(InstructionSubject::isArrayPut).count());
+ assertEquals(
+ canUseFilledNewArrayOfInteger(parameters) ? 1 : 0,
+ main.streamInstructions().filter(InstructionSubject::isFilledNewArray).count());
+ }
+
+ private void inspect(CodeInspector inspector) {
+ inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m1"));
+ inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m2"));
+ }
+
+ @Test
+ public void testD8() throws Exception {
+ parameters.assumeDexRuntime();
+ testForD8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .setMinApi(parameters)
+ .run(parameters.getRuntime(), TestClass.class)
+ .inspect(this::inspect)
+ .assertSuccessWithOutput(EXPECTED_OUTPUT);
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(TestClass.class)
+ .setMinApi(parameters)
+ .enableInliningAnnotations()
+ .addDontObfuscate()
+ .run(parameters.getRuntime(), TestClass.class)
+ .inspect(this::inspect)
+ .assertSuccessWithOutput(EXPECTED_OUTPUT);
+ }
+
+ public static final class TestClass {
+
+ @NeverInline
+ public static void m1() {
+ int[] array = {Integer.MIN_VALUE, -1, 0, 1, Integer.MAX_VALUE};
+ printArray(array);
+ }
+
+ @NeverInline
+ public static void m2() {
+ try {
+ int[] array = {Integer.MIN_VALUE + 1, -2, 0, 2, Integer.MAX_VALUE - 1};
+ printArray(array);
+ } catch (Exception e) {
+ throw new RuntimeException();
+ }
+ }
+
+ @NeverInline
+ public static void printArray(int[] array) {
+ System.out.println(Arrays.toString(array));
+ }
+
+ public static void main(String[] args) {
+ m1();
+ m2();
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/StringArrayTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/StringArrayTest.java
new file mode 100644
index 0000000..b2e70a7
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/rewrite/arrays/StringArrayTest.java
@@ -0,0 +1,138 @@
+// Copyright (c) 2023, 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.rewrite.arrays;
+
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.CompilationMode;
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.StringUtils;
+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 java.util.Arrays;
+import java.util.Iterator;
+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 StringArrayTest extends TestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameter(1)
+ public CompilationMode compilationMode;
+
+ @Parameters(name = "{0}, mode = {1}")
+ public static Iterable<?> data() {
+ return buildParameters(
+ getTestParameters().withDefaultCfRuntime().withDexRuntimesAndAllApiLevels().build(),
+ CompilationMode.values());
+ }
+
+ private static final String EXPECTED_OUTPUT =
+ StringUtils.lines("[A, B, C, D, E]", "[F, G, H, I, J]");
+
+ public boolean canUseFilledNewArrayOfStrings(TestParameters parameters) {
+ return parameters.isDexRuntime()
+ && parameters.getApiLevel().isGreaterThanOrEqualTo(AndroidApiLevel.K);
+ }
+
+ private enum State {
+ EXPECTING_CONSTSTRING,
+ EXPECTING_APUTOBJECT
+ }
+
+ private void inspect(MethodSubject method, boolean insideCatchHandler) {
+ boolean expectingFilledNewArray =
+ canUseFilledNewArrayOfStrings(parameters) && !insideCatchHandler;
+ assertEquals(
+ expectingFilledNewArray ? 0 : 5,
+ method.streamInstructions().filter(InstructionSubject::isArrayPut).count());
+ assertEquals(
+ expectingFilledNewArray ? 1 : 0,
+ method.streamInstructions().filter(InstructionSubject::isFilledNewArray).count());
+ if (!expectingFilledNewArray) {
+ // Test that const-string and aput instructions are interleaved by the lowering of
+ // filled-new-array.
+ int aputCount = 0;
+ State state = State.EXPECTING_CONSTSTRING;
+ Iterator<InstructionSubject> iterator = method.iterateInstructions();
+ while (iterator.hasNext()) {
+ InstructionSubject instruction = iterator.next();
+ if (instruction.isConstString()) {
+ assertEquals(State.EXPECTING_CONSTSTRING, state);
+ state = State.EXPECTING_APUTOBJECT;
+ } else if (instruction.isArrayPut()) {
+ assertEquals(State.EXPECTING_APUTOBJECT, state);
+ state = State.EXPECTING_CONSTSTRING;
+ aputCount++;
+ }
+ }
+ assertEquals(State.EXPECTING_CONSTSTRING, state);
+ assertEquals(5, aputCount);
+ }
+ }
+
+ private void inspect(CodeInspector inspector) {
+ inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m1"), false);
+ inspect(inspector.clazz(TestClass.class).uniqueMethodWithOriginalName("m2"), true);
+ }
+
+ @Test
+ public void testD8() throws Exception {
+ parameters.assumeDexRuntime();
+ testForD8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .setMinApi(parameters)
+ .run(parameters.getRuntime(), TestClass.class)
+ .inspect(this::inspect)
+ .assertSuccessWithOutput(EXPECTED_OUTPUT);
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(TestClass.class)
+ .setMinApi(parameters)
+ .enableInliningAnnotations()
+ .addDontObfuscate()
+ .run(parameters.getRuntime(), TestClass.class)
+ .inspect(this::inspect)
+ .assertSuccessWithOutput(EXPECTED_OUTPUT);
+ }
+
+ public static final class TestClass {
+
+ @NeverInline
+ public static void m1() {
+ String[] stringArray = {"A", "B", "C", "D", "E"};
+ System.out.println(Arrays.asList(stringArray));
+ }
+
+ @NeverInline
+ public static void m2() {
+ try {
+ String[] stringArray = {"F", "G", "H", "I", "J"};
+ System.out.println(Arrays.asList(stringArray));
+ } catch (Exception e) {
+ throw new RuntimeException();
+ }
+ }
+
+ public static void main(String[] args) {
+ m1();
+ m2();
+ }
+ }
+}
