Fix Non determinism in enum emulated dispatch
Bug: b/271385332
Change-Id: I1a36eb74521bee20dd7120ae70d1d20beada1503
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 6e75300..c5ce773 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
@@ -68,10 +68,11 @@
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 it.unimi.dsi.fastutil.ints.Int2ObjectLinkedOpenHashMap;
+import it.unimi.dsi.fastutil.ints.Int2ObjectSortedMap;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Comparator;
import java.util.IdentityHashMap;
import java.util.LinkedHashMap;
import java.util.List;
@@ -643,8 +644,8 @@
LocalEnumUnboxingUtilityClass localUtilityClass,
Map<DexMethod, DexEncodedMethod> localUtilityMethods,
DexClassAndMethod superMethod,
- ProgramMethodSet subimplementations) {
- assert !subimplementations.isEmpty();
+ ProgramMethodSet unorderedSubimplementations) {
+ assert !unorderedSubimplementations.isEmpty();
DexMethod superUtilityMethod;
if (superMethod.isProgramMethod()) {
superUtilityMethod =
@@ -657,7 +658,9 @@
superUtilityMethod = localUtilityClass.computeToStringUtilityMethod(factory);
}
Map<DexMethod, DexMethod> overrideToUtilityMethods = new IdentityHashMap<>();
- for (ProgramMethod subMethod : subimplementations) {
+ List<ProgramMethod> sortedSubimplementations = new ArrayList<>(unorderedSubimplementations);
+ sortedSubimplementations.sort(Comparator.comparing(ProgramMethod::getHolderType));
+ for (ProgramMethod subMethod : sortedSubimplementations) {
DexMethod subEnumLocalUtilityMethod =
installLocalUtilityMethod(localUtilityClass, localUtilityMethods, subMethod);
assert subEnumLocalUtilityMethod != null;
@@ -667,7 +670,7 @@
installDispatchMethod(
localUtilityClass,
localUtilityMethods,
- subimplementations.iterator().next(),
+ sortedSubimplementations.iterator().next(),
superUtilityMethod,
overrideToUtilityMethods)
.getReference();
@@ -719,7 +722,7 @@
fixupProto(factory.prependHolderToProto(representative.getReference())),
localUtilityClass.getType(),
newMethodSignature -> !localUtilityMethods.containsKey(newMethodSignature));
- Int2ObjectMap<DexMethod> methodMap = new Int2ObjectArrayMap<>();
+ Int2ObjectSortedMap<DexMethod> methodMap = new Int2ObjectLinkedOpenHashMap<>();
IdentityHashMap<DexType, DexMethod> typeToMethod = new IdentityHashMap<>();
map.forEach(
(methodReference, newMethodReference) ->
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 545e609..755f207 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
@@ -36,8 +36,7 @@
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 com.android.tools.r8.utils.IntBox;
-import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
+import it.unimi.dsi.fastutil.ints.Int2ObjectSortedMap;
import java.util.ArrayList;
import java.util.List;
import org.objectweb.asm.Opcodes;
@@ -73,13 +72,13 @@
private final GraphLens codeLens;
private final DexMethod superEnumMethod;
- private final Int2ObjectMap<DexMethod> methodMap;
+ private final Int2ObjectSortedMap<DexMethod> methodMap;
public EnumUnboxingMethodDispatchCfCodeProvider(
AppView<?> appView,
DexType holder,
DexMethod superEnumMethod,
- Int2ObjectMap<DexMethod> methodMap) {
+ Int2ObjectSortedMap<DexMethod> methodMap) {
super(appView, holder);
this.codeLens = appView.codeLens();
this.superEnumMethod = superEnumMethod;
@@ -107,10 +106,9 @@
for (DexType parameter : representative.getParameters()) {
frameBuilder.appendLocal(FrameType.initialized(parameter));
}
- IntBox index = new IntBox();
methodMap.forEach(
(unboxedEnumValue, method) -> {
- boolean lastCase = index.incrementAndGet() == methodMap.size() && !hasDefaultCase;
+ boolean lastCase = methodMap.lastIntKey() == unboxedEnumValue && !hasDefaultCase;
if (!lastCase) {
CfLabel dest = new CfLabel();
instructions.add(new CfLoad(ValueType.fromDexType(factory.intType), 0));
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/enummerging/DeterministicEnumMergingTest.java b/src/test/java/com/android/tools/r8/enumunboxing/enummerging/DeterministicEnumMergingTest.java
new file mode 100644
index 0000000..00bb1cf
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/enumunboxing/enummerging/DeterministicEnumMergingTest.java
@@ -0,0 +1,125 @@
+// 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.R8TestCompileResult;
+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 DeterministicEnumMergingTest 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 DeterministicEnumMergingTest(
+ TestParameters parameters, boolean enumValueOptimization, EnumKeepRules enumKeepRules) {
+ this.parameters = parameters;
+ this.enumValueOptimization = enumValueOptimization;
+ this.enumKeepRules = enumKeepRules;
+ }
+
+ @Test
+ public void testEnumUnboxingDeterminism() throws Exception {
+ R8TestCompileResult compile1 =
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(Main.class)
+ .addKeepRules(enumKeepRules.getKeepRules())
+ .addOptionsModification(opt -> opt.testing.enableEnumWithSubtypesUnboxing = true)
+ .addEnumUnboxingInspector(inspector -> inspector.assertUnboxed(MyEnum.class))
+ .enableInliningAnnotations()
+ .addOptionsModification(opt -> enableEnumOptions(opt, enumValueOptimization))
+ .setMinApi(parameters)
+ .compile();
+ R8TestCompileResult compile2 =
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(Main.class)
+ .addKeepRules(enumKeepRules.getKeepRules())
+ .addOptionsModification(opt -> opt.testing.enableEnumWithSubtypesUnboxing = true)
+ .addEnumUnboxingInspector(inspector -> inspector.assertUnboxed(MyEnum.class))
+ .enableInliningAnnotations()
+ .addOptionsModification(opt -> enableEnumOptions(opt, enumValueOptimization))
+ .setMinApi(parameters)
+ .compile();
+ assertIdenticalInspectors(compile1.inspector(), compile2.inspector());
+ compile1
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("336", "74", "22", "10794", "96", "44", "52", "3084");
+ }
+
+ enum MyEnum {
+ // The more cases, the highest the chance to trigger non determinism.
+ // The operate methods have to be moved in the same order, and the dispatch method has to
+ // be generated in the same order too.
+ A(8) {
+ @NeverInline
+ @Override
+ public long operate(long another) {
+ return num * another;
+ }
+ },
+ B(32) {
+ @NeverInline
+ @Override
+ public long operate(long another) {
+ return num + another;
+ }
+ },
+ C(64) {
+ @NeverInline
+ @Override
+ public long operate(long another) {
+ return num - another;
+ }
+ },
+ D(256) {
+
+ @NeverInline
+ @Override
+ public long operate(long another) {
+ return num * another + another;
+ }
+ };
+ final long num;
+
+ MyEnum(long num) {
+ this.num = num;
+ }
+
+ public abstract long operate(long another);
+ }
+
+ static class Main {
+
+ public static void main(String[] args) {
+ System.out.println(MyEnum.A.operate(42));
+ System.out.println(MyEnum.B.operate(42));
+ System.out.println(MyEnum.C.operate(42));
+ System.out.println(MyEnum.D.operate(42));
+ System.out.println(indirect(MyEnum.A));
+ System.out.println(indirect(MyEnum.B));
+ System.out.println(indirect(MyEnum.C));
+ System.out.println(indirect(MyEnum.D));
+ }
+
+ @NeverInline
+ public static long indirect(MyEnum e) {
+ return e.operate(12);
+ }
+ }
+}
