Enum unboxing: postpone field analysis
Bug: 155368026
Change-Id: Ifb91956225fcfb7293476fa0fb7323b685c3633b
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index b838429..4c10624 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -739,7 +739,6 @@
postMethodProcessorBuilder.put(inliner);
}
if (enumUnboxer != null) {
- enumUnboxer.finishAnalysis();
enumUnboxer.unboxEnums(postMethodProcessorBuilder, executorService, feedback);
}
if (!options.debug) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumInstanceFieldDataMap.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumInstanceFieldDataMap.java
index 0c9c30c..f02e488 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumInstanceFieldDataMap.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumInstanceFieldDataMap.java
@@ -8,10 +8,6 @@
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.ir.optimize.enums.EnumInstanceFieldData.EnumInstanceFieldKnownData;
import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
-import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.function.Function;
public class EnumInstanceFieldDataMap {
private final ImmutableMap<DexType, ImmutableMap<DexField, EnumInstanceFieldKnownData>>
@@ -28,42 +24,4 @@
assert instanceFieldMap.get(enumType).containsKey(enumInstanceField);
return instanceFieldMap.get(enumType).get(enumInstanceField);
}
-
- public static Builder builder() {
- return new Builder();
- }
-
- public static class Builder {
- private final Map<DexType, Map<DexField, EnumInstanceFieldData>> instanceFieldMap =
- new ConcurrentHashMap<>();
-
- public EnumInstanceFieldData computeInstanceFieldDataIfAbsent(
- DexType enumType,
- DexField enumInstanceField,
- Function<DexField, EnumInstanceFieldData> enumFieldDataSupplier) {
- Map<DexField, EnumInstanceFieldData> typeMap =
- instanceFieldMap.computeIfAbsent(enumType, ignored -> new ConcurrentHashMap<>());
- return typeMap.computeIfAbsent(enumInstanceField, enumFieldDataSupplier);
- }
-
- public EnumInstanceFieldDataMap build(ImmutableSet<DexType> enumsToUnbox) {
- ImmutableMap.Builder<DexType, ImmutableMap<DexField, EnumInstanceFieldKnownData>> builder =
- ImmutableMap.builder();
- instanceFieldMap.forEach(
- (enumType, typeMap) -> {
- ImmutableMap.Builder<DexField, EnumInstanceFieldKnownData> typeBuilder =
- ImmutableMap.builder();
- typeMap.forEach(
- (field, data) -> {
- if (enumsToUnbox.contains(enumType)) {
- assert data.isKnown();
- typeBuilder.put(field, data.asEnumFieldKnownData());
- }
- });
- builder.put(enumType, typeBuilder.build());
- });
- instanceFieldMap.clear();
- return new EnumInstanceFieldDataMap(builder.build());
- }
- }
}
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 62c1087..dac0f6d 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
@@ -66,6 +66,7 @@
import com.android.tools.r8.ir.conversion.PostMethodProcessor;
import com.android.tools.r8.ir.conversion.PostOptimization;
import com.android.tools.r8.ir.optimize.Inliner.Constraint;
+import com.android.tools.r8.ir.optimize.enums.EnumInstanceFieldData.EnumInstanceFieldKnownData;
import com.android.tools.r8.ir.optimize.enums.EnumInstanceFieldData.EnumInstanceFieldMappingData;
import com.android.tools.r8.ir.optimize.enums.EnumInstanceFieldData.EnumInstanceFieldOrdinalData;
import com.android.tools.r8.ir.optimize.enums.EnumInstanceFieldData.EnumInstanceFieldUnknownData;
@@ -104,8 +105,8 @@
// Map the enum candidates with their dependencies, i.e., the methods to reprocess for the given
// enum if the optimization eventually decides to unbox it.
private final Map<DexType, ProgramMethodSet> enumsUnboxingCandidates;
- private final EnumInstanceFieldDataMap.Builder enumInstanceFieldData =
- EnumInstanceFieldDataMap.builder();
+ private final Map<DexType, Set<DexField>> requiredEnumInstanceFieldData =
+ new ConcurrentHashMap<>();
private final Set<DexType> enumsToUnboxWithPackageRequirement = Sets.newIdentityHashSet();
private EnumUnboxingRewriter enumUnboxerRewriter;
@@ -127,6 +128,12 @@
enumsUnboxingCandidates = new EnumUnboxingCandidateAnalysis(appView, this).findCandidates();
}
+ private void requiredEnumInstanceFieldData(DexType enumType, DexField field) {
+ requiredEnumInstanceFieldData
+ .computeIfAbsent(enumType, ignored -> Sets.newConcurrentHashSet())
+ .add(field);
+ }
+
private void markEnumAsUnboxable(Reason reason, DexProgramClass enumClass) {
assert enumClass.isEnum();
reportFailure(enumClass.type, reason);
@@ -299,15 +306,7 @@
}
// The name data is required for the correct mapping from the enum name to the ordinal in the
// valueOf utility method.
- EnumInstanceFieldData nameData =
- enumInstanceFieldData.computeInstanceFieldDataIfAbsent(
- enumType,
- factory.enumMembers.nameField,
- field -> computeEnumFieldData(field, enumClass));
- if (nameData.isUnknown()) {
- markEnumAsUnboxable(Reason.MISSING_NAME_DATA, enumClass);
- return;
- }
+ requiredEnumInstanceFieldData(enumType, factory.enumMembers.nameField);
eligibleEnums.add(enumType);
}
@@ -365,6 +364,7 @@
ExecutorService executorService,
OptimizationFeedbackDelayed feedback)
throws ExecutionException {
+ EnumInstanceFieldDataMap enumInstanceFieldDataMap = finishAnalysis();
// At this point the enumsToUnbox are no longer candidates, they will all be unboxed.
if (enumsUnboxingCandidates.isEmpty()) {
return;
@@ -372,8 +372,7 @@
ImmutableSet<DexType> enumsToUnbox = ImmutableSet.copyOf(this.enumsUnboxingCandidates.keySet());
// Update keep info on any of the enum methods of the removed classes.
updatePinnedItems(enumsToUnbox);
- enumUnboxerRewriter =
- new EnumUnboxingRewriter(appView, enumsToUnbox, enumInstanceFieldData.build(enumsToUnbox));
+ enumUnboxerRewriter = new EnumUnboxingRewriter(appView, enumsToUnbox, enumInstanceFieldDataMap);
DirectMappedDexApplication.Builder appBuilder = appView.appInfo().app().asDirect().builder();
Map<DexType, DexType> newMethodLocation = synthesizeUnboxedEnumsMethodsLocations(appBuilder);
NestedGraphLens enumUnboxingLens =
@@ -496,12 +495,38 @@
});
}
- public void finishAnalysis() {
+ public EnumInstanceFieldDataMap finishAnalysis() {
analyzeInitializers();
analyzeAccessibility();
+ EnumInstanceFieldDataMap enumInstanceFieldDataMap = analyzeFields();
if (debugLogEnabled) {
reportEnumsAnalysis();
}
+ return enumInstanceFieldDataMap;
+ }
+
+ private EnumInstanceFieldDataMap analyzeFields() {
+ ImmutableMap.Builder<DexType, ImmutableMap<DexField, EnumInstanceFieldKnownData>> builder =
+ ImmutableMap.builder();
+ requiredEnumInstanceFieldData.forEach(
+ (enumType, fields) -> {
+ ImmutableMap.Builder<DexField, EnumInstanceFieldKnownData> typeBuilder =
+ ImmutableMap.builder();
+ if (enumsUnboxingCandidates.containsKey(enumType)) {
+ DexProgramClass enumClass = appView.definitionFor(enumType).asProgramClass();
+ assert enumClass != null;
+ for (DexField field : fields) {
+ EnumInstanceFieldData enumInstanceFieldData = computeEnumFieldData(field, enumClass);
+ if (enumInstanceFieldData.isUnknown()) {
+ markEnumAsUnboxable(Reason.MISSING_INSTANCE_FIELD_DATA, enumClass);
+ return;
+ }
+ typeBuilder.put(field, enumInstanceFieldData.asEnumFieldKnownData());
+ }
+ builder.put(enumType, typeBuilder.build());
+ }
+ });
+ return new EnumInstanceFieldDataMap(builder.build());
}
private void analyzeAccessibility() {
@@ -839,15 +864,7 @@
} else if (singleTarget == factory.enumMembers.nameMethod
|| singleTarget == factory.enumMembers.toString) {
assert invokeMethod.asInvokeMethodWithReceiver().getReceiver() == enumValue;
- EnumInstanceFieldData nameData =
- enumInstanceFieldData.computeInstanceFieldDataIfAbsent(
- enumClass.type,
- factory.enumMembers.nameField,
- field -> computeEnumFieldData(field, enumClass));
- if (nameData.isUnknown()) {
- computeEnumFieldData(factory.enumMembers.nameField, enumClass);
- return Reason.MISSING_NAME_DATA;
- }
+ requiredEnumInstanceFieldData(enumClass.type, factory.enumMembers.nameField);
return Reason.ELIGIBLE;
} else if (singleTarget == factory.enumMembers.ordinalMethod) {
return Reason.ELIGIBLE;
@@ -892,12 +909,7 @@
InstanceGet instanceGet = instruction.asInstanceGet();
assert instanceGet.getField().holder == enumClass.type;
DexField field = instanceGet.getField();
- EnumInstanceFieldData enumFieldData =
- enumInstanceFieldData.computeInstanceFieldDataIfAbsent(
- enumClass.type, field, theField -> computeEnumFieldData(theField, enumClass));
- if (enumFieldData.isUnknown()) {
- return Reason.INVALID_FIELD_READ;
- }
+ requiredEnumInstanceFieldData(enumClass.type, field);
return Reason.ELIGIBLE;
}
@@ -1113,7 +1125,7 @@
COMPARE_TO_INVOKE,
UNSUPPORTED_LIBRARY_CALL,
MISSING_INFO_MAP,
- MISSING_NAME_DATA,
+ MISSING_INSTANCE_FIELD_DATA,
INVALID_FIELD_READ,
INVALID_FIELD_PUT,
INVALID_ARRAY_PUT,
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 c2d139b..54d9aaf 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
@@ -86,6 +86,7 @@
this.factory = appView.dexItemFactory();
EnumValueInfoMapCollection.Builder builder = EnumValueInfoMapCollection.builder();
for (DexType toUnbox : enumsToUnbox) {
+ assert appView.appInfo().withLiveness().getEnumValueInfoMap(toUnbox) != null;
builder.put(toUnbox, appView.appInfo().withLiveness().getEnumValueInfoMap(toUnbox));
}
this.enumsToUnbox = builder.build();
@@ -460,7 +461,7 @@
appView,
factory.enumUnboxingUtilityType,
field.type,
- enumsToUnbox.getEnumValueInfoMap(field.holder),
+ enumsToUnbox.getEnumValueInfoMap(enumType),
unboxedEnumsInstanceFieldData
.getInstanceFieldData(enumType, field)
.asEnumFieldMappingData())
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingB160535628Test.java b/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingB160535628Test.java
index 5ccd903..0c51e9d 100644
--- a/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingB160535628Test.java
+++ b/src/test/java/com/android/tools/r8/enumunboxing/EnumUnboxingB160535628Test.java
@@ -21,16 +21,21 @@
private final TestParameters parameters;
private final boolean missingStaticMethods;
+ private final EnumKeepRules enumKeepRules;
- @Parameterized.Parameters(name = "{0}")
+ @Parameterized.Parameters(name = "{0} missing: {1} keep: {2}")
public static List<Object[]> data() {
return buildParameters(
- getTestParameters().withDexRuntimes().withAllApiLevels().build(), BooleanUtils.values());
+ getTestParameters().withDexRuntimes().withAllApiLevels().build(),
+ BooleanUtils.values(),
+ getAllEnumKeepRules());
}
- public EnumUnboxingB160535628Test(TestParameters parameters, boolean missingStaticMethods) {
+ public EnumUnboxingB160535628Test(
+ TestParameters parameters, boolean missingStaticMethods, EnumKeepRules enumKeepRules) {
this.parameters = parameters;
this.missingStaticMethods = missingStaticMethods;
+ this.enumKeepRules = enumKeepRules;
}
@Test
@@ -44,6 +49,7 @@
.addProgramClasses(ProgramValueOf.class, ProgramStaticMethod.class)
.addProgramFiles(javaLibShrunk)
.addKeepMainRules(ProgramValueOf.class, ProgramStaticMethod.class)
+ .addKeepRules(enumKeepRules.getKeepRules())
.addOptionsModification(
options -> {
options.enableEnumUnboxing = true;
@@ -58,18 +64,23 @@
this::assertEnumUnboxedIfStaticMethodsPresent);
if (missingStaticMethods) {
compile
- .run(parameters.getRuntime(), ProgramValueOf.class)
- .assertFailureWithErrorThatMatches(containsString("NoSuchMethodError"))
- .assertFailureWithErrorThatMatches(containsString("valueOf"));
- compile
.run(parameters.getRuntime(), ProgramStaticMethod.class)
.assertFailureWithErrorThatMatches(containsString("NoSuchMethodError"))
.assertFailureWithErrorThatMatches(containsString("staticMethod"));
} else {
- compile.run(parameters.getRuntime(), ProgramValueOf.class).assertSuccessWithOutputLines("0");
compile
.run(parameters.getRuntime(), ProgramStaticMethod.class)
- .assertSuccessWithOutputLines("42");
+ .assertSuccessWithOutputLines("0", "42");
+ }
+ if (missingStaticMethods && enumKeepRules == EnumKeepRules.NONE) {
+ compile
+ .run(parameters.getRuntime(), ProgramValueOf.class)
+ .assertFailureWithErrorThatMatches(containsString("NoSuchMethodError"))
+ .assertFailureWithErrorThatMatches(containsString("valueOf"));
+ } else {
+ compile
+ .run(parameters.getRuntime(), ProgramValueOf.class)
+ .assertSuccessWithOutputLines("0", "0");
}
}
@@ -77,6 +88,7 @@
return testForR8(Backend.CF)
.addProgramClasses(Lib.class, Lib.LibEnumStaticMethod.class, Lib.LibEnum.class)
.addKeepRules("-keep enum * { <fields>; }")
+ .addKeepRules(enumKeepRules.getKeepRules())
.addKeepRules(missingStaticMethods ? "" : "-keep enum * { static <methods>; }")
.addOptionsModification(
options -> {
@@ -130,13 +142,15 @@
public static class ProgramValueOf {
public static void main(String[] args) {
- System.out.println(Lib.LibEnumStaticMethod.valueOf(Lib.LibEnum.A.name()).ordinal());
+ System.out.println(Lib.LibEnum.A.ordinal());
+ System.out.println(Lib.LibEnum.valueOf(Lib.LibEnum.A.name()).ordinal());
}
}
public static class ProgramStaticMethod {
public static void main(String[] args) {
+ System.out.println(Lib.LibEnumStaticMethod.A.ordinal());
System.out.println(Lib.LibEnumStaticMethod.staticMethod());
}
}
