Replace calls to Enum.ordinal() on constant-like enum fields
Bug: 135667497
Test: tools/test.py --no-internal -v *EnumOptimizationTest*
Change-Id: I815668f17fc9c7f1c31ad578a77b45f09343b363
diff --git a/src/main/java/com/android/tools/r8/D8Command.java b/src/main/java/com/android/tools/r8/D8Command.java
index f6469b0..f427e0f 100644
--- a/src/main/java/com/android/tools/r8/D8Command.java
+++ b/src/main/java/com/android/tools/r8/D8Command.java
@@ -263,7 +263,7 @@
assert !internal.enableHorizontalClassMerging;
assert !internal.enableVerticalClassMerging;
assert !internal.enableClassStaticizer;
- assert !internal.enableSwitchMapRemoval;
+ assert !internal.enableEnumValueOptimization;
assert !internal.outline.enabled;
assert !internal.enableValuePropagation;
assert !internal.enableLambdaMerging;
diff --git a/src/main/java/com/android/tools/r8/GenerateMainDexListCommand.java b/src/main/java/com/android/tools/r8/GenerateMainDexListCommand.java
index 5eb0151..a79167a 100644
--- a/src/main/java/com/android/tools/r8/GenerateMainDexListCommand.java
+++ b/src/main/java/com/android/tools/r8/GenerateMainDexListCommand.java
@@ -227,7 +227,7 @@
internal.mainDexListConsumer = mainDexListConsumer;
internal.mainDexKeptGraphConsumer = mainDexKeptGraphConsumer;
internal.minimalMainDex = internal.debug;
- internal.enableSwitchMapRemoval = false;
+ internal.enableEnumValueOptimization = false;
internal.enableInlining = false;
return internal;
}
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index d907a81..a8afcfc 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -531,7 +531,7 @@
assert appView.dexItemFactory().verifyNoCachedTypeLatticeElements();
// Collect switch maps and ordinals maps.
- if (options.enableSwitchMapRemoval) {
+ if (options.enableEnumValueOptimization) {
appViewWithLiveness.setAppInfo(new SwitchMapCollector(appViewWithLiveness).run());
appViewWithLiveness.setAppInfo(new EnumOrdinalMapCollector(appViewWithLiveness).run());
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
index 2bd69e5..9901957 100644
--- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
+++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -655,6 +655,7 @@
public class EnumMethods {
public DexMethod valueOf;
+ public DexMethod ordinal;
private EnumMethods() {
valueOf =
@@ -663,6 +664,12 @@
valueOfMethodName,
enumDescriptor,
new DexString[] {classDescriptor, stringDescriptor});
+ ordinal =
+ createMethod(
+ enumDescriptor,
+ ordinalMethodName,
+ intDescriptor,
+ DexString.EMPTY_ARRAY);
}
}
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 fb6edd2..26ea6c5 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
@@ -992,7 +992,7 @@
memberValuePropagation.rewriteWithConstantValues(
code, method.method.holder, isProcessedConcurrently);
}
- if (options.enableSwitchMapRemoval) {
+ if (options.enableEnumValueOptimization) {
assert appView.enableWholeProgramOptimizations();
codeRewriter.removeSwitchMaps(code);
}
@@ -1052,6 +1052,11 @@
assert code.verifyTypes(appView);
codeRewriter.removeTrivialCheckCastAndInstanceOfInstructions(code);
+ if (options.enableEnumValueOptimization) {
+ assert appView.enableWholeProgramOptimizations();
+ codeRewriter.rewriteConstantEnumOrdinal(code);
+ }
+
codeRewriter.rewriteLongCompareAndRequireNonNull(code, options);
codeRewriter.commonSubexpressionElimination(code);
codeRewriter.simplifyArrayConstruction(code);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
index 5c6240e..bb41249 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
@@ -64,6 +64,7 @@
import com.android.tools.r8.ir.code.Invoke;
import com.android.tools.r8.ir.code.InvokeDirect;
import com.android.tools.r8.ir.code.InvokeMethod;
+import com.android.tools.r8.ir.code.InvokeMethodWithReceiver;
import com.android.tools.r8.ir.code.InvokeNewArray;
import com.android.tools.r8.ir.code.InvokeStatic;
import com.android.tools.r8.ir.code.InvokeVirtual;
@@ -3620,6 +3621,51 @@
return true;
}
+ public void rewriteConstantEnumOrdinal(IRCode code) {
+ InstructionIterator iterator = code.instructionIterator();
+ while (iterator.hasNext()) {
+ Instruction current = iterator.next();
+
+ if (!current.isInvokeMethodWithReceiver()) {
+ continue;
+ }
+ InvokeMethodWithReceiver methodWithReceiver = current.asInvokeMethodWithReceiver();
+ if (methodWithReceiver.getInvokedMethod() != dexItemFactory.enumMethods.ordinal) {
+ continue;
+ }
+
+ Value receiver = methodWithReceiver.getReceiver().getAliasedValue();
+ if (receiver.isPhi()) {
+ continue;
+ }
+ Instruction definition = receiver.getDefinition();
+ if (!definition.isStaticGet()) {
+ continue;
+ }
+ DexField enumField = definition.asStaticGet().getField();
+
+ Reference2IntMap<DexField> ordinalMap =
+ appView.appInfo().withLiveness().getOrdinalsMapFor(enumField.type);
+ if (ordinalMap == null) {
+ continue;
+ }
+
+ // The receiver value is identified as being from a constant enum field lookup by the fact
+ // that it is a static-get to a field whose type is the same as the enclosing class (which
+ // is known to be an enum type). An enum may still define a static field using the enum type
+ // so ensure the field is present in the ordinal map for final validation.
+ if (!ordinalMap.containsKey(enumField)) {
+ continue;
+ }
+ int ordinalValue = ordinalMap.getInt(enumField);
+
+ Value outValue = methodWithReceiver.outValue();
+ iterator.replaceCurrentInstruction(new ConstNumber(outValue, ordinalValue));
+ }
+
+ assert code.isConsistentSSA();
+ }
+
public void rewriteLongCompareAndRequireNonNull(IRCode code, InternalOptions options) {
if (options.canUseJava7CompareAndObjectsOperations()) {
return;
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 91da309..14a7ac1 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -144,7 +144,7 @@
enableUninstantiatedTypeOptimization = false;
enableUnusedArgumentRemoval = false;
outline.enabled = false;
- enableSwitchMapRemoval = false;
+ enableEnumValueOptimization = false;
enableValuePropagation = false;
enableSideEffectAnalysis = false;
enableTreeShakingOfLibraryMethodOverrides = false;
@@ -205,7 +205,7 @@
// GMS Core.
public int inliningControlFlowResolutionBlocksThreshold = 15;
public boolean enableStringSwitchConversion = false;
- public boolean enableSwitchMapRemoval = true;
+ public boolean enableEnumValueOptimization = true;
public final OutlineOptions outline = new OutlineOptions();
public boolean enableValuePropagation = true;
public boolean enableUninstantiatedTypeOptimization = true;
diff --git a/src/test/java/com/android/tools/r8/kotlin/optimize/switches/KotlinEnumSwitchTest.java b/src/test/java/com/android/tools/r8/kotlin/optimize/switches/KotlinEnumSwitchTest.java
index ff5d0b5..374f91d 100644
--- a/src/test/java/com/android/tools/r8/kotlin/optimize/switches/KotlinEnumSwitchTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/optimize/switches/KotlinEnumSwitchTest.java
@@ -41,7 +41,8 @@
testForR8(parameters.getBackend())
.addProgramFiles(Paths.get(ToolHelper.EXAMPLES_KOTLIN_BUILD_DIR, "enumswitch.jar"))
.addKeepMainRule("enumswitch.EnumSwitchKt")
- .addOptionsModification(options -> options.enableSwitchMapRemoval = enableSwitchMapRemoval)
+ .addOptionsModification(
+ options -> options.enableEnumValueOptimization = enableSwitchMapRemoval)
.setMinApi(parameters.getRuntime())
.noMinification()
.compile()
diff --git a/src/test/java/com/android/tools/r8/rewrite/enums/EnumOptimizationTest.java b/src/test/java/com/android/tools/r8/rewrite/enums/EnumOptimizationTest.java
new file mode 100644
index 0000000..40afc97
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/rewrite/enums/EnumOptimizationTest.java
@@ -0,0 +1,188 @@
+// Copyright (c) 2017, 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.rewrite.enums;
+
+import static com.android.tools.r8.ToolHelper.getDefaultAndroidJar;
+import static java.util.Collections.emptyList;
+import static java.util.stream.Collectors.toList;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.ForceInline;
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.utils.BooleanUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.InstructionSubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+/**
+ * Tests checking that Enum.ordinal() calls on enum fields are rewritten into a constant integer.
+ */
+@RunWith(Parameterized.class)
+public class EnumOptimizationTest extends TestBase {
+ private final boolean enableOptimization;
+ private final TestParameters parameters;
+
+ @Parameters(name = "{1}, enable enum optimization: {0}")
+ public static List<Object[]> data() {
+ return buildParameters(BooleanUtils.values(), getTestParameters().withAllRuntimes().build());
+ }
+
+ public EnumOptimizationTest(boolean enableOptimization, TestParameters parameters) {
+ this.enableOptimization = enableOptimization;
+ this.parameters = parameters;
+ }
+
+ static class Ordinals {
+ enum Number {
+ ONE, TWO;
+
+ public static final Direction DOWN = Direction.DOWN;
+ public static final Number DEFAULT = TWO;
+ }
+
+ enum Direction {
+ UP, DOWN
+ }
+
+ @NeverInline
+ static long simple() {
+ return Number.TWO.ordinal();
+ }
+
+ @NeverInline
+ static long local() {
+ Number two = Number.TWO;
+ return two.ordinal();
+ }
+
+ @NeverInline
+ static String multipleUsages() {
+ Number two = Number.TWO;
+ return two.name() + two.ordinal();
+ }
+
+ @NeverInline
+ static long inlined() {
+ return inlined2(Number.TWO);
+ }
+ @ForceInline
+ private static long inlined2(Number number) {
+ return number.ordinal();
+ }
+
+ @NeverInline
+ static long libraryType() {
+ return TimeUnit.SECONDS.ordinal();
+ }
+
+ @NeverInline
+ static long wrongTypeStaticField() {
+ return Number.DOWN.ordinal();
+ }
+
+ @NeverInline
+ static long nonValueStaticField() {
+ return Number.DEFAULT.ordinal();
+ }
+
+ @NeverInline
+ static long phi(boolean value) {
+ Number number = Number.ONE;
+ if (value) {
+ number = Number.TWO;
+ }
+ return number.ordinal();
+ }
+
+ @NeverInline
+ static long nonStaticGet() {
+ return new Ordinals().two.ordinal();
+ }
+ private final Number two = Number.TWO;
+
+ public static void main(String[] args) {
+ System.out.println(simple());
+ System.out.println(local());
+ System.out.println(multipleUsages());
+ System.out.println(inlined());
+ System.out.println(libraryType());
+ System.out.println(wrongTypeStaticField());
+ System.out.println(nonValueStaticField());
+ System.out.println(phi(true));
+ System.out.println(nonStaticGet());
+ }
+ }
+
+ @Test public void ordinals() throws Exception {
+ testForR8(parameters.getBackend())
+ .addLibraryFiles(getDefaultAndroidJar())
+ .addProgramClassesAndInnerClasses(Ordinals.class)
+ .addKeepMainRule(Ordinals.class)
+ .noMinification()
+ .enableInliningAnnotations()
+ .addOptionsModification(options -> options.enableEnumValueOptimization = enableOptimization)
+ .setMinApi(parameters.getRuntime())
+ .compile()
+ .inspect(this::inspectOrdinals)
+ .run(parameters.getRuntime(), Ordinals.class)
+ .assertSuccessWithOutputLines("1", "1", "TWO1", "1", "3", "1", "1", "1", "1");
+ }
+
+ private void inspectOrdinals(CodeInspector inspector) {
+ ClassSubject clazz = inspector.clazz(Ordinals.class);
+ assertTrue(clazz.isPresent());
+
+ if (enableOptimization) {
+ assertOrdinalReplacedWithConst(clazz.uniqueMethodWithName("simple"), 1);
+ assertOrdinalReplacedWithConst(clazz.uniqueMethodWithName("local"), 1);
+ assertOrdinalReplacedWithConst(clazz.uniqueMethodWithName("multipleUsages"), 1);
+ assertOrdinalReplacedWithConst(clazz.uniqueMethodWithName("inlined"), 1);
+ } else {
+ assertOrdinalWasNotReplaced(clazz.uniqueMethodWithName("simple"));
+ assertOrdinalWasNotReplaced(clazz.uniqueMethodWithName("local"));
+ assertOrdinalWasNotReplaced(clazz.uniqueMethodWithName("multipleUsages"));
+ assertOrdinalWasNotReplaced(clazz.uniqueMethodWithName("inlined"));
+ }
+
+ assertOrdinalWasNotReplaced(clazz.uniqueMethodWithName("libraryType"));
+ assertOrdinalWasNotReplaced(clazz.uniqueMethodWithName("wrongTypeStaticField"));
+ assertOrdinalWasNotReplaced(clazz.uniqueMethodWithName("nonValueStaticField"));
+ assertOrdinalWasNotReplaced(clazz.uniqueMethodWithName("phi"));
+ assertOrdinalWasNotReplaced(clazz.uniqueMethodWithName("nonStaticGet"));
+ }
+
+ private static void assertOrdinalReplacedWithConst(MethodSubject method, int expectedConst) {
+ assertTrue(method.isPresent());
+ assertEquals(emptyList(), enumOrdinalInvokes(method));
+
+ long[] actualConst = method.streamInstructions()
+ .filter(InstructionSubject::isConstNumber)
+ .mapToLong(InstructionSubject::getConstNumber)
+ .toArray();
+ assertEquals(expectedConst, actualConst[0]);
+ }
+
+ private static void assertOrdinalWasNotReplaced(MethodSubject method) {
+ assertTrue(method.isPresent());
+ List<InstructionSubject> invokes = enumOrdinalInvokes(method);
+ assertEquals(invokes.toString(), 1, invokes.size());
+ }
+
+ private static List<InstructionSubject> enumOrdinalInvokes(MethodSubject method) {
+ return method.streamInstructions()
+ .filter(instruction -> instruction.isInvoke()
+ && instruction.getMethod().name.toString().equals("ordinal"))
+ .collect(toList());
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java
index ff48000..d7ea3ae 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/CfInstructionSubject.java
@@ -122,8 +122,7 @@
@Override
public boolean isConstNumber(long value) {
- return instruction instanceof CfConstNumber
- && ((CfConstNumber) instruction).getRawValue() == value;
+ return isConstNumber() && getConstNumber() == value;
}
@Override
@@ -147,6 +146,11 @@
return false;
}
+ @Override public long getConstNumber() {
+ assert isConstNumber();
+ return ((CfConstNumber) instruction).getRawValue();
+ }
+
@Override
public String getConstString() {
if (instruction instanceof CfConstString) {
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java
index 32e3f00..53fb02c 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/DexInstructionSubject.java
@@ -222,20 +222,7 @@
@Override
public boolean isConstNumber(long value) {
- if (!isConstNumber()) {
- return false;
- }
- if (instruction instanceof Const
- || instruction instanceof Const4
- || instruction instanceof Const16
- || instruction instanceof ConstHigh16) {
- return ((SingleConstant) instruction).decodedValue() == value;
- }
- assert instruction instanceof ConstWide
- || instruction instanceof ConstWide16
- || instruction instanceof ConstWide32
- || instruction instanceof ConstWideHigh16;
- return ((WideConstant) instruction).decodedValue() == value;
+ return isConstNumber() && getConstNumber() == value;
}
@Override
@@ -263,6 +250,15 @@
return instruction instanceof ConstStringJumbo;
}
+ @Override public long getConstNumber() {
+ assert isConstNumber();
+ if (instruction instanceof SingleConstant) {
+ return ((SingleConstant) instruction).decodedValue();
+ }
+ assert instruction instanceof WideConstant;
+ return ((WideConstant) instruction).decodedValue();
+ }
+
@Override
public String getConstString() {
if (instruction instanceof ConstString) {
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java
index 722aa5f..25e10f4 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java
@@ -50,6 +50,8 @@
boolean isJumboString();
+ long getConstNumber();
+
String getConstString();
boolean isConstClass();
