commit d107b1d56e3a91a2aaea2837f4a33bd72eba238b
author Clément Béra <clementbera@google.com> Fri Aug 25 09:24:46 2023 +0200
committer Clément Béra <clementbera@google.com> Mon Aug 28 07:08:23 2023 +0000
tree 68ecaf6e43e51773b916a8d5f55c4ef8cac1bb4e
parent 19bbb13ae71312c7468ef07962993ae36070592b

Fix enum merging with single enum instance

Bug: b/297158103
Change-Id: I83ae40ec48020787c02d44cc277f2e51a9f0b43e

src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingTreeFixer.java

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 a0d4d36..bcaf48b 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
@@ -664,6 +664,7 @@
     }
     if (superMethod == null || subimplementations.isEmpty()) {
       // No emulated dispatch is required, just move everything.
+      // If an abstract method with no implementors is found, effectively don't do anything.
       if (superMethod != null && !superMethod.getAccessFlags().isAbstract()) {
         assert superMethod.isProgramMethod();
         directMoveAndMap(localUtilityClass, localUtilityMethods, superMethod.asProgramMethod());
@@ -673,6 +674,13 @@
       }
       return;
     }
+    if (superMethod.getDefinition().isAbstract() && subimplementations.size() == 1) {
+      // No emulated dispatch is required, just forward everything to the unique implementation.
+      ProgramMethod override = subimplementations.iterator().next();
+      DexMethod uniqueUtility = directMoveAndMap(localUtilityClass, localUtilityMethods, override);
+      lensBuilder.mapToDispatch(superMethod.getReference(), uniqueUtility);
+      return;
+    }
     // These methods require emulated dispatch.
     emulatedDispatchMoveAndMap(
         localUtilityClass, localUtilityMethods, superMethod, subimplementations);
@@ -736,7 +744,7 @@
     }
   }
 
-  private void directMoveAndMap(
+  private DexMethod directMoveAndMap(
       LocalEnumUnboxingUtilityClass localUtilityClass,
       Map<DexMethod, DexEncodedMethod> localUtilityMethods,
       ProgramMethod method) {
@@ -745,6 +753,7 @@
         installLocalUtilityMethod(localUtilityClass, localUtilityMethods, method);
     assert utilityMethod != null;
     lensBuilder.moveAndMap(method.getReference(), utilityMethod, method.getDefinition().isStatic());
+    return utilityMethod;
   }
 
   public void recordEmulatedDispatch(DexMethod from, DexMethod move, DexMethod dispatch) {

src/test/java/com/android/tools/r8/enumunboxing/enummerging/AbstractEnumMergingTest.java

diff --git a/src/test/java/com/android/tools/r8/enumunboxing/enummerging/AbstractEnumMergingTest.java b/src/test/java/com/android/tools/r8/enumunboxing/enummerging/AbstractEnumMergingTest.java
index be3e6b3..e2103e4 100644
--- a/src/test/java/com/android/tools/r8/enumunboxing/enummerging/AbstractEnumMergingTest.java
+++ b/src/test/java/com/android/tools/r8/enumunboxing/enummerging/AbstractEnumMergingTest.java
@@ -37,15 +37,16 @@
         .addInnerClasses(getClass())
         .addKeepMainRule(Main.class)
         .addKeepRules(enumKeepRules.getKeepRules())
-        .addEnumUnboxingInspector(inspector -> inspector.assertUnboxed(MyEnum.class))
+        .addEnumUnboxingInspector(
+            inspector -> inspector.assertUnboxed(MyEnum2Cases.class, MyEnum1Case.class))
         .enableInliningAnnotations()
         .addOptionsModification(opt -> enableEnumOptions(opt, enumValueOptimization))
         .setMinApi(parameters)
         .run(parameters.getRuntime(), Main.class)
-        .assertSuccessWithOutputLines("336", "74", "96", "44");
+        .assertSuccessWithOutputLines("336", "74", "96", "44", "336", "56");
   }
 
-  enum MyEnum {
+  enum MyEnum2Cases {
     A(8) {
       @NeverInline
       @Override
@@ -62,7 +63,24 @@
     };
     final long num;
 
-    MyEnum(long num) {
+    MyEnum2Cases(long num) {
+      this.num = num;
+    }
+
+    public abstract long operate(long another);
+  }
+
+  enum MyEnum1Case {
+    A(8) {
+      @NeverInline
+      @Override
+      public long operate(long another) {
+        return num * another;
+      }
+    };
+    final long num;
+
+    MyEnum1Case(long num) {
       this.num = num;
     }
 
@@ -72,15 +90,23 @@
   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(indirect(MyEnum.A));
-      System.out.println(indirect(MyEnum.B));
+      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));
+
+      System.out.println(MyEnum1Case.A.operate(42));
+      System.out.println(indirect(MyEnum1Case.A));
     }
 
     @NeverInline
-    public static long indirect(MyEnum e) {
+    public static long indirect(MyEnum2Cases e) {
       return e.operate(12);
     }
+
+    @NeverInline
+    public static long indirect(MyEnum1Case e) {
+      return e.operate(7);
+    }
   }
 }