Replace all occurrences of SingleEnumValue by SingleFieldValue
Change-Id: If73c8ebda320fa61ace737c65709757c6abcdf4c
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedField.java b/src/main/java/com/android/tools/r8/graph/DexEncodedField.java
index 0d5c50d..4e3c645 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedField.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedField.java
@@ -129,6 +129,10 @@
return this;
}
+ public boolean isEnum() {
+ return accessFlags.isEnum();
+ }
+
public boolean isFinal() {
return accessFlags.isFinal();
}
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/ValueMayDependOnEnvironmentAnalysis.java b/src/main/java/com/android/tools/r8/ir/analysis/ValueMayDependOnEnvironmentAnalysis.java
index f417d65..f68aa3a 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/ValueMayDependOnEnvironmentAnalysis.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/ValueMayDependOnEnvironmentAnalysis.java
@@ -11,6 +11,7 @@
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.ir.analysis.value.AbstractValue;
import com.android.tools.r8.ir.code.ArrayPut;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.Instruction;
@@ -109,8 +110,13 @@
if (isConstantArrayThroughoutMethod(root, assumedNotToDependOnEnvironment)) {
return false;
}
- if (root.getAbstractValue(appView, context).isSingleEnumValue()) {
- return false;
+ AbstractValue abstractValue = root.getAbstractValue(appView, context);
+ if (abstractValue.isSingleFieldValue()) {
+ DexEncodedField field =
+ appView.definitionFor(abstractValue.asSingleFieldValue().getField());
+ if (field != null && field.isEnum()) {
+ return false;
+ }
}
if (isNewInstanceWithoutEnvironmentDependentFields(root, assumedNotToDependOnEnvironment)) {
return false;
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/StaticFieldValueAnalysis.java b/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/StaticFieldValueAnalysis.java
index e2ab677..596cd06 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/StaticFieldValueAnalysis.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/StaticFieldValueAnalysis.java
@@ -4,27 +4,17 @@
package com.android.tools.r8.ir.analysis.fieldvalueanalysis;
-import static com.android.tools.r8.ir.code.Opcodes.*;
-
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexEncodedMethod;
-import com.android.tools.r8.graph.DexField;
-import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.ir.analysis.type.ClassTypeElement;
import com.android.tools.r8.ir.analysis.type.Nullability;
import com.android.tools.r8.ir.analysis.type.TypeElement;
import com.android.tools.r8.ir.analysis.value.AbstractValue;
import com.android.tools.r8.ir.analysis.value.AbstractValueFactory;
-import com.android.tools.r8.ir.analysis.value.SingleEnumValue;
-import com.android.tools.r8.ir.analysis.value.UnknownValue;
-import com.android.tools.r8.ir.code.ArrayPut;
import com.android.tools.r8.ir.code.FieldInstruction;
import com.android.tools.r8.ir.code.IRCode;
-import com.android.tools.r8.ir.code.Instruction;
-import com.android.tools.r8.ir.code.InvokeDirect;
-import com.android.tools.r8.ir.code.NewInstance;
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.optimize.ClassInitializerDefaultsOptimization.ClassInitializerDefaultsResult;
import com.android.tools.r8.ir.optimize.info.OptimizationFeedback;
@@ -100,7 +90,7 @@
void updateFieldOptimizationInfo(DexEncodedField field, FieldInstruction fieldPut, Value value) {
// Abstract value.
Value root = value.getAliasedValue();
- AbstractValue abstractValue = computeAbstractValue(root);
+ AbstractValue abstractValue = root.getAbstractValue(appView, clazz.type);
if (abstractValue.isUnknown()) {
feedback.recordFieldHasAbstractValue(
field, appView, appView.abstractValueFactory().createSingleFieldValue(field.field));
@@ -123,113 +113,4 @@
feedback.markFieldHasDynamicLowerBoundType(field, dynamicLowerBoundType);
}
}
-
- private AbstractValue computeAbstractValue(Value value) {
- assert !value.hasAliasedValue();
- if (clazz.isEnum()) {
- SingleEnumValue singleEnumValue = getSingleEnumValue(value);
- if (singleEnumValue != null) {
- return singleEnumValue;
- }
- }
- if (!value.isPhi()) {
- return value.definition.getAbstractValue(appView, clazz.type);
- }
- return UnknownValue.getInstance();
- }
-
- /**
- * If {@param value} is defined by a new-instance instruction that instantiates the enclosing enum
- * class, and the value is assigned into exactly one static enum field on the enclosing enum
- * class, then returns a {@link SingleEnumValue} instance. Otherwise, returns {@code null}.
- *
- * <p>Note that enum constructors also store the newly instantiated enums in the {@code $VALUES}
- * array field on the enum. Therefore, this code also allows {@param value} to be stored into an
- * array as long as the array is identified as being the {@code $VALUES} array.
- */
- private SingleEnumValue getSingleEnumValue(Value value) {
- assert clazz.isEnum();
- assert !value.hasAliasedValue();
- if (value.isPhi() || !value.definition.isNewInstance()) {
- return null;
- }
-
- NewInstance newInstance = value.definition.asNewInstance();
- if (newInstance.clazz != clazz.type) {
- return null;
- }
-
- if (value.hasDebugUsers() || value.hasPhiUsers()) {
- return null;
- }
-
- DexEncodedField enumField = null;
- for (Instruction user : value.uniqueUsers()) {
- switch (user.opcode()) {
- case ARRAY_PUT:
- // Check that this is assigning the enum into the enum values array.
- ArrayPut arrayPut = user.asArrayPut();
- if (arrayPut.value().getAliasedValue() != value || !isEnumValuesArray(arrayPut.array())) {
- return null;
- }
- break;
-
- case INVOKE_DIRECT:
- // Check that this is the corresponding constructor call.
- InvokeDirect invoke = user.asInvokeDirect();
- if (!appView.dexItemFactory().isConstructor(invoke.getInvokedMethod())
- || invoke.getReceiver() != value) {
- return null;
- }
- break;
-
- case STATIC_PUT:
- DexEncodedField field = clazz.lookupStaticField(user.asStaticPut().getField());
- if (field != null && field.accessFlags.isEnum()) {
- if (enumField != null) {
- return null;
- }
- enumField = field;
- }
- break;
-
- default:
- return null;
- }
- }
-
- if (enumField == null) {
- return null;
- }
-
- return appView.abstractValueFactory().createSingleEnumValue(enumField.field);
- }
-
- private boolean isEnumValuesArray(Value value) {
- assert clazz.isEnum();
- DexItemFactory dexItemFactory = appView.dexItemFactory();
- DexField valuesField =
- dexItemFactory.createField(
- clazz.type,
- clazz.type.toArrayType(1, dexItemFactory),
- dexItemFactory.enumValuesFieldName);
-
- Value root = value.getAliasedValue();
- if (root.isPhi()) {
- return false;
- }
-
- Instruction definition = root.definition;
- if (definition.isNewArrayEmpty()) {
- for (Instruction user : root.aliasedUsers()) {
- if (user.isStaticPut() && user.asStaticPut().getField() == valuesField) {
- return true;
- }
- }
- } else if (definition.isStaticGet()) {
- return definition.asStaticGet().getField() == valuesField;
- }
-
- return false;
- }
}
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/value/AbstractValue.java b/src/main/java/com/android/tools/r8/ir/analysis/value/AbstractValue.java
index 404faff..9939980 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/value/AbstractValue.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/value/AbstractValue.java
@@ -44,14 +44,6 @@
return null;
}
- public boolean isSingleEnumValue() {
- return false;
- }
-
- public SingleEnumValue asSingleEnumValue() {
- return null;
- }
-
public boolean isSingleFieldValue() {
return false;
}
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/value/AbstractValueFactory.java b/src/main/java/com/android/tools/r8/ir/analysis/value/AbstractValueFactory.java
index 80ca063..bb467a5 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/value/AbstractValueFactory.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/value/AbstractValueFactory.java
@@ -13,7 +13,6 @@
private ConcurrentHashMap<DexType, SingleConstClassValue> singleConstClassValues =
new ConcurrentHashMap<>();
- private ConcurrentHashMap<DexField, SingleEnumValue> singleEnumValues = new ConcurrentHashMap<>();
private ConcurrentHashMap<DexField, SingleFieldValue> singleFieldValues =
new ConcurrentHashMap<>();
private ConcurrentHashMap<Long, SingleNumberValue> singleNumberValues = new ConcurrentHashMap<>();
@@ -24,10 +23,6 @@
return singleConstClassValues.computeIfAbsent(type, SingleConstClassValue::new);
}
- public SingleEnumValue createSingleEnumValue(DexField field) {
- return singleEnumValues.computeIfAbsent(field, SingleEnumValue::new);
- }
-
public SingleFieldValue createSingleFieldValue(DexField field) {
return singleFieldValues.computeIfAbsent(field, SingleFieldValue::new);
}
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/value/SingleEnumValue.java b/src/main/java/com/android/tools/r8/ir/analysis/value/SingleEnumValue.java
deleted file mode 100644
index 4beb525..0000000
--- a/src/main/java/com/android/tools/r8/ir/analysis/value/SingleEnumValue.java
+++ /dev/null
@@ -1,48 +0,0 @@
-// Copyright (c) 2019, 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.ir.analysis.value;
-
-import com.android.tools.r8.graph.AppView;
-import com.android.tools.r8.graph.DexField;
-import com.android.tools.r8.graph.EnumValueInfoMapCollection.EnumValueInfoMap;
-import com.android.tools.r8.graph.GraphLense;
-import com.android.tools.r8.shaking.AppInfoWithLiveness;
-
-public class SingleEnumValue extends SingleFieldValue {
-
- /** Intentionally package private, use {@link AbstractValueFactory} instead. */
- SingleEnumValue(DexField field) {
- super(field);
- }
-
- @Override
- public boolean isSingleEnumValue() {
- return true;
- }
-
- @Override
- public SingleEnumValue asSingleEnumValue() {
- return this;
- }
-
- @Override
- public String toString() {
- return "SingleEnumValue(" + getField().toSourceString() + ")";
- }
-
- @Override
- public SingleValue rewrittenWithLens(AppView<AppInfoWithLiveness> appView, GraphLense lens) {
- DexField field = getField();
- EnumValueInfoMap unboxedEnumInfo = appView.unboxedEnums().getEnumValueInfoMap(field.type);
- if (unboxedEnumInfo != null) {
- // Return the ordinal of the unboxed enum.
- assert unboxedEnumInfo.hasEnumValueInfo(field);
- return appView
- .abstractValueFactory()
- .createSingleNumberValue(unboxedEnumInfo.getEnumValueInfo(field).convertToInt());
- }
- return appView.abstractValueFactory().createSingleEnumValue(lens.lookupField(field));
- }
-}
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/value/SingleFieldValue.java b/src/main/java/com/android/tools/r8/ir/analysis/value/SingleFieldValue.java
index 855694a..bc51a08 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/value/SingleFieldValue.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/value/SingleFieldValue.java
@@ -13,6 +13,7 @@
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.EnumValueInfoMapCollection.EnumValueInfoMap;
import com.android.tools.r8.graph.GraphLense;
import com.android.tools.r8.ir.analysis.type.ClassTypeElement;
import com.android.tools.r8.ir.analysis.type.TypeElement;
@@ -108,9 +109,14 @@
@Override
public SingleValue rewrittenWithLens(AppView<AppInfoWithLiveness> appView, GraphLense lens) {
- DexField rewrittenField = lens.lookupField(field);
- assert !appView.unboxedEnums().containsEnum(field.holder)
- || !appView.appInfo().resolveField(rewrittenField).accessFlags.isEnum();
- return appView.abstractValueFactory().createSingleFieldValue(rewrittenField);
+ EnumValueInfoMap unboxedEnumInfo = appView.unboxedEnums().getEnumValueInfoMap(field.type);
+ if (unboxedEnumInfo != null) {
+ // Return the ordinal of the unboxed enum.
+ assert unboxedEnumInfo.hasEnumValueInfo(field);
+ return appView
+ .abstractValueFactory()
+ .createSingleNumberValue(unboxedEnumInfo.getEnumValueInfo(field).convertToInt());
+ }
+ return appView.abstractValueFactory().createSingleFieldValue(lens.lookupField(field));
}
}
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 eb36e39..3ab21cf 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
@@ -15,6 +15,7 @@
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DebugLocalInfo;
import com.android.tools.r8.graph.DexClass;
+import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexItemFactory.ThrowableMethods;
@@ -27,6 +28,8 @@
import com.android.tools.r8.ir.analysis.type.TypeAnalysis;
import com.android.tools.r8.ir.analysis.type.TypeElement;
import com.android.tools.r8.ir.analysis.value.AbstractValue;
+import com.android.tools.r8.ir.analysis.value.SingleConstClassValue;
+import com.android.tools.r8.ir.analysis.value.SingleFieldValue;
import com.android.tools.r8.ir.code.AlwaysMaterializingNop;
import com.android.tools.r8.ir.code.ArrayLength;
import com.android.tools.r8.ir.code.ArrayPut;
@@ -75,6 +78,7 @@
import com.android.tools.r8.ir.conversion.IRConverter;
import com.android.tools.r8.ir.optimize.controlflow.SwitchCaseAnalyzer;
import com.android.tools.r8.ir.regalloc.LinearScanRegisterAllocator;
+import com.android.tools.r8.utils.BooleanUtils;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.InternalOutputMode;
import com.android.tools.r8.utils.LongInterval;
@@ -2499,12 +2503,37 @@
} else {
DexType context = code.method.holder();
AbstractValue abstractValue = lhs.getAbstractValue(appView, context);
- if (abstractValue.isSingleConstClassValue() || abstractValue.isSingleFieldValue()) {
+ if (abstractValue.isSingleConstClassValue()) {
AbstractValue otherAbstractValue = rhs.getAbstractValue(appView, context);
- if (abstractValue == otherAbstractValue) {
- simplifyIfWithKnownCondition(code, block, theIf, 0);
- } else if (otherAbstractValue.isSingleEnumValue()) {
- simplifyIfWithKnownCondition(code, block, theIf, 1);
+ if (otherAbstractValue.isSingleConstClassValue()) {
+ SingleConstClassValue singleConstClassValue =
+ abstractValue.asSingleConstClassValue();
+ SingleConstClassValue otherSingleConstClassValue =
+ otherAbstractValue.asSingleConstClassValue();
+ simplifyIfWithKnownCondition(
+ code,
+ block,
+ theIf,
+ BooleanUtils.intValue(
+ singleConstClassValue.getType() != otherSingleConstClassValue.getType()));
+ }
+ } else if (abstractValue.isSingleFieldValue()) {
+ AbstractValue otherAbstractValue = rhs.getAbstractValue(appView, context);
+ if (otherAbstractValue.isSingleFieldValue()) {
+ SingleFieldValue singleFieldValue = abstractValue.asSingleFieldValue();
+ SingleFieldValue otherSingleFieldValue = otherAbstractValue.asSingleFieldValue();
+ if (singleFieldValue.getField() == otherSingleFieldValue.getField()) {
+ simplifyIfWithKnownCondition(code, block, theIf, 0);
+ } else {
+ DexEncodedField field = appView.definitionFor(singleFieldValue.getField());
+ if (field != null && field.isEnum()) {
+ DexEncodedField otherField =
+ appView.definitionFor(otherSingleFieldValue.getField());
+ if (otherField != null && otherField.isEnum()) {
+ simplifyIfWithKnownCondition(code, block, theIf, 1);
+ }
+ }
+ }
}
}
}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/ifs/ConstClassComparisonTest.java b/src/test/java/com/android/tools/r8/ir/optimize/ifs/ConstClassComparisonTest.java
new file mode 100644
index 0000000..6b8cb68
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/ifs/ConstClassComparisonTest.java
@@ -0,0 +1,83 @@
+// Copyright (c) 2019, 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.ir.optimize.ifs;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+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 org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class ConstClassComparisonTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public ConstClassComparisonTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(ConstClassComparisonTest.class)
+ .addKeepMainRule(TestClass.class)
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspect(this::inspect)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("true", "false", "false", "true");
+ }
+
+ private void inspect(CodeInspector inspector) {
+ ClassSubject classSubject = inspector.clazz(TestClass.class);
+ assertThat(classSubject, isPresent());
+
+ MethodSubject mainMethod = classSubject.mainMethod();
+ assertThat(mainMethod, isPresent());
+ assertTrue(mainMethod.streamInstructions().noneMatch(InstructionSubject::isConstClass));
+
+ assertThat(inspector.clazz(A.class), not(isPresent()));
+ assertThat(inspector.clazz(B.class), not(isPresent()));
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ System.out.println(getA() == getA());
+ System.out.println(getA() != getA());
+ System.out.println(getA() == getB());
+ System.out.println(getA() != getB());
+ }
+
+ static Class<?> getA() {
+ return A.class;
+ }
+
+ static Class<?> getB() {
+ return B.class;
+ }
+ }
+
+ static class A {}
+
+ static class B {}
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/ifs/EnumAliasComparisonTest.java b/src/test/java/com/android/tools/r8/ir/optimize/ifs/EnumAliasComparisonTest.java
index 5c0f75b..08fdca2 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/ifs/EnumAliasComparisonTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/ifs/EnumAliasComparisonTest.java
@@ -4,18 +4,10 @@
package com.android.tools.r8.ir.optimize.ifs;
-import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
-import static org.hamcrest.CoreMatchers.not;
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.assertTrue;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
-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 org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -42,40 +34,27 @@
.addKeepMainRule(TestClass.class)
.setMinApi(parameters.getApiLevel())
.compile()
- .inspect(this::inspect)
.run(parameters.getRuntime(), TestClass.class)
- .assertSuccessWithOutputLines("true");
- }
-
- private void inspect(CodeInspector inspector) {
- ClassSubject classSubject = inspector.clazz(TestClass.class);
- assertThat(classSubject, isPresent());
-
- MethodSubject mainMethod = classSubject.mainMethod();
- assertThat(mainMethod, isPresent());
- assertTrue(
- mainMethod
- .streamInstructions()
- .filter(InstructionSubject::isStaticGet)
- .map(InstructionSubject::getField)
- .map(field -> field.name.toSourceString())
- .allMatch("out"::equals));
-
- assertThat(inspector.clazz(MyEnum.class), not(isPresent()));
+ .assertSuccessWithOutputLines("true", "false", "false", "true", "true", "false");
}
static class TestClass {
public static void main(String[] args) {
- // Should print "true" since MyEnum.B is an alias of MyEnum.A.
+ System.out.println(MyEnum.A == MyEnum.A);
+ System.out.println(MyEnum.A != MyEnum.A);
System.out.println(MyEnum.A == MyEnum.B);
+ System.out.println(MyEnum.A != MyEnum.B);
+ System.out.println(MyEnum.A == MyEnum.C);
+ System.out.println(MyEnum.A != MyEnum.C);
}
}
enum MyEnum {
- A;
+ A,
+ B;
// Introduce an alias of MyEnum.A.
- static MyEnum B = A;
+ static MyEnum C = A;
}
}
