Merging enum with subtypes
Change-Id: Iacfe18165347007b9f94dcbeba6ea2c21c3f29b0
diff --git a/src/main/java/com/android/tools/r8/graph/AppView.java b/src/main/java/com/android/tools/r8/graph/AppView.java
index 5ebfb20..7a7b8d6 100644
--- a/src/main/java/com/android/tools/r8/graph/AppView.java
+++ b/src/main/java/com/android/tools/r8/graph/AppView.java
@@ -771,7 +771,7 @@
}
public boolean validateUnboxedEnumsHaveBeenPruned() {
- for (DexType unboxedEnum : unboxedEnums.getUnboxedEnums()) {
+ for (DexType unboxedEnum : unboxedEnums.computeAllUnboxedEnums()) {
assert appInfo.definitionForWithoutExistenceAssert(unboxedEnum) == null
: "Enum " + unboxedEnum + " has been unboxed but is still in the program.";
assert appInfo().withLiveness().wasPruned(unboxedEnum)
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 4d6f31a..cfa8347 100644
--- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
+++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -1992,8 +1992,17 @@
}
public boolean isEnumField(DexEncodedField staticField, DexType enumType) {
+ return isEnumField(staticField, enumType, ImmutableSet.of());
+ }
+
+ // In some case, the enum field may be respecialized to an enum subtype. In this case, one
+ // can pass the encoded field as well as the field with the super enum type for the checks.
+ public boolean isEnumField(
+ DexEncodedField staticField, DexType enumType, Set<DexType> subtypes) {
assert staticField.isStatic();
- return staticField.getType() == enumType && staticField.isEnum() && staticField.isFinal();
+ return (staticField.getType() == enumType || subtypes.contains(staticField.getType()))
+ && staticField.isEnum()
+ && staticField.isFinal();
}
public boolean isValuesFieldCandidate(DexEncodedField staticField, DexType enumType) {
diff --git a/src/main/java/com/android/tools/r8/graph/ObjectAllocationInfoCollectionImpl.java b/src/main/java/com/android/tools/r8/graph/ObjectAllocationInfoCollectionImpl.java
index 45936b7..97dfaf5 100644
--- a/src/main/java/com/android/tools/r8/graph/ObjectAllocationInfoCollectionImpl.java
+++ b/src/main/java/com/android/tools/r8/graph/ObjectAllocationInfoCollectionImpl.java
@@ -482,7 +482,7 @@
(clazz, allocationSitesForClass) -> {
DexType type = lens.lookupType(clazz.type);
if (type.isPrimitiveType()) {
- assert !objectAllocationInfos.hasInstantiatedStrictSubtype(clazz);
+ assert clazz.isEnum();
return;
}
DexProgramClass rewrittenClass = asProgramClassOrNull(definitions.definitionFor(type));
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/StaticFieldValues.java b/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/StaticFieldValues.java
index e57052f..0f40f03 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/StaticFieldValues.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/StaticFieldValues.java
@@ -4,12 +4,15 @@
package com.android.tools.r8.ir.analysis.fieldvalueanalysis;
+import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexProgramClass;
+import com.android.tools.r8.graph.lens.GraphLens;
import com.android.tools.r8.ir.analysis.value.AbstractValue;
import com.android.tools.r8.ir.analysis.value.objectstate.ObjectState;
+import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.google.common.collect.ImmutableMap;
public abstract class StaticFieldValues {
@@ -47,6 +50,16 @@
return new Builder();
}
+ public EnumStaticFieldValues rewrittenWithLens(
+ AppView<AppInfoWithLiveness> appView, GraphLens lens, GraphLens codeLens) {
+ ImmutableMap.Builder<DexField, ObjectState> builder = ImmutableMap.builder();
+ enumAbstractValues.forEach(
+ (field, state) ->
+ builder.put(
+ lens.lookupField(field), state.rewrittenWithLens(appView, lens, codeLens)));
+ return new EnumStaticFieldValues(builder.build());
+ }
+
public static class Builder extends StaticFieldValues.Builder {
private final ImmutableMap.Builder<DexField, ObjectState> enumObjectStateBuilder =
ImmutableMap.builder();
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumDataMap.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumDataMap.java
index 4a91dc0..aacdd3a 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumDataMap.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumDataMap.java
@@ -14,6 +14,7 @@
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
import it.unimi.dsi.fastutil.ints.Int2ReferenceMap;
import java.util.ArrayList;
import java.util.List;
@@ -21,13 +22,20 @@
public class EnumDataMap {
private final ImmutableMap<DexType, EnumData> map;
+ private final ImmutableMap<DexType, DexType> subEnumToSuperEnumMap;
public static EnumDataMap empty() {
- return new EnumDataMap(ImmutableMap.of());
+ return new EnumDataMap(ImmutableMap.of(), ImmutableMap.of());
}
- public EnumDataMap(ImmutableMap<DexType, EnumData> map) {
+ public EnumDataMap(
+ ImmutableMap<DexType, EnumData> map, ImmutableMap<DexType, DexType> subEnumToSuperEnumMap) {
this.map = map;
+ this.subEnumToSuperEnumMap = subEnumToSuperEnumMap;
+ }
+
+ public boolean hasAnyEnumsWithSubtypes() {
+ return !subEnumToSuperEnumMap.isEmpty();
}
public void checkEnumsUnboxed(AppView<AppInfoWithLiveness> appView) {
@@ -47,47 +55,66 @@
}
}
- public boolean isUnboxedEnum(DexProgramClass clazz) {
- return isUnboxedEnum(clazz.getType());
- }
-
- public boolean isUnboxedEnum(DexType type) {
- return map.containsKey(type);
+ public DexType representativeType(DexType type) {
+ return subEnumToSuperEnumMap.getOrDefault(type, type);
}
public boolean isEmpty() {
return map.isEmpty();
}
- public EnumData get(DexProgramClass enumClass) {
- EnumData enumData = map.get(enumClass.getType());
+ public boolean isSuperUnboxedEnum(DexType type) {
+ return map.containsKey(type);
+ }
+
+ public boolean isUnboxedEnum(DexProgramClass clazz) {
+ return isUnboxedEnum(clazz.getType());
+ }
+
+ public boolean isUnboxedEnum(DexType type) {
+ return map.containsKey(representativeType(type));
+ }
+
+ private EnumData get(DexType type) {
+ EnumData enumData = map.get(representativeType(type));
assert enumData != null;
return enumData;
}
- public Set<DexType> getUnboxedEnums() {
+ public EnumData get(DexProgramClass enumClass) {
+ return get(enumClass.getType());
+ }
+
+ public Set<DexType> getUnboxedSuperEnums() {
return map.keySet();
}
+ public Set<DexType> computeAllUnboxedEnums() {
+ Set<DexType> items = Sets.newIdentityHashSet();
+ items.addAll(map.keySet());
+ items.addAll(subEnumToSuperEnumMap.keySet());
+ return items;
+ }
+
public EnumInstanceFieldKnownData getInstanceFieldData(
DexType enumType, DexField enumInstanceField) {
- assert map.containsKey(enumType);
- return map.get(enumType).getInstanceFieldData(enumInstanceField);
+ assert isUnboxedEnum(enumType);
+ return get(enumType).getInstanceFieldData(enumInstanceField);
}
public boolean hasUnboxedValueFor(DexField enumStaticField) {
- return isUnboxedEnum(enumStaticField.holder)
- && map.get(enumStaticField.holder).hasUnboxedValueFor(enumStaticField);
+ return isUnboxedEnum(enumStaticField.getHolderType())
+ && get(enumStaticField.getHolderType()).hasUnboxedValueFor(enumStaticField);
}
public int getUnboxedValue(DexField enumStaticField) {
- assert map.containsKey(enumStaticField.holder);
- return map.get(enumStaticField.holder).getUnboxedValue(enumStaticField);
+ assert isUnboxedEnum(enumStaticField.getHolderType());
+ return get(enumStaticField.getHolderType()).getUnboxedValue(enumStaticField);
}
public int getValuesSize(DexType enumType) {
- assert map.containsKey(enumType);
- return map.get(enumType).getValuesSize();
+ assert isUnboxedEnum(enumType);
+ return get(enumType).getValuesSize();
}
public int getMaxValuesSize() {
@@ -101,8 +128,8 @@
}
public boolean matchesValuesField(DexField staticField) {
- assert map.containsKey(staticField.holder);
- return map.get(staticField.holder).matchesValuesField(staticField);
+ assert isUnboxedEnum(staticField.getHolderType());
+ return get(staticField.getHolderType()).matchesValuesField(staticField);
}
public static class EnumData {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxerImpl.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxerImpl.java
index c147322..ef10ff8 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxerImpl.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxerImpl.java
@@ -51,6 +51,7 @@
import com.android.tools.r8.ir.analysis.type.ArrayTypeElement;
import com.android.tools.r8.ir.analysis.type.ClassTypeElement;
import com.android.tools.r8.ir.analysis.type.DynamicType;
+import com.android.tools.r8.ir.analysis.type.ReferenceTypeElement;
import com.android.tools.r8.ir.analysis.type.TypeElement;
import com.android.tools.r8.ir.analysis.value.AbstractValue;
import com.android.tools.r8.ir.analysis.value.objectstate.EnumValuesObjectState;
@@ -116,9 +117,11 @@
import com.google.common.collect.HashMultiset;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
import it.unimi.dsi.fastutil.ints.Int2ReferenceArrayMap;
import it.unimi.dsi.fastutil.ints.Int2ReferenceMap;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceMaps;
import it.unimi.dsi.fastutil.objects.Object2IntMap;
import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap;
import java.util.ArrayList;
@@ -683,8 +686,8 @@
GraphLens previousLens = appView.graphLens();
ImmutableSet<DexType> enumsToUnbox = enumUnboxingCandidatesInfo.candidates();
- ImmutableSet<DexProgramClass> enumClassesToUnbox =
- enumUnboxingCandidatesInfo.candidateClasses();
+ ImmutableMap<DexProgramClass, Set<DexProgramClass>> enumClassesToUnbox =
+ enumUnboxingCandidatesInfo.candidateClassesWithSubclasses();
LongLivedProgramMethodSetBuilder<ProgramMethodSet> dependencies =
enumUnboxingCandidatesInfo.allMethodDependencies();
enumUnboxingCandidatesInfo.clear();
@@ -693,7 +696,7 @@
EnumUnboxingUtilityClasses utilityClasses =
EnumUnboxingUtilityClasses.builder(appView)
- .synthesizeEnumUnboxingUtilityClasses(enumClassesToUnbox, enumDataMap)
+ .synthesizeEnumUnboxingUtilityClasses(enumClassesToUnbox.keySet(), enumDataMap)
.build(converter, executorService);
// Fixup the application.
@@ -729,7 +732,8 @@
.merge(
methodsDependingOnLibraryModelisation
.rewrittenWithLens(appView)
- .removeAll(treeFixerResult.getPrunedItems().getRemovedMethods()));
+ .removeAll(treeFixerResult.getPrunedItems().getRemovedMethods()))
+ .addAll(treeFixerResult.getDispatchMethods(), appView.graphLens());
methodsDependingOnLibraryModelisation.clear();
updateOptimizationInfos(executorService, feedback, treeFixerResult, previousLens);
@@ -806,15 +810,18 @@
debugLogs.keySet().forEach(enumUnboxingCandidatesInfo::removeCandidate);
reportEnumsAnalysis();
}
- assert enumDataMap.getUnboxedEnums().size() == enumUnboxingCandidatesInfo.candidates().size();
+ assert enumDataMap.getUnboxedSuperEnums().size()
+ == enumUnboxingCandidatesInfo.candidates().size();
return enumDataMap;
}
private EnumDataMap analyzeEnumInstances() {
+ ImmutableMap.Builder<DexType, DexType> enumSubtypes = ImmutableMap.builder();
ImmutableMap.Builder<DexType, EnumData> builder = ImmutableMap.builder();
- enumUnboxingCandidatesInfo.forEachCandidateAndRequiredInstanceFieldData(
- (enumClass, instanceFields) -> {
- EnumData data = buildData(enumClass, instanceFields);
+ enumUnboxingCandidatesInfo.forEachCandidateInfo(
+ info -> {
+ DexProgramClass enumClass = info.getEnumClass();
+ EnumData data = buildData(enumClass, info.getRequiredInstanceFieldData());
if (data == null) {
// Reason is already reported at this point.
enumUnboxingCandidatesInfo.removeCandidate(enumClass);
@@ -822,10 +829,17 @@
}
if (!debugLogEnabled || !debugLogs.containsKey(enumClass.getType())) {
builder.put(enumClass.type, data);
+ if (data.valuesTypes != null) {
+ for (DexType value : data.valuesTypes.values()) {
+ if (value != enumClass.type) {
+ enumSubtypes.put(value, enumClass.type);
+ }
+ }
+ }
}
});
staticFieldValuesMap.clear();
- return new EnumDataMap(builder.build());
+ return new EnumDataMap(builder.build(), enumSubtypes.build());
}
private EnumData buildData(DexProgramClass enumClass, Set<DexField> instanceFields) {
@@ -854,13 +868,21 @@
return null;
}
+ enumStaticFieldValues =
+ enumStaticFieldValues.rewrittenWithLens(appView, appView.graphLens(), appView.codeLens());
+ Set<DexType> enumSubtypes = enumUnboxingCandidatesInfo.getSubtypes(enumClass.getType());
+
// Step 1: We iterate over the field to find direct enum instance information and the values
// fields.
for (DexEncodedField staticField : enumClass.staticFields()) {
- if (factory.enumMembers.isEnumField(staticField, enumClass.type)) {
+ // The field might be specialized while the data was recorded without the specialization.
+ if (factory.enumMembers.isEnumField(staticField, enumClass.type, enumSubtypes)) {
ObjectState enumState =
enumStaticFieldValues.getObjectStateForPossiblyPinnedField(staticField.getReference());
if (enumState == null) {
+ assert enumStaticFieldValues.getObjectStateForPossiblyPinnedField(
+ staticField.getReference().withType(enumClass.type, factory))
+ == null;
if (staticField.getOptimizationInfo().isDead()) {
// We don't care about unused field data.
continue;
@@ -957,7 +979,7 @@
return new EnumData(
instanceFieldsData,
- isEnumWithSubtypes ? valueTypes : null,
+ isEnumWithSubtypes ? valueTypes : Int2ReferenceMaps.emptyMap(),
unboxedValues.build(),
valuesField.build(),
valuesContents == null ? EnumData.INVALID_VALUES_SIZE : valuesContents.getEnumValuesSize());
@@ -1051,26 +1073,44 @@
}
private void analyzeInitializers() {
- enumUnboxingCandidatesInfo.forEachCandidate(
- enumClass -> {
- for (DexEncodedMethod directMethod : enumClass.directMethods()) {
- if (directMethod.isInstanceInitializer()) {
- if (directMethod
- .getOptimizationInfo()
- .getContextInsensitiveInstanceInitializerInfo()
- .mayHaveOtherSideEffectsThanInstanceFieldAssignments()) {
- if (markEnumAsUnboxable(Reason.INVALID_INIT, enumClass)) {
- break;
- }
- }
+ enumUnboxingCandidatesInfo.forEachCandidateInfo(
+ (info) -> {
+ DexProgramClass enumClass = info.getEnumClass();
+ if (!instanceInitializersAllowUnboxing(enumClass)) {
+ if (markEnumAsUnboxable(Reason.INVALID_INIT, enumClass)) {
+ return;
}
}
if (enumClass.classInitializationMayHaveSideEffects(appView)) {
- markEnumAsUnboxable(Reason.INVALID_CLINIT, enumClass);
+ if (markEnumAsUnboxable(Reason.INVALID_CLINIT, enumClass)) {
+ return;
+ }
+ }
+ for (DexProgramClass subclass : info.getSubclasses()) {
+ if (!instanceInitializersAllowUnboxing(subclass)) {
+ if (markEnumAsUnboxable(Reason.INVALID_SUBTYPE_INIT, enumClass)) {
+ return;
+ }
+ }
+ if (subclass.hasClassInitializer()) {
+ if (markEnumAsUnboxable(Reason.SUBTYPE_CLINIT, enumClass)) {
+ return;
+ }
+ }
}
});
}
+ private boolean instanceInitializersAllowUnboxing(DexProgramClass clazz) {
+ return !Iterables.any(
+ clazz.programInstanceInitializers(),
+ instanceInitializer ->
+ instanceInitializer
+ .getOptimizationInfo()
+ .getContextInsensitiveInstanceInitializerInfo()
+ .mayHaveOtherSideEffectsThanInstanceFieldAssignments());
+ }
+
private Reason instructionAllowEnumUnboxing(
Instruction instruction, IRCode code, DexProgramClass enumClass, Value enumValue) {
ProgramMethod context = code.context();
@@ -1142,6 +1182,28 @@
return Reason.ELIGIBLE;
}
+ private ReferenceTypeElement getValueBaseType(Value value, TypeElement arrayType) {
+ TypeElement valueBaseType = value.getType();
+ if (valueBaseType.isArrayType()) {
+ assert valueBaseType.asArrayType().getBaseType().isClassType();
+ assert valueBaseType.asArrayType().getNesting() == arrayType.asArrayType().getNesting() - 1;
+ valueBaseType = valueBaseType.asArrayType().getBaseType();
+ }
+ assert valueBaseType.isClassType() || valueBaseType.isNullType();
+ return valueBaseType.asReferenceType();
+ }
+
+ private boolean areCompatibleArrayTypes(
+ ClassTypeElement arrayBaseType, ReferenceTypeElement valueBaseType) {
+ assert valueBaseType.isClassType() || valueBaseType.isNullType();
+ if (valueBaseType.isNullType()) {
+ // TODO(b/271385332): Allow nulls in enum arrays to be unboxed.
+ return false;
+ }
+ return enumUnboxingCandidatesInfo.isAssignableTo(
+ valueBaseType.asClassType().getClassType(), arrayBaseType.getClassType());
+ }
+
private Reason analyzeArrayPutUser(
ArrayPut arrayPut,
IRCode code,
@@ -1157,14 +1219,8 @@
assert arrayType.isArrayType();
assert arrayType.asArrayType().getBaseType().isClassType();
ClassTypeElement arrayBaseType = arrayType.asArrayType().getBaseType().asClassType();
- TypeElement valueBaseType = arrayPut.value().getType();
- if (valueBaseType.isArrayType()) {
- assert valueBaseType.asArrayType().getBaseType().isClassType();
- assert valueBaseType.asArrayType().getNesting() == arrayType.asArrayType().getNesting() - 1;
- valueBaseType = valueBaseType.asArrayType().getBaseType();
- }
- if (arrayBaseType.equalUpToNullability(valueBaseType)
- && arrayBaseType.getClassType() == enumClass.type) {
+ ReferenceTypeElement valueBaseType = getValueBaseType(arrayPut.value(), arrayType);
+ if (areCompatibleArrayTypes(arrayBaseType, valueBaseType)) {
return Reason.ELIGIBLE;
}
return Reason.INVALID_ARRAY_PUT;
@@ -1191,13 +1247,8 @@
}
for (Value value : invokeNewArray.inValues()) {
- TypeElement valueBaseType = value.getType();
- if (valueBaseType.isArrayType()) {
- assert valueBaseType.asArrayType().getBaseType().isClassType();
- assert valueBaseType.asArrayType().getNesting() == arrayType.asArrayType().getNesting() - 1;
- valueBaseType = valueBaseType.asArrayType().getBaseType();
- }
- if (!arrayBaseType.equalUpToNullability(valueBaseType)) {
+ ReferenceTypeElement valueBaseType = getValueBaseType(value, arrayType);
+ if (!areCompatibleArrayTypes(arrayBaseType, valueBaseType)) {
return Reason.INVALID_INVOKE_NEW_ARRAY;
}
}
@@ -1305,7 +1356,9 @@
if (targetHolder.isEnum() && singleTarget.getDefinition().isInstanceInitializer()) {
// The enum instance initializer is only allowed to be called from an initializer of the
// enum itself.
- if (code.context().getHolder() != targetHolder || !code.method().isInitializer()) {
+ if (getEnumUnboxingCandidateOrNull(code.context().getHolder().getType())
+ != getEnumUnboxingCandidateOrNull(targetHolder.getType())
+ || !context.getDefinition().isInitializer()) {
return Reason.INVALID_INIT;
}
if (code.method().isInstanceInitializer() && !invoke.getFirstArgument().isThis()) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingCandidateAnalysis.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingCandidateAnalysis.java
index abc0fde..8b82922 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingCandidateAnalysis.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingCandidateAnalysis.java
@@ -72,11 +72,12 @@
}
private void setEnumSubclassesOnCandidates() {
- enumToUnboxCandidates.forEachCandidate(
- candidate ->
- enumToUnboxCandidates.setEnumSubclasses(
- candidate.getType(),
- enumSubclasses.getOrDefault(candidate.getType(), ImmutableSet.of())));
+ enumToUnboxCandidates.forEachCandidateInfo(
+ info -> {
+ DexType type = info.getEnumClass().getType();
+ enumToUnboxCandidates.setEnumSubclasses(
+ type, enumSubclasses.getOrDefault(type, ImmutableSet.of()));
+ });
}
private void removeIneligibleCandidates() {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingCandidateInfoCollection.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingCandidateInfoCollection.java
index bfeb83b..3c2fc90 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingCandidateInfoCollection.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingCandidateInfoCollection.java
@@ -5,6 +5,7 @@
package com.android.tools.r8.ir.optimize.enums;
import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexProgramClass;
@@ -12,20 +13,23 @@
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.graph.lens.GraphLens;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
+import com.android.tools.r8.utils.SetUtils;
import com.android.tools.r8.utils.collections.LongLivedProgramMethodSetBuilder;
import com.android.tools.r8.utils.collections.ProgramMethodSet;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
+import java.util.IdentityHashMap;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
-import java.util.function.BiConsumer;
import java.util.function.Consumer;
public class EnumUnboxingCandidateInfoCollection {
private final Map<DexType, EnumUnboxingCandidateInfo> enumTypeToInfo = new ConcurrentHashMap<>();
+ private final Map<DexType, DexType> subEnumToSuperEnumMap = new IdentityHashMap<>();
private final Set<DexMethod> prunedMethods = Sets.newConcurrentHashSet();
public void addCandidate(
@@ -42,8 +46,16 @@
return !enumTypeToInfo.get(enumType).getSubclasses().isEmpty();
}
+ public Set<DexType> getSubtypes(DexType enumType) {
+ return SetUtils.mapIdentityHashSet(
+ enumTypeToInfo.get(enumType).getSubclasses(), DexClass::getType);
+ }
+
public void setEnumSubclasses(DexType superEnum, Set<DexProgramClass> subclasses) {
enumTypeToInfo.get(superEnum).setSubclasses(subclasses);
+ for (DexProgramClass subclass : subclasses) {
+ subEnumToSuperEnumMap.put(subclass.getType(), superEnum);
+ }
}
public void addPrunedMethod(ProgramMethod method) {
@@ -70,16 +82,27 @@
return ImmutableSet.copyOf(enumTypeToInfo.keySet());
}
- public ImmutableSet<DexProgramClass> candidateClasses() {
- ImmutableSet.Builder<DexProgramClass> builder = ImmutableSet.builder();
+ public ImmutableMap<DexProgramClass, Set<DexProgramClass>> candidateClassesWithSubclasses() {
+ ImmutableMap.Builder<DexProgramClass, Set<DexProgramClass>> builder = ImmutableMap.builder();
for (EnumUnboxingCandidateInfo info : enumTypeToInfo.values()) {
- builder.add(info.getEnumClass());
+ builder.put(info.getEnumClass(), info.getSubclasses());
}
return builder.build();
}
+ /** Answers true if both enums are identical, or if one inherit from the other. */
+ public boolean isAssignableTo(DexType subtype, DexType superType) {
+ assert superType != null;
+ assert subtype != null;
+ if (superType == subtype) {
+ return true;
+ }
+ return superType == subEnumToSuperEnumMap.get(subtype);
+ }
+
public DexProgramClass getCandidateClassOrNull(DexType enumType) {
- EnumUnboxingCandidateInfo info = enumTypeToInfo.get(enumType);
+ DexType superEnum = subEnumToSuperEnumMap.getOrDefault(enumType, enumType);
+ EnumUnboxingCandidateInfo info = enumTypeToInfo.get(superEnum);
if (info == null) {
return null;
}
@@ -120,16 +143,8 @@
info.addRequiredInstanceFieldData(field);
}
- public void forEachCandidate(Consumer<DexProgramClass> enumClassConsumer) {
- enumTypeToInfo.values().forEach(info -> enumClassConsumer.accept(info.enumClass));
- }
-
- public void forEachCandidateAndRequiredInstanceFieldData(
- BiConsumer<DexProgramClass, Set<DexField>> biConsumer) {
- enumTypeToInfo
- .values()
- .forEach(
- info -> biConsumer.accept(info.getEnumClass(), info.getRequiredInstanceFieldData()));
+ public void forEachCandidateInfo(Consumer<EnumUnboxingCandidateInfo> consumer) {
+ enumTypeToInfo.values().forEach(consumer);
}
public void clear() {
@@ -143,7 +158,7 @@
return true;
}
- private static class EnumUnboxingCandidateInfo {
+ public static class EnumUnboxingCandidateInfo {
private final DexProgramClass enumClass;
private final LongLivedProgramMethodSetBuilder<ProgramMethodSet> methodDependencies;
@@ -164,6 +179,7 @@
}
public Set<DexProgramClass> getSubclasses() {
+ assert subclasses != null;
return subclasses;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingLens.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingLens.java
index 91e6e67..56a2d20 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingLens.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingLens.java
@@ -4,12 +4,15 @@
package com.android.tools.r8.ir.optimize.enums;
+import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.ProgramMethod;
+import com.android.tools.r8.graph.lens.GraphLens;
+import com.android.tools.r8.graph.lens.MethodLookupResult;
import com.android.tools.r8.graph.lens.NestedGraphLens;
import com.android.tools.r8.graph.proto.ArgumentInfoCollection;
import com.android.tools.r8.graph.proto.RewrittenPrototypeDescription;
@@ -43,10 +46,11 @@
EnumUnboxingLens(
AppView<?> appView,
BidirectionalOneToOneMap<DexField, DexField> fieldMap,
- BidirectionalOneToManyRepresentativeMap<DexMethod, DexMethod> methodMap,
+ BidirectionalOneToManyRepresentativeMap<DexMethod, DexMethod> renamedSignatures,
Map<DexType, DexType> typeMap,
+ Map<DexMethod, DexMethod> methodMap,
Map<DexMethod, RewrittenPrototypeDescription> prototypeChangesPerMethod) {
- super(appView, fieldMap, methodMap::getRepresentativeValue, typeMap, methodMap);
+ super(appView, fieldMap, methodMap, typeMap, renamedSignatures);
assert !appView.unboxedEnums().isEmpty();
this.abstractValueFactory = appView.abstractValueFactory();
this.prototypeChangesPerMethod = prototypeChangesPerMethod;
@@ -69,6 +73,58 @@
}
@Override
+ public boolean isContextFreeForMethods(GraphLens codeLens) {
+ if (codeLens == this) {
+ return true;
+ }
+ return !unboxedEnums.hasAnyEnumsWithSubtypes()
+ && getPrevious().isContextFreeForMethods(codeLens);
+ }
+
+ @Override
+ public boolean verifyIsContextFreeForMethod(DexMethod method, GraphLens codeLens) {
+ if (codeLens == this) {
+ return true;
+ }
+ assert getPrevious().verifyIsContextFreeForMethod(getPreviousMethodSignature(method), codeLens);
+ DexMethod previous =
+ getPrevious()
+ .lookupMethod(getPreviousMethodSignature(method), null, null, codeLens)
+ .getReference();
+ assert unboxedEnums.representativeType(previous.getHolderType()) == previous.getHolderType();
+ return true;
+ }
+
+ @Override
+ public MethodLookupResult internalDescribeLookupMethod(
+ MethodLookupResult previous, DexMethod context, GraphLens codeLens) {
+ assert context != null || verifyIsContextFreeForMethod(previous.getReference(), codeLens);
+ assert context == null || previous.getType() != null;
+ DexMethod result;
+ if (previous.getType() == InvokeType.SUPER) {
+ assert context != null;
+ DexType superEnum = unboxedEnums.representativeType(context.getHolderType());
+ if (superEnum != context.getHolderType()) {
+ // TODO(b/271385332): implement this.
+ throw new Unreachable();
+ } else {
+ result = methodMap.apply(previous.getReference());
+ }
+ } else {
+ result = methodMap.apply(previous.getReference());
+ }
+ if (result == null) {
+ return previous;
+ }
+ return MethodLookupResult.builder(this)
+ .setReference(result)
+ .setPrototypeChanges(
+ internalDescribePrototypeChanges(previous.getPrototypeChanges(), result))
+ .setType(mapInvocationType(result, previous.getReference(), previous.getType()))
+ .build();
+ }
+
+ @Override
protected RewrittenPrototypeDescription internalDescribePrototypeChanges(
RewrittenPrototypeDescription prototypeChanges, DexMethod method) {
// Rewrite the single value of the given RewrittenPrototypeDescription if it is referring to an
@@ -133,6 +189,7 @@
new BidirectionalOneToOneHashMap<>();
private final MutableBidirectionalOneToManyRepresentativeMap<DexMethod, DexMethod>
newMethodSignatures = new BidirectionalOneToManyRepresentativeHashMap<>();
+ private final Map<DexMethod, DexMethod> methodMap = new IdentityHashMap<>();
private Map<DexMethod, RewrittenPrototypeDescription> prototypeChangesPerMethod =
new IdentityHashMap<>();
@@ -155,18 +212,59 @@
newFieldSignatures.put(from, to);
}
- public void move(DexMethod from, DexMethod to, boolean fromStatic, boolean toStatic) {
- move(from, to, fromStatic, toStatic, Collections.emptyList());
+ private RewrittenPrototypeDescription recordPrototypeChanges(
+ DexMethod from,
+ DexMethod to,
+ boolean fromStatic,
+ boolean toStatic,
+ boolean virtualReceiverAlreadyRemapped,
+ List<ExtraUnusedNullParameter> extraUnusedNullParameters) {
+ assert from != to;
+ RewrittenPrototypeDescription prototypeChanges =
+ computePrototypeChanges(
+ from,
+ to,
+ fromStatic,
+ toStatic,
+ virtualReceiverAlreadyRemapped,
+ extraUnusedNullParameters);
+ prototypeChangesPerMethod.put(to, prototypeChanges);
+ return prototypeChanges;
}
- public RewrittenPrototypeDescription move(
+ public void moveAndMap(DexMethod from, DexMethod to, boolean fromStatic) {
+ moveAndMap(from, to, fromStatic, true, Collections.emptyList());
+ }
+
+ public RewrittenPrototypeDescription moveVirtual(DexMethod from, DexMethod to) {
+ newMethodSignatures.put(from, to);
+ return recordPrototypeChanges(from, to, false, true, false, Collections.emptyList());
+ }
+
+ public RewrittenPrototypeDescription mapToDispatch(DexMethod from, DexMethod to) {
+ methodMap.put(from, to);
+ return recordPrototypeChanges(from, to, false, true, true, Collections.emptyList());
+ }
+
+ public RewrittenPrototypeDescription moveAndMap(
DexMethod from,
DexMethod to,
boolean fromStatic,
boolean toStatic,
List<ExtraUnusedNullParameter> extraUnusedNullParameters) {
- assert from != to;
newMethodSignatures.put(from, to);
+ methodMap.put(from, to);
+ return recordPrototypeChanges(
+ from, to, fromStatic, toStatic, false, extraUnusedNullParameters);
+ }
+
+ private RewrittenPrototypeDescription computePrototypeChanges(
+ DexMethod from,
+ DexMethod to,
+ boolean fromStatic,
+ boolean toStatic,
+ boolean virtualReceiverAlreadyRemapped,
+ List<ExtraUnusedNullParameter> extraUnusedNullParameters) {
int offsetDiff = 0;
int toOffset = BooleanUtils.intValue(!toStatic);
ArgumentInfoCollection.Builder builder =
@@ -175,14 +273,21 @@
if (fromStatic != toStatic) {
assert toStatic;
offsetDiff = 1;
- builder
- .addArgumentInfo(
- 0,
- RewrittenTypeInfo.builder()
- .setOldType(from.getHolderType())
- .setNewType(to.getParameter(0))
- .build())
- .setIsConvertedToStaticMethod();
+ if (!virtualReceiverAlreadyRemapped) {
+ builder
+ .addArgumentInfo(
+ 0,
+ RewrittenTypeInfo.builder()
+ .setOldType(from.getHolderType())
+ .setNewType(to.getParameter(0))
+ .build())
+ .setIsConvertedToStaticMethod();
+ } else {
+ assert to.getParameter(0).isIntType();
+ assert !fromStatic;
+ assert toStatic;
+ assert from.getArity() == to.getArity() - 1;
+ }
}
for (int i = 0; i < from.getParameters().size(); i++) {
DexType fromType = from.getParameter(i);
@@ -200,11 +305,8 @@
.setOldType(from.getReturnType())
.setNewType(to.getReturnType())
.build();
- RewrittenPrototypeDescription prototypeChanges =
- RewrittenPrototypeDescription.createForRewrittenTypes(returnInfo, builder.build())
- .withExtraParameters(extraUnusedNullParameters);
- prototypeChangesPerMethod.put(to, prototypeChanges);
- return prototypeChanges;
+ return RewrittenPrototypeDescription.createForRewrittenTypes(returnInfo, builder.build())
+ .withExtraParameters(extraUnusedNullParameters);
}
void recordCheckNotZeroMethod(
@@ -227,6 +329,7 @@
newFieldSignatures,
newMethodSignatures,
typeMap,
+ methodMap,
ImmutableMap.copyOf(prototypeChangesPerMethod));
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingTreeFixer.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingTreeFixer.java
index 738ed1e..7d7ce1c 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingTreeFixer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingTreeFixer.java
@@ -5,6 +5,7 @@
package com.android.tools.r8.ir.optimize.enums;
import static com.android.tools.r8.ir.analysis.type.Nullability.definitelyNotNull;
+import static com.android.tools.r8.ir.optimize.enums.EnumUnboxerImpl.ordinalToUnboxedInt;
import com.android.tools.r8.contexts.CompilationContext.ProcessorContext;
import com.android.tools.r8.graph.AppView;
@@ -49,15 +50,22 @@
import com.android.tools.r8.ir.optimize.info.OptimizationFeedback;
import com.android.tools.r8.ir.optimize.info.OptimizationFeedbackIgnore;
import com.android.tools.r8.ir.optimize.info.field.InstanceFieldInitializationInfo;
+import com.android.tools.r8.ir.synthetic.EnumUnboxingCfCodeProvider.EnumUnboxingMethodDispatchCfCodeProvider;
+import com.android.tools.r8.ir.synthetic.EnumUnboxingCfCodeProvider.EnumUnboxingMethodDispatchCfCodeProvider.CfCodeWithLens;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.ImmutableArrayUtils;
import com.android.tools.r8.utils.OptionalBool;
import com.android.tools.r8.utils.SetUtils;
import com.android.tools.r8.utils.ThreadUtils;
import com.android.tools.r8.utils.Timing;
+import com.android.tools.r8.utils.collections.DexMethodSignatureSet;
import com.android.tools.r8.utils.collections.ProgramMethodMap;
+import com.android.tools.r8.utils.collections.ProgramMethodSet;
import com.google.common.collect.BiMap;
import com.google.common.collect.HashBiMap;
+import com.google.common.collect.Sets;
+import it.unimi.dsi.fastutil.ints.Int2ObjectArrayMap;
+import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
import java.util.ArrayList;
import java.util.Collection;
import java.util.IdentityHashMap;
@@ -69,6 +77,7 @@
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
+import java.util.function.Consumer;
import java.util.function.Predicate;
class EnumUnboxingTreeFixer {
@@ -77,27 +86,45 @@
private final AppView<AppInfoWithLiveness> appView;
private final ProgramMethodMap<Set<DexProgramClass>> checkNotNullMethods;
private final DexItemFactory factory;
+ // Provides information for each unboxed enum regarding the instance field values of each enum
+ // instance, the original instance class, and the ordinal value.
private final EnumDataMap enumDataMap;
- private final Set<DexProgramClass> unboxedEnums;
+ // Maps the superEnum to the enumSubtypes. This is already present in the enumDataMap as DexTypes,
+ // we duplicate that here as DexProgramClasses.
+ private final Map<DexProgramClass, Set<DexProgramClass>> unboxedEnumHierarchy;
private final EnumUnboxingUtilityClasses utilityClasses;
+ private final ProgramMethodMap<CfCodeWithLens> dispatchMethods = ProgramMethodMap.create();
EnumUnboxingTreeFixer(
AppView<AppInfoWithLiveness> appView,
ProgramMethodMap<Set<DexProgramClass>> checkNotNullMethods,
EnumDataMap enumDataMap,
- Set<DexProgramClass> unboxedEnums,
+ Map<DexProgramClass, Set<DexProgramClass>> unboxedEnums,
EnumUnboxingUtilityClasses utilityClasses) {
this.appView = appView;
this.checkNotNullMethods = checkNotNullMethods;
this.enumDataMap = enumDataMap;
this.factory = appView.dexItemFactory();
+ this.unboxedEnumHierarchy = unboxedEnums;
this.lensBuilder =
- EnumUnboxingLens.enumUnboxingLensBuilder(appView)
- .mapUnboxedEnums(enumDataMap.getUnboxedEnums());
- this.unboxedEnums = unboxedEnums;
+ EnumUnboxingLens.enumUnboxingLensBuilder(appView).mapUnboxedEnums(getUnboxedEnums());
this.utilityClasses = utilityClasses;
}
+ private Set<DexProgramClass> computeUnboxedEnumClasses() {
+ Set<DexProgramClass> unboxedEnumClasses = Sets.newIdentityHashSet();
+ unboxedEnumHierarchy.forEach(
+ (superEnum, subEnums) -> {
+ unboxedEnumClasses.add(superEnum);
+ unboxedEnumClasses.addAll(subEnums);
+ });
+ return unboxedEnumClasses;
+ }
+
+ private Set<DexType> getUnboxedEnums() {
+ return enumDataMap.computeAllUnboxedEnums();
+ }
+
Result fixupTypeReferences(IRConverter converter, ExecutorService executorService)
throws ExecutionException {
PrunedItems.Builder prunedItemsBuilder = PrunedItems.builder();
@@ -108,25 +135,27 @@
// Fix all methods and fields using enums to unbox.
// TODO(b/191617665): Parallelize this fixup.
for (DexProgramClass clazz : appView.appInfo().classes()) {
- if (enumDataMap.isUnboxedEnum(clazz)) {
+ if (enumDataMap.isSuperUnboxedEnum(clazz.getType())) {
+
// Clear the initializers and move the other methods to the new location.
LocalEnumUnboxingUtilityClass localUtilityClass =
utilityClasses.getLocalUtilityClass(clazz);
Collection<DexEncodedField> localUtilityFields =
createLocalUtilityFields(clazz, localUtilityClass, prunedItemsBuilder);
Collection<DexEncodedMethod> localUtilityMethods =
- createLocalUtilityMethods(clazz, localUtilityClass, prunedItemsBuilder);
+ createLocalUtilityMethods(
+ clazz, unboxedEnumHierarchy.get(clazz), localUtilityClass, prunedItemsBuilder);
- // Cleanup old class.
- clazz.clearInstanceFields();
- clazz.clearStaticFields();
- clazz.getMethodCollection().clearDirectMethods();
- clazz.getMethodCollection().clearVirtualMethods();
+ // Cleanup old classes.
+ cleanUpOldClass(clazz);
+ for (DexProgramClass subEnum : unboxedEnumHierarchy.get(clazz)) {
+ cleanUpOldClass(subEnum);
+ }
// Update members on the local utility class.
localUtilityClass.getDefinition().setDirectMethods(localUtilityMethods);
localUtilityClass.getDefinition().setStaticFields(localUtilityFields);
- } else {
+ } else if (!enumDataMap.isUnboxedEnum(clazz.getType())) {
clazz.getMethodCollection().replaceMethods(this::fixupEncodedMethod);
clazz.getFieldCollection().replaceFields(this::fixupEncodedField);
}
@@ -143,7 +172,22 @@
BiMap<DexMethod, DexMethod> checkNotNullToCheckNotZeroMapping =
duplicateCheckNotNullMethods(converter, executorService);
- return new Result(checkNotNullToCheckNotZeroMapping, lens, prunedItemsBuilder.build());
+ ProgramMethodSet dispatchMethodSet = ProgramMethodSet.create();
+ dispatchMethods.forEach(
+ (method, code) -> {
+ dispatchMethodSet.add(method);
+ code.setCodeLens(lens);
+ });
+
+ return new Result(
+ checkNotNullToCheckNotZeroMapping, dispatchMethodSet, lens, prunedItemsBuilder.build());
+ }
+
+ private void cleanUpOldClass(DexProgramClass clazz) {
+ clazz.clearInstanceFields();
+ clazz.clearStaticFields();
+ clazz.getMethodCollection().clearDirectMethods();
+ clazz.getMethodCollection().clearVirtualMethods();
}
private BiMap<DexMethod, DexMethod> duplicateCheckNotNullMethods(
@@ -154,10 +198,12 @@
OneTimeMethodProcessor.Builder methodProcessorBuilder =
OneTimeMethodProcessor.builder(eventConsumer, processorContext);
+ Set<DexProgramClass> unboxedEnumClasses = computeUnboxedEnumClasses();
+
// Only duplicate checkNotNull() methods that are required for enum unboxing.
checkNotNullMethods.removeIf(
(checkNotNullMethod, dependentEnums) ->
- !SetUtils.containsAnyOf(unboxedEnums, dependentEnums));
+ !SetUtils.containsAnyOf(unboxedEnumClasses, dependentEnums));
// For each checkNotNull() method, synthesize a free flowing static checkNotZero() method that
// takes an int instead of an Object with the same implementation.
@@ -230,7 +276,7 @@
.resolveField(appView.dexItemFactory().enumMembers.ordinalField)
.getResolvedField();
ThreadUtils.processItems(
- unboxedEnums,
+ unboxedEnumHierarchy.keySet(),
unboxedEnum -> fixupEnumClassInitializer(converter, unboxedEnum, ordinalField),
executorService);
}
@@ -487,6 +533,7 @@
private Collection<DexEncodedMethod> createLocalUtilityMethods(
DexProgramClass unboxedEnum,
+ Set<DexProgramClass> subEnums,
LocalEnumUnboxingUtilityClass localUtilityClass,
PrunedItems.Builder prunedItemsBuilder) {
Map<DexMethod, DexEncodedMethod> localUtilityMethods =
@@ -496,22 +543,157 @@
localUtilityClass
.getDefinition()
.forEachMethod(method -> localUtilityMethods.put(method.getReference(), method));
+ // First generate all methods from the super enums, for any override, generate the dispatch
+ // method and both the move and super contextual move.
+ DexMethodSignatureSet methodsWithOverride = DexMethodSignatureSet.create();
+ forEachWhilePruningInstanceInitializers(
+ unboxedEnum,
+ prunedItemsBuilder,
+ method -> {
+ DexEncodedMethod superEnumUtilityMethod =
+ installLocalUtilityMethod(localUtilityClass, localUtilityMethods, method);
+ Map<DexMethod, DexMethod> overrideToUtilityMethods = new IdentityHashMap<>();
+ if (method.getDefinition().isNonPrivateVirtualMethod()) {
+ for (DexProgramClass subEnum : subEnums) {
+ ProgramMethod override = subEnum.lookupProgramMethod(method.getReference());
+ if (override != null) {
+ methodsWithOverride.add(method);
+ DexEncodedMethod subEnumLocalUtilityMethod =
+ installLocalUtilityMethod(localUtilityClass, localUtilityMethods, override);
+ overrideToUtilityMethods.put(
+ override.getReference(), subEnumLocalUtilityMethod.getReference());
+ }
+ }
+ }
+ if (overrideToUtilityMethods.isEmpty()) {
+ lensBuilder.moveAndMap(
+ method.getReference(),
+ superEnumUtilityMethod.getReference(),
+ method.getDefinition().isStatic());
+ } else {
+ DexMethod dispatch =
+ installDispatchMethod(
+ localUtilityClass,
+ localUtilityMethods,
+ method,
+ superEnumUtilityMethod,
+ overrideToUtilityMethods)
+ .getReference();
+ recordEmulatedDispatch(
+ method.getReference(), superEnumUtilityMethod.getReference(), dispatch);
+ for (DexMethod override : overrideToUtilityMethods.keySet()) {
+ recordEmulatedDispatch(override, overrideToUtilityMethods.get(override), dispatch);
+ }
+ }
+ });
+ // TODO(b/271385332): Check dispatch methods without implementation in the superEnum.
+ // Second for each subEnum generate the remaining methods if not already generated.
+ for (DexProgramClass subEnum : subEnums) {
+ forEachWhilePruningInstanceInitializers(
+ subEnum,
+ prunedItemsBuilder,
+ method -> {
+ if (!methodsWithOverride.contains(method.getReference())) {
+ DexEncodedMethod newLocalUtilityMethod =
+ installLocalUtilityMethod(localUtilityClass, localUtilityMethods, method);
+ // We cannot remap the subtype <clinit> to the same method than the superEnum one,
+ // so we have to extend the design to support subtype <clinit> here.
+ assert !method.getDefinition().isClassInitializer();
+ lensBuilder.moveAndMap(
+ method.getReference(),
+ newLocalUtilityMethod.getReference(),
+ method.getDefinition().isStatic());
+ }
+ });
+ }
+ return localUtilityMethods.values();
+ }
- unboxedEnum.forEachProgramMethod(
+ public void recordEmulatedDispatch(DexMethod from, DexMethod move, DexMethod dispatch) {
+ // Move is used for getRenamedSignature and to remap invoke-super.
+ // Map is used to remap all the other invokes.
+ lensBuilder.moveVirtual(from, move);
+ lensBuilder.mapToDispatch(from, dispatch);
+ }
+
+ private DexEncodedMethod installDispatchMethod(
+ LocalEnumUnboxingUtilityClass localUtilityClass,
+ Map<DexMethod, DexEncodedMethod> localUtilityMethods,
+ ProgramMethod method,
+ DexEncodedMethod superEnumUtilityMethod,
+ Map<DexMethod, DexMethod> map) {
+ assert !map.isEmpty();
+ DexMethod newLocalUtilityMethodReference =
+ factory.createFreshMethodNameWithoutHolder(
+ "_dispatch_" + method.getName().toString(),
+ fixupProto(factory.prependHolderToProto(method.getReference())),
+ localUtilityClass.getType(),
+ newMethodSignature -> !localUtilityMethods.containsKey(newMethodSignature));
+ Int2ObjectMap<DexMethod> methodMap = new Int2ObjectArrayMap<>();
+ IdentityHashMap<DexType, DexMethod> typeToMethod = new IdentityHashMap<>();
+ map.forEach(
+ (methodReference, newMethodReference) ->
+ typeToMethod.put(methodReference.getHolderType(), newMethodReference));
+ DexProgramClass unboxedEnum = method.getHolder();
+ assert enumDataMap.get(unboxedEnum).valuesTypes != null;
+ enumDataMap
+ .get(unboxedEnum)
+ .valuesTypes
+ .forEach(
+ (i, type) -> {
+ if (typeToMethod.containsKey(type)) {
+ methodMap.put(ordinalToUnboxedInt(i), typeToMethod.get(type));
+ }
+ });
+ CfCodeWithLens codeWithLens =
+ new EnumUnboxingMethodDispatchCfCodeProvider(
+ appView,
+ localUtilityClass.getType(),
+ superEnumUtilityMethod.getReference(),
+ methodMap)
+ .generateCfCode();
+ DexEncodedMethod newLocalUtilityMethod =
+ DexEncodedMethod.builder()
+ .setMethod(newLocalUtilityMethodReference)
+ .setAccessFlags(MethodAccessFlags.createPublicStaticSynthetic())
+ .setCode(codeWithLens)
+ .setClassFileVersion(unboxedEnum.getInitialClassFileVersion())
+ .setApiLevelForDefinition(superEnumUtilityMethod.getApiLevelForDefinition())
+ .setApiLevelForCode(superEnumUtilityMethod.getApiLevelForCode())
+ .build();
+ dispatchMethods.put(
+ newLocalUtilityMethod.asProgramMethod(localUtilityClass.getDefinition()), codeWithLens);
+ assert !localUtilityMethods.containsKey(newLocalUtilityMethodReference);
+ localUtilityMethods.put(newLocalUtilityMethodReference, newLocalUtilityMethod);
+ return newLocalUtilityMethod;
+ }
+
+ private DexEncodedMethod installLocalUtilityMethod(
+ LocalEnumUnboxingUtilityClass localUtilityClass,
+ Map<DexMethod, DexEncodedMethod> localUtilityMethods,
+ ProgramMethod method) {
+ DexEncodedMethod newLocalUtilityMethod =
+ createLocalUtilityMethod(
+ method,
+ localUtilityClass,
+ newMethodSignature -> !localUtilityMethods.containsKey(newMethodSignature));
+ assert !localUtilityMethods.containsKey(newLocalUtilityMethod.getReference());
+ localUtilityMethods.put(newLocalUtilityMethod.getReference(), newLocalUtilityMethod);
+ return newLocalUtilityMethod;
+ }
+
+ private void forEachWhilePruningInstanceInitializers(
+ DexProgramClass clazz,
+ PrunedItems.Builder prunedItemsBuilder,
+ Consumer<? super ProgramMethod> consumer) {
+ clazz.forEachProgramMethod(
method -> {
if (method.getDefinition().isInstanceInitializer()) {
prunedItemsBuilder.addRemovedMethod(method.getReference());
} else {
- DexEncodedMethod newLocalUtilityMethod =
- createLocalUtilityMethod(
- method,
- localUtilityClass,
- newMethodSignature -> !localUtilityMethods.containsKey(newMethodSignature));
- assert !localUtilityMethods.containsKey(newLocalUtilityMethod.getReference());
- localUtilityMethods.put(newLocalUtilityMethod.getReference(), newLocalUtilityMethod);
+ consumer.accept(method);
}
});
- return localUtilityMethods.values();
}
private DexEncodedMethod createLocalUtilityMethod(
@@ -534,9 +716,6 @@
localUtilityClass.getType(),
availableMethodSignatures);
- // Record the move.
- lensBuilder.move(methodReference, newMethod, method.getDefinition().isStatic(), true);
-
return method
.getDefinition()
.toTypeSubstitutedMethod(
@@ -581,7 +760,7 @@
ExtraUnusedNullParameter.computeExtraUnusedNullParameters(method.getReference(), newMethod);
boolean isStatic = method.isStatic();
RewrittenPrototypeDescription prototypeChanges =
- lensBuilder.move(
+ lensBuilder.moveAndMap(
method.getReference(), newMethod, isStatic, isStatic, extraUnusedNullParameters);
return method.toTypeSubstitutedMethod(
newMethod,
@@ -662,14 +841,17 @@
public static class Result {
private final BiMap<DexMethod, DexMethod> checkNotNullToCheckNotZeroMapping;
+ private final ProgramMethodSet dispatchMethods;
private final EnumUnboxingLens lens;
private final PrunedItems prunedItems;
Result(
BiMap<DexMethod, DexMethod> checkNotNullToCheckNotZeroMapping,
+ ProgramMethodSet dispatchMethods,
EnumUnboxingLens lens,
PrunedItems prunedItems) {
this.checkNotNullToCheckNotZeroMapping = checkNotNullToCheckNotZeroMapping;
+ this.dispatchMethods = dispatchMethods;
this.lens = lens;
this.prunedItems = prunedItems;
}
@@ -678,6 +860,10 @@
return checkNotNullToCheckNotZeroMapping;
}
+ public ProgramMethodSet getDispatchMethods() {
+ return dispatchMethods;
+ }
+
EnumUnboxingLens getLens() {
return lens;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/eligibility/Reason.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/eligibility/Reason.java
index bfd215d..56b05f6 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/eligibility/Reason.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/eligibility/Reason.java
@@ -31,6 +31,8 @@
public static final Reason NO_INIT = new StringReason("NO_INIT");
public static final Reason INVALID_INIT = new StringReason("INVALID_INIT");
public static final Reason INVALID_CLINIT = new StringReason("INVALID_CLINIT");
+ public static final Reason INVALID_SUBTYPE_INIT = new StringReason("INVALID_SUBTYPE_INIT");
+ public static final Reason SUBTYPE_CLINIT = new StringReason("SUBTYPE_CLINIT");
public static final Reason INVALID_INVOKE = new StringReason("INVALID_INVOKE");
public static final Reason INVALID_INVOKE_CLASSPATH =
new StringReason("INVALID_INVOKE_CLASSPATH");
diff --git a/src/main/java/com/android/tools/r8/ir/synthetic/EnumUnboxingCfCodeProvider.java b/src/main/java/com/android/tools/r8/ir/synthetic/EnumUnboxingCfCodeProvider.java
index 941248a..af9e41c 100644
--- a/src/main/java/com/android/tools/r8/ir/synthetic/EnumUnboxingCfCodeProvider.java
+++ b/src/main/java/com/android/tools/r8/ir/synthetic/EnumUnboxingCfCodeProvider.java
@@ -16,6 +16,7 @@
import com.android.tools.r8.cf.code.CfLoad;
import com.android.tools.r8.cf.code.CfNew;
import com.android.tools.r8.cf.code.CfReturn;
+import com.android.tools.r8.cf.code.CfReturnVoid;
import com.android.tools.r8.cf.code.CfStackInstruction;
import com.android.tools.r8.cf.code.CfStackInstruction.Opcode;
import com.android.tools.r8.cf.code.CfStaticFieldRead;
@@ -29,11 +30,13 @@
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.lens.GraphLens;
import com.android.tools.r8.ir.analysis.value.AbstractValue;
import com.android.tools.r8.ir.code.IfType;
import com.android.tools.r8.ir.code.ValueType;
import com.android.tools.r8.ir.optimize.enums.EnumDataMap.EnumData;
import com.android.tools.r8.ir.optimize.enums.EnumInstanceFieldData.EnumInstanceFieldMappingData;
+import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
import java.util.ArrayList;
import java.util.List;
import org.objectweb.asm.Opcodes;
@@ -65,6 +68,85 @@
}
}
+ public static class EnumUnboxingMethodDispatchCfCodeProvider extends EnumUnboxingCfCodeProvider {
+
+ private final GraphLens codeLens;
+ private final DexMethod superEnumMethod;
+ private final Int2ObjectMap<DexMethod> methodMap;
+
+ public EnumUnboxingMethodDispatchCfCodeProvider(
+ AppView<?> appView,
+ DexType holder,
+ DexMethod superEnumMethod,
+ Int2ObjectMap<DexMethod> methodMap) {
+ super(appView, holder);
+ this.codeLens = appView.codeLens();
+ this.superEnumMethod = superEnumMethod;
+ this.methodMap = methodMap;
+ }
+
+ @Override
+ public CfCodeWithLens generateCfCode() {
+ // TODO(b/167942775): Should use a table-switch for large enums (maybe same threshold in the
+ // rewriter of switchmaps).
+
+ DexItemFactory factory = appView.dexItemFactory();
+ int returnInvokeSize = superEnumMethod.getParameters().size() + 2;
+ List<CfInstruction> instructions =
+ new ArrayList<>(methodMap.size() * (returnInvokeSize + 5) + returnInvokeSize);
+
+ CfFrame.Builder frameBuilder = CfFrame.builder();
+ for (DexType parameter : superEnumMethod.getParameters()) {
+ frameBuilder.appendLocal(FrameType.initialized(parameter));
+ }
+ methodMap.forEach(
+ (unboxedEnumValue, method) -> {
+ CfLabel dest = new CfLabel();
+ instructions.add(new CfLoad(ValueType.fromDexType(factory.intType), 0));
+ instructions.add(new CfConstNumber(unboxedEnumValue, ValueType.INT));
+ instructions.add(new CfIfCmp(IfType.NE, ValueType.INT, dest));
+ addReturnInvoke(instructions, method);
+ instructions.add(dest);
+ instructions.add(frameBuilder.build());
+ });
+
+ addReturnInvoke(instructions, superEnumMethod);
+ return new CfCodeWithLens(getHolder(), defaultMaxStack(), defaultMaxLocals(), instructions);
+ }
+
+ private void addReturnInvoke(List<CfInstruction> instructions, DexMethod method) {
+ int localIndex = 0;
+ for (DexType parameterType : method.getParameters()) {
+ instructions.add(new CfLoad(ValueType.fromDexType(parameterType), localIndex));
+ localIndex += parameterType.getRequiredRegisters();
+ }
+ instructions.add(new CfInvoke(Opcodes.INVOKESTATIC, method, false));
+ instructions.add(
+ method.getReturnType().isVoidType()
+ ? new CfReturnVoid()
+ : new CfReturn(ValueType.fromDexType(method.getReturnType())));
+ }
+
+ public static class CfCodeWithLens extends CfCode {
+ private GraphLens codeLens;
+
+ public void setCodeLens(GraphLens codeLens) {
+ this.codeLens = codeLens;
+ }
+
+ public CfCodeWithLens(
+ DexType originalHolder, int maxStack, int maxLocals, List<CfInstruction> instructions) {
+ super(originalHolder, maxStack, maxLocals, instructions);
+ }
+
+ @Override
+ public GraphLens getCodeLens(AppView<?> appView) {
+ assert codeLens != null;
+ return codeLens;
+ }
+ }
+ }
+
public static class EnumUnboxingInstanceFieldCfCodeProvider extends EnumUnboxingCfCodeProvider {
private final DexType returnType;
diff --git a/src/main/java/com/android/tools/r8/profile/art/ArtProfileCompletenessChecker.java b/src/main/java/com/android/tools/r8/profile/art/ArtProfileCompletenessChecker.java
index d7fde7d..06464c7 100644
--- a/src/main/java/com/android/tools/r8/profile/art/ArtProfileCompletenessChecker.java
+++ b/src/main/java/com/android/tools/r8/profile/art/ArtProfileCompletenessChecker.java
@@ -85,7 +85,9 @@
ProgramDefinition definition,
Set<CompletenessExceptions> completenessExceptions,
List<DexReference> missing) {
- if (completenessExceptions.contains(ALLOW_MISSING_ENUM_UNBOXING_UTILITY_METHODS)) {
+ // TODO(b/274030968): Fix profile for enum unboxing with subtypes.
+ if (appView.options().testing.enableEnumWithSubtypesUnboxing
+ || completenessExceptions.contains(ALLOW_MISSING_ENUM_UNBOXING_UTILITY_METHODS)) {
DexType contextType = definition.getContextType();
SyntheticItems syntheticItems = appView.getSyntheticItems();
if (syntheticItems.isSynthetic(contextType)) {
diff --git a/src/main/java/com/android/tools/r8/utils/collections/LongLivedProgramMethodSetBuilder.java b/src/main/java/com/android/tools/r8/utils/collections/LongLivedProgramMethodSetBuilder.java
index 5bc7e56..81578f8 100644
--- a/src/main/java/com/android/tools/r8/utils/collections/LongLivedProgramMethodSetBuilder.java
+++ b/src/main/java/com/android/tools/r8/utils/collections/LongLivedProgramMethodSetBuilder.java
@@ -8,6 +8,7 @@
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexDefinitionSupplier;
+import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.ProgramMethod;
@@ -166,7 +167,9 @@
DexMethod rewrittenMethod =
appView.graphLens().getRenamedMethodSignature(method, appliedGraphLens);
DexProgramClass holder = appView.definitionForHolder(rewrittenMethod).asProgramClass();
- result.createAndAdd(holder, holder.lookupMethod(rewrittenMethod));
+ DexEncodedMethod definition = holder.lookupMethod(rewrittenMethod);
+ assert definition != null;
+ result.createAndAdd(holder, definition);
}
return result;
}
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingTestBase.java b/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingTestBase.java
index 4690144..420b2e9 100644
--- a/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingTestBase.java
+++ b/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingTestBase.java
@@ -61,7 +61,7 @@
options.enableEnumSwitchMapRemoval = enumValueOptimization;
}
- static List<Object[]> enumUnboxingTestParameters() {
+ public static List<Object[]> enumUnboxingTestParameters() {
return enumUnboxingTestParameters(
getTestParameters().withDexRuntimes().withAllApiLevels().build());
}
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/enummerging/BasicEnumMergingTest.java b/src/test/java/com/android/tools/r8/enumunboxing/enummerging/BasicEnumMergingTest.java
new file mode 100644
index 0000000..56f3d9e
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/enumunboxing/enummerging/BasicEnumMergingTest.java
@@ -0,0 +1,130 @@
+// 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.enumunboxing.enummerging;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.enumunboxing.EnumUnboxingTestBase;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class BasicEnumMergingTest extends EnumUnboxingTestBase {
+
+ private final TestParameters parameters;
+ private final boolean enumValueOptimization;
+ private final EnumKeepRules enumKeepRules;
+
+ @Parameters(name = "{0} valueOpt: {1} keep: {2}")
+ public static List<Object[]> data() {
+ return enumUnboxingTestParameters();
+ }
+
+ public BasicEnumMergingTest(
+ TestParameters parameters, boolean enumValueOptimization, EnumKeepRules enumKeepRules) {
+ this.parameters = parameters;
+ this.enumValueOptimization = enumValueOptimization;
+ this.enumKeepRules = enumKeepRules;
+ }
+
+ @Test
+ public void testEnumUnboxing() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(BasicEnumMergingTest.class)
+ .addKeepMainRule(Main.class)
+ .addKeepRules(enumKeepRules.getKeepRules())
+ .addOptionsModification(opt -> opt.testing.enableEnumWithSubtypesUnboxing = true)
+ .addEnumUnboxingInspector(
+ inspector ->
+ inspector
+ .assertUnboxed(
+ EnumWithVirtualOverride.class,
+ EnumWithVirtualOverrideAndPrivateMethod.class,
+ EnumWithVirtualOverrideWide.class)
+ .assertNotUnboxed(EnumWithVirtualOverrideAndPrivateField.class))
+ .enableInliningAnnotations()
+ .addOptionsModification(opt -> enableEnumOptions(opt, enumValueOptimization))
+ .addOptionsModification(opt -> opt.testing.enableEnumUnboxingDebugLogs = true)
+ .setMinApi(parameters)
+ .allowDiagnosticInfoMessages()
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("a", "B", "a", "B", "a", "B", "A 1 1.0 A", "B");
+ }
+
+ enum EnumWithVirtualOverride {
+ A {
+ public void method() {
+ System.out.println("a");
+ }
+ },
+ B;
+
+ public void method() {
+ System.out.println(name());
+ }
+ }
+
+ enum EnumWithVirtualOverrideAndPrivateMethod {
+ A {
+ @NeverInline
+ private void privateMethod() {
+ System.out.println("a");
+ }
+
+ public void methodpm() {
+ privateMethod();
+ }
+ },
+ B;
+
+ public void methodpm() {
+ System.out.println(name());
+ }
+ }
+
+ enum EnumWithVirtualOverrideWide {
+ A {
+ public void methodpmw(long l1, double d2, EnumWithVirtualOverrideWide itself) {
+ System.out.println("A " + l1 + " " + d2 + " " + itself);
+ }
+ },
+ B;
+
+ public void methodpmw(long l1, double d2, EnumWithVirtualOverrideWide itself) {
+ System.out.println(name());
+ }
+ }
+
+ enum EnumWithVirtualOverrideAndPrivateField {
+ A {
+ private String a = "a";
+
+ public void methodpf() {
+ System.out.println(a);
+ }
+ },
+ B;
+
+ public void methodpf() {
+ System.out.println(name());
+ }
+ }
+
+ static class Main {
+
+ public static void main(String[] args) {
+ EnumWithVirtualOverrideAndPrivateMethod.A.methodpm();
+ EnumWithVirtualOverrideAndPrivateMethod.B.methodpm();
+ EnumWithVirtualOverrideAndPrivateField.A.methodpf();
+ EnumWithVirtualOverrideAndPrivateField.B.methodpf();
+ EnumWithVirtualOverride.A.method();
+ EnumWithVirtualOverride.B.method();
+ EnumWithVirtualOverrideWide.A.methodpmw(1L, 1.0, EnumWithVirtualOverrideWide.A);
+ EnumWithVirtualOverrideWide.B.methodpmw(1L, 1.0, EnumWithVirtualOverrideWide.A);
+ }
+ }
+}
