Allow enums with subtypes without enum flag to be unboxed
Bug: b/315186101
Change-Id: I98d002594af56c2f63e953d1236652f3d3c882f8
diff --git a/src/main/java/com/android/tools/r8/graph/ClassAccessFlags.java b/src/main/java/com/android/tools/r8/graph/ClassAccessFlags.java
index dba763b..888f1e8 100644
--- a/src/main/java/com/android/tools/r8/graph/ClassAccessFlags.java
+++ b/src/main/java/com/android/tools/r8/graph/ClassAccessFlags.java
@@ -184,6 +184,10 @@
set(Constants.ACC_ENUM);
}
+ public void unsetEnum() {
+ unset(Constants.ACC_ENUM);
+ }
+
public boolean isRecord() {
return isSet(Constants.ACC_RECORD);
}
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 0eb2d7e..6050c9d 100644
--- a/src/main/java/com/android/tools/r8/graph/ObjectAllocationInfoCollectionImpl.java
+++ b/src/main/java/com/android/tools/r8/graph/ObjectAllocationInfoCollectionImpl.java
@@ -490,7 +490,6 @@
(clazz, allocationSitesForClass) -> {
DexType type = lens.lookupType(clazz.type);
if (type.isPrimitiveType()) {
- assert clazz.isEnum();
return;
}
DexProgramClass rewrittenClass = asProgramClassOrNull(definitions.definitionFor(type));
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 5132141..373c579 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
@@ -1417,12 +1417,12 @@
DexClassAndMethod mostAccurateTarget = singleTarget == null ? resolvedMethod : singleTarget;
if (mostAccurateTarget.isProgramMethod()) {
- if (mostAccurateTarget.getHolder().isEnum()
- && resolvedMethod.getDefinition().isInstanceInitializer()) {
+ DexProgramClass candidateOrNull =
+ getEnumUnboxingCandidateOrNull(mostAccurateTarget.getHolderType());
+ if (candidateOrNull != null && resolvedMethod.getDefinition().isInstanceInitializer()) {
// The enum instance initializer is only allowed to be called from an initializer of the
// enum itself.
- if (getEnumUnboxingCandidateOrNull(code.context().getHolderType())
- != getEnumUnboxingCandidateOrNull(mostAccurateTarget.getHolderType())
+ if (getEnumUnboxingCandidateOrNull(code.context().getHolderType()) != candidateOrNull
|| !context.getDefinition().isInitializer()) {
return Reason.INVALID_INIT;
}
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 86709f7..c0a7655 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
@@ -4,7 +4,6 @@
package com.android.tools.r8.ir.optimize.enums;
-import static com.android.tools.r8.utils.MapUtils.ignoreKey;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexClass;
@@ -14,16 +13,14 @@
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexProto;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.ImmediateProgramSubtypingInfo;
import com.android.tools.r8.graph.lens.GraphLens;
import com.android.tools.r8.ir.optimize.enums.eligibility.Reason;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.shaking.KeepInfoCollection;
import com.android.tools.r8.utils.InternalOptions;
import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Sets;
-import java.util.IdentityHashMap;
-import java.util.Map;
-import java.util.Set;
+import java.util.List;
class EnumUnboxingCandidateAnalysis {
@@ -35,12 +32,9 @@
private final AppView<AppInfoWithLiveness> appView;
private final EnumUnboxerImpl enumUnboxer;
private final DexItemFactory factory;
- private EnumUnboxingCandidateInfoCollection enumToUnboxCandidates =
+ private final EnumUnboxingCandidateInfoCollection enumToUnboxCandidates =
new EnumUnboxingCandidateInfoCollection();
- private final Map<DexType, Set<DexProgramClass>> enumSubclasses = new IdentityHashMap<>();
- private final Set<DexType> ineligibleCandidates = Sets.newIdentityHashSet();
-
EnumUnboxingCandidateAnalysis(AppView<AppInfoWithLiveness> appView, EnumUnboxerImpl enumUnboxer) {
this.appView = appView;
this.enumUnboxer = enumUnboxer;
@@ -54,13 +48,13 @@
// disables the enum unboxer.
return enumToUnboxCandidates;
}
+ ImmediateProgramSubtypingInfo subtypingInfo = ImmediateProgramSubtypingInfo.create(appView);
+ // In recent Kotlin enum subclasses are not flagged as enum, so we cannot rely on the flag.
for (DexProgramClass clazz : appView.appInfo().classes()) {
- if (clazz.isEnum()) {
- analyzeEnum(graphLensForPrimaryOptimizationPass, clazz);
+ if (clazz.isEnum() && clazz.superType.isIdenticalTo(factory.enumType)) {
+ analyzeEnum(graphLensForPrimaryOptimizationPass, clazz, subtypingInfo);
}
}
- removeIneligibleCandidates();
- setEnumSubclassesOnCandidates();
removeEnumsInAnnotations();
removePinnedCandidates();
if (appView.options().protoShrinking().isProtoShrinkingEnabled()) {
@@ -70,42 +64,28 @@
return enumToUnboxCandidates;
}
- private void setEnumSubclassesOnCandidates() {
- enumToUnboxCandidates.forEachCandidateInfo(
- info -> {
- DexType type = info.getEnumClass().getType();
- enumToUnboxCandidates.setEnumSubclasses(
- type, enumSubclasses.getOrDefault(type, ImmutableSet.of()));
- });
- }
-
- private void removeIneligibleCandidates() {
- for (DexType ineligibleCandidate : ineligibleCandidates) {
- enumToUnboxCandidates.removeCandidate(ineligibleCandidate);
+ private void analyzeEnum(
+ GraphLens graphLensForPrimaryOptimizationPass,
+ DexProgramClass clazz,
+ ImmediateProgramSubtypingInfo subtypingInfo) {
+ if (!isSuperEnumUnboxingCandidate(clazz)) {
+ return;
}
- }
- @SuppressWarnings("ReferenceEquality")
- private void analyzeEnum(GraphLens graphLensForPrimaryOptimizationPass, DexProgramClass clazz) {
- if (clazz.superType == factory.enumType) {
- if (isSuperEnumUnboxingCandidate(clazz)) {
- enumToUnboxCandidates.addCandidate(appView, clazz, graphLensForPrimaryOptimizationPass);
+ List<DexProgramClass> subtypes = subtypingInfo.getSubclasses(clazz);
+ ImmutableSet.Builder<DexProgramClass> subEnumClassesBuilder = ImmutableSet.builder();
+ for (DexProgramClass subEnum : subtypes) {
+ if (!isSubEnumUnboxingCandidate(subEnum)) {
+ return;
}
- } else {
- if (isSubEnumUnboxingCandidate(clazz)
- && appView.options().testing.enableEnumWithSubtypesUnboxing) {
- enumSubclasses
- .computeIfAbsent(clazz.superType, ignoreKey(Sets::newIdentityHashSet))
- .add(clazz);
- } else {
- ineligibleCandidates.add(clazz.superType);
- }
+ subEnumClassesBuilder.add(subEnum);
}
+ enumToUnboxCandidates.addCandidate(
+ appView, clazz, subEnumClassesBuilder.build(), graphLensForPrimaryOptimizationPass);
}
@SuppressWarnings("ReferenceEquality")
private boolean isSubEnumUnboxingCandidate(DexProgramClass clazz) {
- assert clazz.isEnum();
boolean result = true;
// Javac does not seem to generate enums with more than a single subtype level.
// TODO(b/273910479): Stop using isEffectivelyFinal.
@@ -132,29 +112,14 @@
return result;
}
- @SuppressWarnings("ReferenceEquality")
private boolean isSuperEnumUnboxingCandidate(DexProgramClass clazz) {
assert clazz.isEnum();
-
- // This is used in debug mode, where we don't do quick returns to log all the reasons an enum
- // is not unboxed.
- boolean result = true;
-
- // TODO(b/271385332): Change this into an assert when legacy is removed.
- if (clazz.superType != factory.enumType) {
- if (!enumUnboxer.reportFailure(clazz, Reason.INVALID_LIBRARY_SUPERTYPE)) {
- return false;
- }
- result = false;
- }
-
+ assert clazz.superType.isIdenticalTo(factory.enumType);
if (clazz.instanceFields().size() > MAX_INSTANCE_FIELDS_FOR_UNBOXING) {
- if (!enumUnboxer.reportFailure(clazz, Reason.MANY_INSTANCE_FIELDS)) {
- return false;
- }
- result = false;
+ enumUnboxer.reportFailure(clazz, Reason.MANY_INSTANCE_FIELDS);
+ return false;
}
- return result;
+ return true;
}
private void removeEnumsInAnnotations() {
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 aaa428d..c2444a8 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
@@ -34,11 +34,16 @@
public void addCandidate(
AppView<AppInfoWithLiveness> appView,
DexProgramClass enumClass,
+ Set<DexProgramClass> subclasses,
GraphLens graphLensForPrimaryOptimizationPass) {
assert !enumTypeToInfo.containsKey(enumClass.type);
enumTypeToInfo.put(
enumClass.type,
- new EnumUnboxingCandidateInfo(appView, enumClass, graphLensForPrimaryOptimizationPass));
+ new EnumUnboxingCandidateInfo(
+ appView, enumClass, subclasses, graphLensForPrimaryOptimizationPass));
+ for (DexProgramClass subclass : subclasses) {
+ subEnumToSuperEnumMap.put(subclass.getType(), enumClass.getType());
+ }
}
public boolean hasSubtypes(DexType enumType) {
@@ -50,13 +55,6 @@
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) {
prunedMethods.add(method.getReference());
}
@@ -174,10 +172,12 @@
public EnumUnboxingCandidateInfo(
AppView<AppInfoWithLiveness> appView,
DexProgramClass enumClass,
+ Set<DexProgramClass> subclasses,
GraphLens graphLensForPrimaryOptimizationPass) {
assert enumClass != null;
assert appView.graphLens() == graphLensForPrimaryOptimizationPass;
this.enumClass = enumClass;
+ this.subclasses = subclasses;
this.methodDependencies =
LongLivedProgramMethodSetBuilder.createConcurrentForIdentitySet(
graphLensForPrimaryOptimizationPass);
@@ -188,10 +188,6 @@
return subclasses;
}
- public void setSubclasses(Set<DexProgramClass> subclasses) {
- this.subclasses = subclasses;
- }
-
public DexProgramClass getEnumClass() {
return enumClass;
}
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 56b05f6..3cf50f1 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
@@ -11,18 +11,13 @@
public abstract class Reason {
public static final Reason ELIGIBLE = new StringReason("ELIGIBLE");
- public static final Reason ACCESSIBILITY = new StringReason("ACCESSIBILITY");
public static final Reason ANNOTATION = new StringReason("ANNOTATION");
public static final Reason PINNED = new StringReason("PINNED");
public static final Reason DOWN_CAST = new StringReason("DOWN_CAST");
- public static final Reason INVALID_LIBRARY_SUPERTYPE =
- new StringReason("INVALID_LIBRARY_SUPERTYPE");
- public static final Reason SUBTYPES = new StringReason("SUBTYPES");
public static final Reason SUBENUM_SUBTYPES = new StringReason("SUBENUM_SUBTYPES");
public static final Reason SUBENUM_INVALID_HIERARCHY =
new StringReason("SUBENUM_INVALID_HIERARCHY");
public static final Reason SUBENUM_INSTANCE_FIELDS = new StringReason("SUBENUM_INSTANCE_FIELDS");
- public static final Reason SUBENUM_STATIC_MEMBER = new StringReason("SUBENUM_STATIC_MEMBER");
public static final Reason MANY_INSTANCE_FIELDS = new StringReason("MANY_INSTANCE_FIELDS");
public static final Reason DEFAULT_METHOD_INVOKE = new StringReason("DEFAULT_METHOD_INVOKE");
public static final Reason UNRESOLVABLE_FIELD = new StringReason("UNRESOLVABLE_FIELD");
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index b71ad68..97aadb3 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -2411,7 +2411,6 @@
public boolean enableSwitchToIfRewriting = true;
public boolean enableEnumUnboxingDebugLogs =
System.getProperty("com.android.tools.r8.enableEnumUnboxingDebugLogs") != null;
- public boolean enableEnumWithSubtypesUnboxing = true;
public boolean enableVerticalClassMergerLensAssertion = false;
public boolean forceRedundantConstNumberRemoval = false;
public boolean enableExperimentalDesugaredLibraryKeepRuleGenerator = false;
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/enummerging/BasicEnumMergingKeepSubtypeTest.java b/src/test/java/com/android/tools/r8/enumunboxing/enummerging/BasicEnumMergingKeepSubtypeTest.java
index 4f1d44e..c2446e9 100644
--- a/src/test/java/com/android/tools/r8/enumunboxing/enummerging/BasicEnumMergingKeepSubtypeTest.java
+++ b/src/test/java/com/android/tools/r8/enumunboxing/enummerging/BasicEnumMergingKeepSubtypeTest.java
@@ -42,7 +42,6 @@
.addKeepMainRule(Main.class)
.addKeepRules("-keep class " + SUBTYPE_NAME + " { public void method(); }")
.addKeepRules(enumKeepRules.getKeepRules())
- .addOptionsModification(opt -> opt.testing.enableEnumWithSubtypesUnboxing = true)
.addEnumUnboxingInspector(
inspector -> inspector.assertNotUnboxed(EnumWithVirtualOverride.class))
.addOptionsModification(opt -> enableEnumOptions(opt, enumValueOptimization))
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/enummerging/NoEnumFlagEnumMergingTest.java b/src/test/java/com/android/tools/r8/enumunboxing/enummerging/NoEnumFlagEnumMergingTest.java
new file mode 100644
index 0000000..61df4af
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/enumunboxing/enummerging/NoEnumFlagEnumMergingTest.java
@@ -0,0 +1,127 @@
+// 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.ToolHelper;
+import com.android.tools.r8.enumunboxing.EnumUnboxingTestBase;
+import com.android.tools.r8.graph.ClassAccessFlags;
+import com.android.tools.r8.references.Reference;
+import com.android.tools.r8.utils.DescriptorUtils;
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.stream.Collectors;
+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 NoEnumFlagEnumMergingTest 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 NoEnumFlagEnumMergingTest(
+ TestParameters parameters, boolean enumValueOptimization, EnumKeepRules enumKeepRules) {
+ this.parameters = parameters;
+ this.enumValueOptimization = enumValueOptimization;
+ this.enumKeepRules = enumKeepRules;
+ }
+
+ @Test
+ public void testEnumUnboxing() throws Exception {
+ testForR8(parameters.getBackend())
+ .addProgramFiles(getEnumSubtypesOrOtherInputs(false))
+ .addProgramClassFileData(getSubEnumProgramData(getEnumSubtypesOrOtherInputs(true)))
+ .addKeepMainRule(Main.class)
+ .addKeepRules(enumKeepRules.getKeepRules())
+ .addEnumUnboxingInspector(inspector -> inspector.assertUnboxed(MyEnum2Cases.class))
+ .enableInliningAnnotations()
+ .addOptionsModification(opt -> enableEnumOptions(opt, enumValueOptimization))
+ .setMinApi(parameters)
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("336", "74", "96", "44");
+ }
+
+ private List<Path> getEnumSubtypesOrOtherInputs(boolean enumSubtypes) throws IOException {
+ return ToolHelper.getClassFilesForInnerClasses(getClass()).stream()
+ .filter(path -> isMyEnum2CasesSubtype(path) == enumSubtypes)
+ .collect(Collectors.toList());
+ }
+
+ private boolean isMyEnum2CasesSubtype(Path c) {
+ return c.toString().contains("MyEnum2Cases") && !c.toString().endsWith("MyEnum2Cases.class");
+ }
+
+ private List<byte[]> getSubEnumProgramData(List<Path> input) {
+ // Some Kotlin enum subclasses don't have the enum flag set. See b/315186101.
+ return input.stream()
+ .map(
+ path -> {
+ try {
+ String subtype = path.getFileName().toString();
+ String subtypeNoClass =
+ subtype.substring(0, (subtype.length() - ".class".length()));
+ String descr =
+ DescriptorUtils.replaceSimpleClassNameInDescriptor(
+ DescriptorUtils.javaTypeToDescriptor(MyEnum2Cases.class.getTypeName()),
+ subtypeNoClass);
+ return transformer(path, Reference.classFromDescriptor(descr))
+ .setAccessFlags(ClassAccessFlags::unsetEnum)
+ .transform();
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+ })
+ .collect(Collectors.toList());
+ }
+
+ enum MyEnum2Cases {
+ A(8) {
+ @NeverInline
+ @Override
+ public long operate(long another) {
+ return num * another;
+ }
+ },
+ B(32) {
+ @NeverInline
+ @Override
+ public long operate(long another) {
+ return num + another;
+ }
+ };
+ final long num;
+
+ MyEnum2Cases(long num) {
+ this.num = num;
+ }
+
+ public abstract long operate(long another);
+ }
+
+ static class Main {
+
+ public static void main(String[] args) {
+ System.out.println(MyEnum2Cases.A.operate(42));
+ System.out.println(MyEnum2Cases.B.operate(42));
+ System.out.println(indirect(MyEnum2Cases.A));
+ System.out.println(indirect(MyEnum2Cases.B));
+ }
+
+ @NeverInline
+ public static long indirect(MyEnum2Cases e) {
+ return e.operate(12);
+ }
+ }
+}
