Enum unboxing: Enum arrays support
Bug: 147860220
Change-Id: Ib6236ae57c88e83fac9fede6298c14e636e42f15
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/type/ArrayTypeLatticeElement.java b/src/main/java/com/android/tools/r8/ir/analysis/type/ArrayTypeLatticeElement.java
index 39ebff8..64885a8 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/type/ArrayTypeLatticeElement.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/type/ArrayTypeLatticeElement.java
@@ -48,7 +48,7 @@
return factory.createArrayType(getNesting(), baseType);
}
- int getNesting() {
+ public int getNesting() {
int nesting = 1;
TypeLatticeElement member = getArrayMemberTypeAsMemberType();
while (member.isArrayType()) {
diff --git a/src/main/java/com/android/tools/r8/ir/code/ArrayAccess.java b/src/main/java/com/android/tools/r8/ir/code/ArrayAccess.java
new file mode 100644
index 0000000..14ac116
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/ir/code/ArrayAccess.java
@@ -0,0 +1,38 @@
+// Copyright (c) 2020, 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.code;
+
+import java.util.List;
+
+public abstract class ArrayAccess extends Instruction implements ImpreciseMemberTypeInstruction {
+
+ // Input values are ordered according to the stack order of the Java bytecodes.
+ private static final int ARRAY_INDEX = 0;
+ private static final int INDEX_INDEX = 1;
+
+ ArrayAccess(Value outValue, List<? extends Value> inValues) {
+ super(outValue, inValues);
+ }
+
+ public Value array() {
+ return inValues.get(ARRAY_INDEX);
+ }
+
+ public Value index() {
+ return inValues.get(INDEX_INDEX);
+ }
+
+ @Override
+ public boolean isArrayAccess() {
+ return true;
+ }
+
+ @Override
+ public ArrayAccess asArrayAccess() {
+ return this;
+ }
+
+ public abstract ArrayAccess withMemberType(MemberType newMemberType);
+}
diff --git a/src/main/java/com/android/tools/r8/ir/code/ArrayGet.java b/src/main/java/com/android/tools/r8/ir/code/ArrayGet.java
index 17dcc5e..b6eb113 100644
--- a/src/main/java/com/android/tools/r8/ir/code/ArrayGet.java
+++ b/src/main/java/com/android/tools/r8/ir/code/ArrayGet.java
@@ -30,7 +30,7 @@
import java.util.Arrays;
import java.util.Set;
-public class ArrayGet extends Instruction implements ImpreciseMemberTypeInstruction {
+public class ArrayGet extends ArrayAccess {
private MemberType type;
@@ -53,14 +53,6 @@
return outValue;
}
- public Value array() {
- return inValues.get(0);
- }
-
- public Value index() {
- return inValues.get(1);
- }
-
@Override
public MemberType getMemberType() {
return type;
@@ -281,4 +273,9 @@
public boolean instructionMayTriggerMethodInvocation(AppView<?> appView, DexType context) {
return false;
}
+
+ @Override
+ public ArrayAccess withMemberType(MemberType newMemberType) {
+ return new ArrayGet(newMemberType, outValue(), array(), index());
+ }
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/ArrayPut.java b/src/main/java/com/android/tools/r8/ir/code/ArrayPut.java
index 589710f..b1046d3 100644
--- a/src/main/java/com/android/tools/r8/ir/code/ArrayPut.java
+++ b/src/main/java/com/android/tools/r8/ir/code/ArrayPut.java
@@ -26,11 +26,9 @@
import com.android.tools.r8.ir.regalloc.RegisterAllocator;
import java.util.Arrays;
-public class ArrayPut extends Instruction implements ImpreciseMemberTypeInstruction {
+public class ArrayPut extends ArrayAccess {
// Input values are ordered according to the stack order of the Java bytecode astore.
- private static final int ARRAY_INDEX = 0;
- private static final int INDEX_INDEX = 1;
private static final int VALUE_INDEX = 2;
private MemberType type;
@@ -53,14 +51,6 @@
return visitor.visit(this);
}
- public Value array() {
- return inValues.get(ARRAY_INDEX);
- }
-
- public Value index() {
- return inValues.get(INDEX_INDEX);
- }
-
public Value value() {
return inValues.get(VALUE_INDEX);
}
@@ -275,4 +265,9 @@
public boolean instructionMayTriggerMethodInvocation(AppView<?> appView, DexType context) {
return false;
}
+
+ @Override
+ public ArrayAccess withMemberType(MemberType newMemberType) {
+ return new ArrayPut(newMemberType, array(), index(), value());
+ }
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/ImpreciseMemberTypeInstruction.java b/src/main/java/com/android/tools/r8/ir/code/ImpreciseMemberTypeInstruction.java
index 2b5bc02..ff21fbd 100644
--- a/src/main/java/com/android/tools/r8/ir/code/ImpreciseMemberTypeInstruction.java
+++ b/src/main/java/com/android/tools/r8/ir/code/ImpreciseMemberTypeInstruction.java
@@ -6,6 +6,7 @@
import com.android.tools.r8.ir.conversion.TypeConstraintResolver;
public interface ImpreciseMemberTypeInstruction {
+
MemberType getMemberType();
void constrainType(TypeConstraintResolver constraintResolver);
diff --git a/src/main/java/com/android/tools/r8/ir/code/Instruction.java b/src/main/java/com/android/tools/r8/ir/code/Instruction.java
index a56f0a2..3f85e8a 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Instruction.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Instruction.java
@@ -661,6 +661,14 @@
return this;
}
+ public boolean isArrayAccess() {
+ return false;
+ }
+
+ public ArrayAccess asArrayAccess() {
+ return null;
+ }
+
public boolean isArrayGet() {
return false;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxer.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxer.java
index f440fd3..fb02388 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxer.java
@@ -24,7 +24,9 @@
import com.android.tools.r8.graph.RewrittenPrototypeDescription.ArgumentInfoCollection;
import com.android.tools.r8.graph.RewrittenPrototypeDescription.RewrittenTypeInfo;
import com.android.tools.r8.ir.analysis.type.ArrayTypeLatticeElement;
+import com.android.tools.r8.ir.analysis.type.ClassTypeLatticeElement;
import com.android.tools.r8.ir.analysis.type.TypeLatticeElement;
+import com.android.tools.r8.ir.code.ArrayPut;
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.ConstClass;
import com.android.tools.r8.ir.code.FieldInstruction;
@@ -32,6 +34,7 @@
import com.android.tools.r8.ir.code.If;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InvokeMethod;
+import com.android.tools.r8.ir.code.MemberType;
import com.android.tools.r8.ir.code.Phi;
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.conversion.CodeOptimization;
@@ -159,6 +162,22 @@
markEnumAsUnboxable(
Reason.CONST_CLASS, appView.definitionForProgramType(constClass.getValue()));
}
+ } else if (instruction.isInvokeStatic()) {
+ // TODO(b/150370354): Since we temporary allow enum unboxing on enums with values and
+ // valueOf static methods only if such methods are unused, such methods cannot be
+ // called. the long term solution is to simply move called methods to a companion class,
+ // as any static helper method, and remove these checks.
+ DexMethod invokedMethod = instruction.asInvokeStatic().getInvokedMethod();
+ DexProgramClass enumClass = getEnumUnboxingCandidateOrNull(invokedMethod.holder);
+ if (enumClass != null) {
+ if (factory.enumMethods.isValueOfMethod(invokedMethod, enumClass)) {
+ markEnumAsUnboxable(Reason.VALUE_OF_INVOKE, enumClass);
+ } else if (factory.enumMethods.isValuesMethod(invokedMethod, enumClass)) {
+ markEnumAsUnboxable(Reason.VALUES_INVOKE, enumClass);
+ } else {
+ assert false; // We do not allow any other static call in unboxing candidates.
+ }
+ }
}
}
for (Phi phi : block.getPhis()) {
@@ -262,6 +281,7 @@
if (optimizationInfo.isMutableFieldOptimizationInfo()) {
optimizationInfo
.asMutableFieldOptimizationInfo()
+ .fixupClassTypeReferences(appView.graphLense()::lookupType, appView)
.fixupAbstractValue(appView, appView.graphLense());
} else {
assert optimizationInfo.isDefaultFieldOptimizationInfo();
@@ -274,6 +294,7 @@
if (optimizationInfo.isUpdatableMethodOptimizationInfo()) {
optimizationInfo
.asUpdatableMethodOptimizationInfo()
+ .fixupClassTypeReferences(appView.graphLense()::lookupType, appView)
.fixupAbstractReturnValue(appView, appView.graphLense())
.fixupInstanceInitializerInfo(appView, appView.graphLense());
} else {
@@ -350,7 +371,7 @@
int offset = BooleanUtils.intValue(!encodedSingleTarget.isStatic());
for (int i = 0; i < singleTarget.proto.parameters.size(); i++) {
if (invokeMethod.inValues().get(offset + i) == enumValue) {
- if (singleTarget.proto.parameters.values[i] != enumClass.type) {
+ if (singleTarget.proto.parameters.values[i].toBaseType(factory) != enumClass.type) {
return Reason.GENERIC_INVOKE;
}
}
@@ -426,6 +447,45 @@
return Reason.INVALID_IF_TYPES;
}
+ if (instruction.isArrayLength()) {
+ // MyEnum[] array = ...; array.length; is valid.
+ return Reason.ELIGIBLE;
+ }
+
+ if (instruction.isArrayGet()) {
+ // MyEnum[] array = ...; array[0]; is valid.
+ return Reason.ELIGIBLE;
+ }
+
+ if (instruction.isArrayPut()) {
+ // MyEnum[] array; array[0] = MyEnum.A; is valid.
+ // MyEnum[][] array2d; MyEnum[] array; array2d[0] = array; is valid.
+ // MyEnum[]^N array; MyEnum[]^(N-1) element; array[0] = element; is valid.
+ // We need to prove that the value to put in and the array have correct types.
+ ArrayPut arrayPut = instruction.asArrayPut();
+ assert arrayPut.getMemberType() == MemberType.OBJECT;
+ TypeLatticeElement arrayType = arrayPut.array().getTypeLattice();
+ assert arrayType.isArrayType();
+ assert arrayType.asArrayTypeLatticeElement().getArrayBaseTypeLattice().isClassType();
+ ClassTypeLatticeElement arrayBaseType =
+ arrayType
+ .asArrayTypeLatticeElement()
+ .getArrayBaseTypeLattice()
+ .asClassTypeLatticeElement();
+ TypeLatticeElement valueBaseType = arrayPut.value().getTypeLattice();
+ if (valueBaseType.isArrayType()) {
+ assert valueBaseType.asArrayTypeLatticeElement().getArrayBaseTypeLattice().isClassType();
+ assert valueBaseType.asArrayTypeLatticeElement().getNesting()
+ == arrayType.asArrayTypeLatticeElement().getNesting() - 1;
+ valueBaseType = valueBaseType.asArrayTypeLatticeElement().getArrayBaseTypeLattice();
+ }
+ if (arrayBaseType.equalUpToNullability(valueBaseType)
+ && arrayBaseType.getClassType() == enumClass.type) {
+ return Reason.ELIGIBLE;
+ }
+ return Reason.INVALID_ARRAY_PUT;
+ }
+
if (instruction.isAssume()) {
Value outValue = instruction.outValue();
return validateEnumUsages(code, outValue, enumClass);
@@ -527,6 +587,7 @@
UNSUPPORTED_LIBRARY_CALL,
MISSING_INFO_MAP,
INVALID_FIELD_PUT,
+ INVALID_ARRAY_PUT,
FIELD_PUT_ON_ENUM,
TYPE_MISSMATCH_FIELD_PUT,
INVALID_IF_TYPES,
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingRewriter.java
index cbd3824..1514d99 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingRewriter.java
@@ -22,15 +22,18 @@
import com.android.tools.r8.graph.EnumValueInfoMapCollection.EnumValueInfoMap;
import com.android.tools.r8.graph.MethodAccessFlags;
import com.android.tools.r8.graph.ParameterAnnotationsList;
+import com.android.tools.r8.ir.analysis.type.ArrayTypeLatticeElement;
import com.android.tools.r8.ir.analysis.type.DestructivePhiTypeUpdater;
import com.android.tools.r8.ir.analysis.type.PrimitiveTypeLatticeElement;
import com.android.tools.r8.ir.analysis.type.TypeLatticeElement;
+import com.android.tools.r8.ir.code.ArrayAccess;
import com.android.tools.r8.ir.code.ConstNumber;
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.InvokeMethodWithReceiver;
import com.android.tools.r8.ir.code.InvokeStatic;
+import com.android.tools.r8.ir.code.MemberType;
import com.android.tools.r8.ir.code.NumericType;
import com.android.tools.r8.ir.code.Phi;
import com.android.tools.r8.ir.code.StaticGet;
@@ -129,6 +132,14 @@
affectedPhis.addAll(staticGet.outValue().uniquePhiUsers());
}
}
+ // Rewrite array accesses from MyEnum[] (OBJECT) to int[] (INT).
+ if (instruction.isArrayAccess()) {
+ ArrayAccess arrayAccess = instruction.asArrayAccess();
+ if (shouldRewriteArrayAccess(arrayAccess)) {
+ instruction = arrayAccess.withMemberType(MemberType.INT);
+ iterator.replaceCurrentInstruction(instruction);
+ }
+ }
assert validateEnumToUnboxRemoved(instruction);
}
if (!affectedPhis.isEmpty()) {
@@ -137,7 +148,23 @@
assert code.isConsistentSSABeforeTypesAreCorrect();
}
+ private boolean shouldRewriteArrayAccess(ArrayAccess arrayAccess) {
+ ArrayTypeLatticeElement arrayType =
+ arrayAccess.array().getTypeLattice().asArrayTypeLatticeElement();
+ return arrayAccess.getMemberType() == MemberType.OBJECT
+ && arrayType.getNesting() == 1
+ && arrayType.getArrayBaseTypeLattice().isInt();
+ }
+
private boolean validateEnumToUnboxRemoved(Instruction instruction) {
+ if (instruction.isArrayAccess()) {
+ ArrayAccess arrayAccess = instruction.asArrayAccess();
+ ArrayTypeLatticeElement arrayType =
+ arrayAccess.array().getTypeLattice().asArrayTypeLatticeElement();
+ assert arrayAccess.getMemberType() != MemberType.OBJECT
+ || arrayType.getNesting() > 1
+ || arrayType.getArrayBaseTypeLattice().isReference();
+ }
if (instruction.outValue() == null) {
return true;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/MutableFieldOptimizationInfo.java b/src/main/java/com/android/tools/r8/ir/optimize/info/MutableFieldOptimizationInfo.java
index a4917f2..f7a4b57 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/MutableFieldOptimizationInfo.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/MutableFieldOptimizationInfo.java
@@ -34,7 +34,7 @@
private ClassTypeLatticeElement dynamicLowerBoundType = null;
private TypeLatticeElement dynamicUpperBoundType = null;
- public void fixupClassTypeReferences(
+ public MutableFieldOptimizationInfo fixupClassTypeReferences(
Function<DexType, DexType> mapping, AppView<? extends AppInfoWithSubtyping> appView) {
if (dynamicUpperBoundType != null) {
dynamicUpperBoundType = dynamicUpperBoundType.fixupClassTypeReferences(mapping, appView);
@@ -50,6 +50,7 @@
this.dynamicUpperBoundType = null;
}
}
+ return this;
}
@Override
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/UpdatableMethodOptimizationInfo.java b/src/main/java/com/android/tools/r8/ir/optimize/info/UpdatableMethodOptimizationInfo.java
index 366b4d2..f50a3d1 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/UpdatableMethodOptimizationInfo.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/UpdatableMethodOptimizationInfo.java
@@ -142,7 +142,7 @@
nonNullParamOnNormalExits = template.nonNullParamOnNormalExits;
}
- public void fixupClassTypeReferences(
+ public UpdatableMethodOptimizationInfo fixupClassTypeReferences(
Function<DexType, DexType> mapping, AppView<? extends AppInfoWithSubtyping> appView) {
if (returnsObjectWithUpperBoundType != null) {
returnsObjectWithUpperBoundType =
@@ -160,6 +160,7 @@
this.returnsObjectWithLowerBoundType = DefaultMethodOptimizationInfo.UNKNOWN_CLASS_TYPE;
}
}
+ return this;
}
public UpdatableMethodOptimizationInfo fixupAbstractReturnValue(
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingArrayTest.java b/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingArrayTest.java
index 7d6f2ac..c97b7a7 100644
--- a/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingArrayTest.java
+++ b/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingArrayTest.java
@@ -18,7 +18,7 @@
@RunWith(Parameterized.class)
public class EnumUnboxingArrayTest extends EnumUnboxingTestBase {
- private static final Class<?>[] FAILURES = {
+ private static final Class<?>[] SUCCESSES = {
EnumVarArgs.class,
EnumArrayReadWriteNoEscape.class,
EnumArrayReadWrite.class,
@@ -42,11 +42,11 @@
}
@Test
- public void testEnumUnboxingFailure() throws Exception {
+ public void testEnumUnboxing() throws Exception {
R8TestCompileResult compile =
testForR8(parameters.getBackend())
.addInnerClasses(EnumUnboxingArrayTest.class)
- .addKeepMainRules(FAILURES)
+ .addKeepMainRules(SUCCESSES)
.enableInliningAnnotations()
.enableNeverClassInliningAnnotations()
.addKeepRules(enumKeepRules.getKeepRule())
@@ -54,14 +54,14 @@
.allowDiagnosticInfoMessages()
.setMinApi(parameters.getApiLevel())
.compile();
- for (Class<?> failure : FAILURES) {
+ for (Class<?> success : SUCCESSES) {
R8TestRunResult run =
compile
.inspectDiagnosticMessages(
m ->
- assertEnumIsBoxed(
- failure.getDeclaredClasses()[0], failure.getSimpleName(), m))
- .run(parameters.getRuntime(), failure)
+ assertEnumIsUnboxed(
+ success.getDeclaredClasses()[0], success.getSimpleName(), m))
+ .run(parameters.getRuntime(), success)
.assertSuccess();
assertLines2By2Correct(run.getStdOut());
}
@@ -96,8 +96,8 @@
myEnums[1] = MyEnum.C;
System.out.println(myEnums[1].ordinal());
System.out.println(2);
- System.out.println(myEnums[0]);
- System.out.println("null");
+ System.out.println(myEnums[0] == null);
+ System.out.println("true");
myEnums[0] = MyEnum.B;
System.out.println(myEnums.length);
System.out.println(2);
@@ -117,8 +117,8 @@
MyEnum[] myEnums = getArray();
System.out.println(myEnums[1].ordinal());
System.out.println(2);
- System.out.println(myEnums[0]);
- System.out.println("null");
+ System.out.println(myEnums[0] == null);
+ System.out.println("true");
myEnums[0] = MyEnum.B;
System.out.println(sum(myEnums));
System.out.println(2);
@@ -152,6 +152,8 @@
System.out.println(2);
System.out.println(myEnums[0][0].ordinal());
System.out.println(1);
+ System.out.println(myEnums[0].length);
+ System.out.println(2);
}
@NeverInline
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/SwitchEnumUnboxingTest.java b/src/test/java/com/android/tools/r8/enumunboxing/SwitchEnumUnboxingTest.java
index b7c7005..3ccdd37 100644
--- a/src/test/java/com/android/tools/r8/enumunboxing/SwitchEnumUnboxingTest.java
+++ b/src/test/java/com/android/tools/r8/enumunboxing/SwitchEnumUnboxingTest.java
@@ -51,7 +51,9 @@
.compile()
.inspectDiagnosticMessages(
m -> {
- if (enumValueOptimization) {
+ // TODO(b/150370354): We need to allow static helper method to re-enable unboxing
+ // with switches.
+ if (enumValueOptimization && enumKeepRules.getKeepRule().equals("")) {
assertEnumIsUnboxed(ENUM_CLASS, classToTest.getSimpleName(), m);
} else {
assertEnumIsBoxed(ENUM_CLASS, classToTest.getSimpleName(), m);