Do not enable filled-new-array for pre-N minSdkVersion
ART 6.0.1 tests show an odd failure with them, so disable for pre-N.
Many apps now have N as their minSdkVersion, so this won't be a missed
opportunity for most (or at least this will be the case soon).
This still enables String[] for pre-N so as to not regress the status
quo.
Bug: b/246971330
Change-Id: I0a87f6dc8201b487bce0ff2af0abd35f32fd03a8
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
index a6da9b7..1d4e435 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
@@ -2220,6 +2220,7 @@
return;
}
InternalOptions.RewriteArrayOptions rewriteOptions = options.rewriteArrayOptions();
+ boolean canUseForStrings = rewriteOptions.canUseFilledNewArrayOfStrings();
boolean canUseForObjects = rewriteOptions.canUseFilledNewArrayOfObjects();
boolean canUseForArrays = rewriteOptions.canUseFilledNewArrayOfArrays();
@@ -2242,11 +2243,17 @@
if (size < 1 || size > rewriteOptions.maxFillArrayDataInputs) {
continue;
}
- if (!newArrayEmpty.type.isPrimitiveArrayType() && !canUseForObjects) {
- continue;
- }
- if (newArrayEmpty.type.getNumberOfLeadingSquareBrackets() > 1 && !canUseForArrays) {
- continue;
+ DexType arrayType = newArrayEmpty.type;
+ if (!arrayType.isPrimitiveArrayType()) {
+ if (arrayType == dexItemFactory.stringArrayType) {
+ if (!canUseForStrings) {
+ continue;
+ }
+ } else if (!canUseForObjects) {
+ continue;
+ } else if (!canUseForArrays && arrayType.getNumberOfLeadingSquareBrackets() > 1) {
+ continue;
+ }
}
Value[] values =
@@ -2258,8 +2265,8 @@
// filled-new-array is implemented only for int[] and Object[].
// For int[], using filled-new-array is usually smaller than filled-array-data.
// filled-new-array supports up to 5 registers before it's filled-new-array/range.
- if (!newArrayEmpty.type.isPrimitiveArrayType()
- || (newArrayEmpty.type == dexItemFactory.intArrayType && size <= 5)) {
+ if (!arrayType.isPrimitiveArrayType()
+ || (arrayType == dexItemFactory.intArrayType && size <= 5)) {
// Don't replace with filled-new-array if it requires more than 200 consecutive registers.
if (size > rewriteOptions.maxRangeInputs) {
continue;
@@ -2270,8 +2277,7 @@
// positioned).
Value invokeValue =
code.createValue(newArrayEmpty.getOutType(), newArrayEmpty.getLocalInfo());
- InvokeNewArray invoke =
- new InvokeNewArray(newArrayEmpty.type, invokeValue, Arrays.asList(values));
+ InvokeNewArray invoke = new InvokeNewArray(arrayType, invokeValue, Arrays.asList(values));
for (Value value : newArrayEmpty.inValues()) {
value.removeUser(newArrayEmpty);
}
@@ -2292,7 +2298,7 @@
// throwing instruction to the same block (the first one being NewArrayEmpty).
continue;
}
- int elementSize = newArrayEmpty.type.elementSizeForPrimitiveArrayType();
+ int elementSize = arrayType.elementSizeForPrimitiveArrayType();
short[] contents = computeArrayFilledData(values, size, elementSize);
if (contents == null) {
continue;
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index 5cb7a74..98d5a27 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -1403,11 +1403,19 @@
// Buggy code that accidentally call code that only works on primitives arrays.
//
// https://android.googlesource.com/platform/dalvik/+/ics-mr0/vm/mterp/out/InterpAsm-x86-atom.S#25106
- public boolean canUseFilledNewArrayOfObjects() {
+ public boolean canUseFilledNewArrayOfStrings() {
assert isGeneratingDex();
return hasFeaturePresentFrom(AndroidApiLevel.K);
}
+ // When adding support for emitting new-filled-array for non-String types, ART 6.0.1 had issues.
+ // https://ci.chromium.org/ui/p/r8/builders/ci/linux-android-6.0.1/6507/overview
+ // It somehow had a new-array-filled return null.
+ public boolean canUseFilledNewArrayOfObjects() {
+ assert isGeneratingDex();
+ return hasFeaturePresentFrom(AndroidApiLevel.N);
+ }
+
// Dalvik doesn't handle new-filled-array with arrays as values. It fails with:
// W(629880) VFY: [Ljava/lang/Integer; is not instance of Ljava/lang/Integer; (dalvikvm)
public boolean canUseFilledNewArrayOfArrays() {
diff --git a/src/test/java/com/android/tools/r8/rewrite/arrays/SimplifyArrayConstructionTest.java b/src/test/java/com/android/tools/r8/rewrite/arrays/SimplifyArrayConstructionTest.java
index aad69a0..939753f 100644
--- a/src/test/java/com/android/tools/r8/rewrite/arrays/SimplifyArrayConstructionTest.java
+++ b/src/test/java/com/android/tools/r8/rewrite/arrays/SimplifyArrayConstructionTest.java
@@ -63,11 +63,10 @@
"[1, null, 2]",
"[1, null, 2]",
"[1]",
- "[a, b]",
+ "[1, 2]",
"[1, 2, 3, 4, 5]",
"[1]",
"[a, 1, null, d, e, f]",
- "[a, b, c, d, e, f]",
"[1, null, 3, null, null, 6]",
"[1, 2, 3, 4, 5, 6]",
"[true, false]",
@@ -167,6 +166,7 @@
ClassSubject mainClass = inspector.clazz(Main.class);
assertTrue(mainClass.isPresent());
+ MethodSubject stringArrays = mainClass.uniqueMethodWithOriginalName("stringArrays");
MethodSubject referenceArraysNoCasts =
mainClass.uniqueMethodWithOriginalName("referenceArraysNoCasts");
MethodSubject referenceArraysWithSubclasses =
@@ -210,8 +210,14 @@
assertArrayTypes(reversedArray, DexFilledNewArray.class);
}
- // Cannot use filled-new-array of Object before K.
+ // Cannot use filled-new-array of String before K.
if (parameters.getApiLevel().isLessThan(AndroidApiLevel.K)) {
+ assertArrayTypes(stringArrays, DexNewArray.class);
+ } else {
+ assertArrayTypes(stringArrays, DexFilledNewArray.class);
+ }
+ // Cannot use filled-new-array of Object before L.
+ if (parameters.getApiLevel().isLessThan(AndroidApiLevel.N)) {
assertArrayTypes(referenceArraysNoCasts, DexNewArray.class);
assertArrayTypes(referenceArraysWithSubclasses, DexNewArray.class);
assertArrayTypes(phiFilledNewArray, DexNewArray.class);
@@ -290,6 +296,7 @@
static final String assumedNullField = null;
public static void main(String[] args) {
+ stringArrays();
referenceArraysNoCasts();
referenceArraysWithSubclasses();
interfaceArrayWithRawObject();
@@ -310,10 +317,14 @@
}
@NeverInline
- private static void referenceArraysNoCasts() {
+ private static void stringArrays() {
// Test exact class, no null.
String[] stringArr = {"a"};
System.out.println(Arrays.toString(stringArr));
+ }
+
+ @NeverInline
+ private static void referenceArraysNoCasts() {
// Tests that no type info is needed when array type is Object[].
Object[] objectArr = {"a", 1, null};
System.out.println(Arrays.toString(objectArr));
@@ -454,14 +465,14 @@
@NeverInline
private static void assumedValues() {
- String[] arr = {assumedNonNullField, assumedNullField};
+ Object[] arr = {assumedNonNullField, assumedNullField};
System.out.println(Arrays.toString(arr));
}
@NeverInline
private static void phiFilledNewArray() {
// The presence of ? should not affect use of filled-new-array.
- String[] phiArray = {"a", System.nanoTime() > 0 ? "b" : "c"};
+ Integer[] phiArray = {1, System.nanoTime() > 0 ? 2 : 3};
System.out.println(Arrays.toString(phiArray));
}
@@ -483,8 +494,6 @@
// 6 or more elements use /range variant.
Object[] objectArr = {"a", 1, null, "d", "e", "f"};
System.out.println(Arrays.toString(objectArr));
- String[] stringArr = {"a", "b", "c", "d", "e", "f"};
- System.out.println(Arrays.toString(stringArr));
Serializable[] interfaceArr = {
getSerializable(1), null, getSerializable(3), null, null, getSerializable(6)
};
