Fix enum unboxer reservation state
Bug: b/271385332
Change-Id: Iad7d8992d897efe98a035c4b8939d127252a9bbb
diff --git a/src/main/java/com/android/tools/r8/graph/fixup/ConcurrentMethodFixup.java b/src/main/java/com/android/tools/r8/graph/fixup/ConcurrentMethodFixup.java
index 6afd404..476361d 100644
--- a/src/main/java/com/android/tools/r8/graph/fixup/ConcurrentMethodFixup.java
+++ b/src/main/java/com/android/tools/r8/graph/fixup/ConcurrentMethodFixup.java
@@ -15,6 +15,7 @@
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.ImmediateProgramSubtypingInfo;
+import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.optimize.argumentpropagation.utils.ProgramClassesBidirectedGraph;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.shaking.KeepMethodInfo;
@@ -71,6 +72,9 @@
// When a class is fixed-up, it is guaranteed that its supertype and interfaces were processed
// before. In addition, all interfaces are processed before any class is processed.
void fixupProgramClass(DexProgramClass clazz, MethodNamingUtility namingUtility);
+
+ // Answers true if the method should be reserved as itself.
+ boolean shouldReserveAsIfPinned(ProgramMethod method);
}
private void processConnectedProgramComponents(Set<DexProgramClass> classes) {
@@ -148,14 +152,19 @@
programClassFixer.fixupProgramClass(clazz, utility);
}
+ private boolean shouldReserveAsPinned(ProgramMethod method) {
+ KeepMethodInfo keepInfo = appView.getKeepInfo(method);
+ return !keepInfo.isOptimizationAllowed(appView.options())
+ || !keepInfo.isShrinkingAllowed(appView.options())
+ || programClassFixer.shouldReserveAsIfPinned(method);
+ }
+
private MethodNamingUtility createMethodNamingUtility(
BiMap<DexMethodSignature, DexMethodSignature> inheritedSignatures, DexProgramClass clazz) {
BiMap<DexMethod, DexMethod> localSignatures = HashBiMap.create();
clazz.forEachProgramInstanceInitializer(
method -> {
- KeepMethodInfo keepInfo = appView.getKeepInfo(method);
- if (!keepInfo.isOptimizationAllowed(appView.options())
- || !keepInfo.isShrinkingAllowed(appView.options())) {
+ if (shouldReserveAsPinned(method)) {
localSignatures.put(method.getReference(), method.getReference());
}
});
@@ -172,9 +181,7 @@
clazz.forEachProgramMethodMatching(
m -> !m.isInstanceInitializer(),
method -> {
- KeepMethodInfo keepInfo = appView.getKeepInfo(method);
- if (!keepInfo.isOptimizationAllowed(appView.options())
- || !keepInfo.isShrinkingAllowed(appView.options())) {
+ if (shouldReserveAsPinned(method)) {
componentSignatures.put(method.getMethodSignature(), method.getMethodSignature());
}
});
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 de2b9cf..06185fc 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
@@ -171,6 +171,15 @@
}
@Override
+ public boolean shouldReserveAsIfPinned(ProgramMethod method) {
+ DexProto oldProto = method.getProto();
+ DexProto newProto = fixupProto(oldProto);
+ // We don't track nor reprocess dependencies of unchanged methods so we have to maintain them
+ // with the same signature.
+ return oldProto == newProto;
+ }
+
+ @Override
public void fixupProgramClass(DexProgramClass clazz, MethodNamingUtility utility) {
if (enumDataMap.isSuperUnboxedEnum(clazz.getType())) {
// Clear the initializers and move the other methods to the new location.
@@ -804,14 +813,17 @@
DexEncodedMethod method, MethodNamingUtility utility) {
DexProto oldProto = method.getProto();
DexProto newProto = fixupProto(oldProto);
- // Even if the protos are identical, we may generate collisions and decide to rename the
- // unchanged method. In most cases this is a no-op if the proto are identical.
+ if (oldProto == newProto) {
+ assert method.getReference()
+ == utility.nextUniqueMethod(
+ method, newProto, utilityClasses.getSharedUtilityClass().getType());
+ return method;
+ }
+
DexMethod newMethod =
utility.nextUniqueMethod(
method, newProto, utilityClasses.getSharedUtilityClass().getType());
- if (newMethod == method.getReference()) {
- return method;
- }
+ assert newMethod != method.getReference();
assert !method.isClassInitializer();
assert !method.isLibraryMethodOverride().isTrue()
: "Enum unboxing is changing the signature of a library override in a non unboxed class.";
@@ -822,24 +834,15 @@
RewrittenPrototypeDescription prototypeChanges =
lensBuilder.moveAndMap(
method.getReference(), newMethod, isStatic, isStatic, extraUnusedNullParameters);
- DexEncodedMethod newEncodedMethod =
- method.toTypeSubstitutedMethod(
- newMethod,
- builder ->
- builder
- .fixupOptimizationInfo(
- appView, prototypeChanges.createMethodOptimizationInfoFixer())
- .setCompilationState(method.getCompilationState())
- .setIsLibraryMethodOverrideIf(
- method.isNonPrivateVirtualMethod(), OptionalBool.FALSE));
- if (!extraUnusedNullParameters.isEmpty() && method.getCode() != null) {
- DexEncodedMethod.setDebugInfoWithExtraParameters(
- newEncodedMethod.getCode(),
- newMethod.getArity(),
- extraUnusedNullParameters.size(),
- appView);
- }
- return newEncodedMethod;
+ return method.toTypeSubstitutedMethod(
+ newMethod,
+ builder ->
+ builder
+ .fixupOptimizationInfo(
+ appView, prototypeChanges.createMethodOptimizationInfoFixer())
+ .setCompilationState(method.getCompilationState())
+ .setIsLibraryMethodOverrideIf(
+ method.isNonPrivateVirtualMethod(), OptionalBool.FALSE));
}
private DexEncodedField fixupEncodedField(DexEncodedField encodedField) {
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/OverloadingEnumUnboxingTest.java b/src/test/java/com/android/tools/r8/enumunboxing/OverloadingEnumUnboxingTest.java
index d9e84f4..5d47744 100644
--- a/src/test/java/com/android/tools/r8/enumunboxing/OverloadingEnumUnboxingTest.java
+++ b/src/test/java/com/android/tools/r8/enumunboxing/OverloadingEnumUnboxingTest.java
@@ -45,7 +45,6 @@
.enableInliningAnnotations()
.addOptionsModification(opt -> enableEnumOptions(opt, enumValueOptimization))
.setMinApi(parameters)
- .compile()
.run(parameters.getRuntime(), classToTest)
.assertSuccess()
.inspectStdOut(this::assertLines2By2Correct);
@@ -83,119 +82,143 @@
@NeverInline
private static void constructorTest() {
- new TestClass(42);
- System.out.println("42");
+ constructorOutline();
new TestClass(MyEnum1.A);
- System.out.println("0");
+ System.out.println("1");
new TestClass(MyEnum1.B);
- System.out.println("1");
+ System.out.println("2");
new TestClass(MyEnum2.A);
- System.out.println("0");
+ System.out.println("2");
new TestClass(MyEnum2.B);
- System.out.println("1");
+ System.out.println("3");
new TestClass(MyEnum3.A);
- System.out.println("0");
+ System.out.println("3");
new TestClass(MyEnum3.B);
- System.out.println("1");
+ System.out.println("4");
+ }
+
+ @NeverInline
+ private static void constructorOutline() {
+ // This method is outlined so it is not reprocessed in the second round of IR processing.
+ new TestClass(38);
+ System.out.println("42");
+ new TestClass(40);
+ System.out.println("44");
}
@NeverInline
private static void staticTest() {
- staticMethod(42);
- System.out.println("42");
+ staticOutline();
staticMethod(MyEnum1.A);
- System.out.println("0");
+ System.out.println("1");
staticMethod(MyEnum1.B);
- System.out.println("1");
+ System.out.println("2");
staticMethod(MyEnum2.A);
- System.out.println("0");
+ System.out.println("2");
staticMethod(MyEnum2.B);
- System.out.println("1");
+ System.out.println("3");
staticMethod(MyEnum3.A);
- System.out.println("0");
+ System.out.println("3");
staticMethod(MyEnum3.B);
- System.out.println("1");
+ System.out.println("4");
+ }
+
+ @NeverInline
+ private static void staticOutline() {
+ // This method is outlined so it is not reprocessed in the second round of IR processing.
+ staticMethod(38);
+ System.out.println("42");
+ staticMethod(40);
+ System.out.println("44");
}
@NeverInline
private static void virtualTest() {
TestClass testClass = new TestClass();
- testClass.virtualMethod(42);
- System.out.println("42");
+ virtualOutline(testClass);
testClass.virtualMethod(MyEnum1.A);
- System.out.println("0");
+ System.out.println("1");
testClass.virtualMethod(MyEnum1.B);
- System.out.println("1");
+ System.out.println("2");
testClass.virtualMethod(MyEnum2.A);
- System.out.println("0");
+ System.out.println("2");
testClass.virtualMethod(MyEnum2.B);
- System.out.println("1");
+ System.out.println("3");
testClass.virtualMethod(MyEnum3.A);
- System.out.println("0");
+ System.out.println("3");
testClass.virtualMethod(MyEnum3.B);
- System.out.println("1");
+ System.out.println("4");
+ }
+
+ @NeverInline
+ private static void virtualOutline(TestClass testClass) {
+ // This method is outlined so it is not reprocessed in the second round of IR processing.
+ testClass.virtualMethod(38);
+ System.out.println("42");
+ testClass.virtualMethod(40);
+ System.out.println("44");
}
public TestClass() {}
@NeverInline
public TestClass(MyEnum1 e1) {
- System.out.println(e1.ordinal());
+ System.out.println(e1.ordinal() + 1);
}
@NeverInline
public TestClass(MyEnum2 e2) {
- System.out.println(e2.ordinal());
+ System.out.println(e2.ordinal() + 2);
}
@NeverInline
public TestClass(MyEnum3 e3) {
- System.out.println(e3.ordinal());
+ System.out.println(e3.ordinal() + 3);
}
@NeverInline
public TestClass(int i) {
- System.out.println(i);
+ System.out.println(i + 4);
}
@NeverInline
public void virtualMethod(MyEnum1 e1) {
- System.out.println(e1.ordinal());
+ System.out.println(e1.ordinal() + 1);
}
@NeverInline
public void virtualMethod(MyEnum2 e2) {
- System.out.println(e2.ordinal());
+ System.out.println(e2.ordinal() + 2);
}
@NeverInline
public void virtualMethod(MyEnum3 e3) {
- System.out.println(e3.ordinal());
+ System.out.println(e3.ordinal() + 3);
}
@NeverInline
public void virtualMethod(int i) {
- System.out.println(i);
+ System.out.println(i + 4);
}
@NeverInline
public static void staticMethod(MyEnum1 e1) {
- System.out.println(e1.ordinal());
+ System.out.println(e1.ordinal() + 1);
}
@NeverInline
public static void staticMethod(MyEnum2 e2) {
- System.out.println(e2.ordinal());
+ System.out.println(e2.ordinal() + 2);
}
@NeverInline
public static void staticMethod(MyEnum3 e3) {
- System.out.println(e3.ordinal());
+ System.out.println(e3.ordinal() + 3);
}
@NeverInline
public static void staticMethod(int i) {
- System.out.println(i);
+ System.out.println(i + 4);
}
}
}
