Disable canonicalization for singleton fields
Bug: 171136616
Change-Id: I42a6046030ae104daf1f4b8a41aaf50b74904889
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java b/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
index 36fba35..912a030 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
@@ -11,6 +11,9 @@
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexClass;
+import com.android.tools.r8.graph.DexEncodedField;
+import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.ir.analysis.value.AbstractValue;
import com.android.tools.r8.ir.analysis.value.SingleFieldValue;
@@ -147,13 +150,19 @@
continue;
}
SingleFieldValue singleFieldValue = abstractValue.asSingleFieldValue();
+ DexType fieldHolderType = singleFieldValue.getField().getHolderType();
if (context.getDefinition().isClassInitializer()
- && context.getHolderType() == singleFieldValue.getField().holder) {
+ && context.getHolderType() == fieldHolderType) {
// Avoid that canonicalization inserts a read before the unique write in the class
// initializer, as that would change the program behavior.
continue;
}
- if (current.instructionMayHaveSideEffects(appView, context)) {
+ DexClass fieldHolder = appView.definitionFor(fieldHolderType);
+ DexEncodedField field = singleFieldValue.getField().lookupOnClass(fieldHolder);
+ if (field == null
+ || !field.isEnum()
+ || current.instructionMayHaveSideEffects(appView, context)) {
+ // Only allow canonicalization of enums.
continue;
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/RedundantFieldLoadElimination.java b/src/main/java/com/android/tools/r8/ir/optimize/RedundantFieldLoadElimination.java
index 54976af..97f9d07 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/RedundantFieldLoadElimination.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/RedundantFieldLoadElimination.java
@@ -8,10 +8,11 @@
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexClassAndField;
import com.android.tools.r8.graph.DexClassAndMethod;
-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.FieldResolutionResult.SuccessfulFieldResolutionResult;
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.graph.classmerging.VerticallyMergedClasses;
import com.android.tools.r8.ir.analysis.type.TypeAnalysis;
@@ -148,19 +149,24 @@
}
}
- public boolean isFinal(DexEncodedField field) {
- if (field.isProgramField(appView)) {
- return field.isFinal();
+ public boolean isFinal(DexClassAndField field) {
+ if (field.isProgramField()) {
+ // Treat this field as being final if it is declared final or we have determined a constant
+ // value for it.
+ return field.getDefinition().isFinal()
+ || field.getDefinition().getOptimizationInfo().getAbstractValue().isSingleValue();
}
- return appView.libraryMethodOptimizer().isFinalLibraryField(field);
+ return appView.libraryMethodOptimizer().isFinalLibraryField(field.getDefinition());
}
- private DexEncodedField resolveField(DexField field) {
+ private DexClassAndField resolveField(DexField field) {
if (appView.enableWholeProgramOptimizations()) {
- return appView.appInfo().withLiveness().resolveField(field).getResolvedField();
+ SuccessfulFieldResolutionResult resolutionResult =
+ appView.appInfo().withLiveness().resolveField(field).asSuccessfulResolution();
+ return resolutionResult != null ? resolutionResult.getResolutionPair() : null;
}
- if (field.holder == method.getHolderType()) {
- return method.getHolder().lookupField(field);
+ if (field.getHolderType() == method.getHolderType()) {
+ return method.getHolder().lookupProgramField(field);
}
return null;
}
@@ -187,9 +193,9 @@
while (it.hasNext()) {
Instruction instruction = it.next();
if (instruction.isFieldInstruction()) {
- DexField field = instruction.asFieldInstruction().getField();
- DexEncodedField definition = resolveField(field);
- if (definition == null || definition.isVolatile()) {
+ DexField reference = instruction.asFieldInstruction().getField();
+ DexClassAndField field = resolveField(reference);
+ if (field == null || field.getDefinition().isVolatile()) {
killAllNonFinalActiveFields();
continue;
}
@@ -200,7 +206,7 @@
continue;
}
Value object = instanceGet.object().getAliasedValue();
- FieldAndObject fieldAndObject = new FieldAndObject(field, object);
+ FieldAndObject fieldAndObject = new FieldAndObject(reference, object);
FieldValue replacement = activeState.getInstanceFieldValue(fieldAndObject);
if (replacement != null) {
replacement.eliminateRedundantRead(it, instanceGet);
@@ -215,10 +221,11 @@
killNonFinalActiveFields(instancePut);
// ... but at least we know the field value for this particular object.
Value object = instancePut.object().getAliasedValue();
- FieldAndObject fieldAndObject = new FieldAndObject(field, object);
+ FieldAndObject fieldAndObject = new FieldAndObject(reference, object);
ExistingValue value = new ExistingValue(instancePut.value());
- if (isFinal(definition)) {
- assert method.getDefinition().isInstanceInitializer()
+ if (isFinal(field)) {
+ assert !field.getDefinition().isFinal()
+ || method.getDefinition().isInstanceInitializer()
|| verifyWasInstanceInitializer();
activeState.putFinalInstanceField(fieldAndObject, value);
} else {
@@ -229,7 +236,7 @@
if (staticGet.outValue().hasLocalInfo()) {
continue;
}
- FieldValue replacement = activeState.getStaticFieldValue(field);
+ FieldValue replacement = activeState.getStaticFieldValue(reference);
if (replacement != null) {
replacement.eliminateRedundantRead(it, staticGet);
} else {
@@ -237,10 +244,10 @@
// field values.
killNonFinalActiveFields(staticGet);
FieldValue value = new ExistingValue(staticGet.value());
- if (isFinal(definition)) {
- activeState.putFinalStaticField(field, value);
+ if (isFinal(field)) {
+ activeState.putFinalStaticField(reference, value);
} else {
- activeState.putNonFinalStaticField(field, value);
+ activeState.putNonFinalStaticField(reference, value);
}
}
} else if (instruction.isStaticPut()) {
@@ -249,11 +256,11 @@
// field values.
killNonFinalActiveFields(staticPut);
ExistingValue value = new ExistingValue(staticPut.value());
- if (definition.isFinal()) {
+ if (isFinal(field)) {
assert method.getDefinition().isClassInitializer();
- activeState.putFinalStaticField(field, value);
+ activeState.putFinalStaticField(reference, value);
} else {
- activeState.putNonFinalStaticField(field, value);
+ activeState.putNonFinalStaticField(reference, value);
}
}
} else if (instruction.isInitClass()) {
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/canonicalization/SingletonCanonicalizationWithApiLevelCheckTest.java b/src/test/java/com/android/tools/r8/ir/optimize/canonicalization/SingletonCanonicalizationWithApiLevelCheckTest.java
index 7f5b905..bf78732 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/canonicalization/SingletonCanonicalizationWithApiLevelCheckTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/canonicalization/SingletonCanonicalizationWithApiLevelCheckTest.java
@@ -62,15 +62,7 @@
.compile()
.addRunClasspathFiles(program)
.run(parameters.getRuntime(), TestClass.class)
- .apply(
- runResult -> {
- if (parameters.isCfRuntime()) {
- // Constant canonicalization is disabled for CF.
- runResult.assertSuccessWithEmptyOutput();
- } else {
- runResult.assertFailureWithErrorThatThrows(NoClassDefFoundError.class);
- }
- });
+ .assertSuccessWithEmptyOutput();
}
private List<String> getAssumeValuesRule(int version) {
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizerTest.java b/src/test/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizerTest.java
index df009b4..e57d90b 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizerTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizerTest.java
@@ -180,7 +180,6 @@
"STATIC: String SimpleWithSideEffects.bar(String)",
"STATIC: String SimpleWithSideEffects.foo()",
"STATIC: String TrivialTestClass.next()",
- "SimpleWithSideEffects SimpleWithSideEffects.INSTANCE",
"SimpleWithSideEffects SimpleWithSideEffects.INSTANCE"),
references(clazz, "testSimpleWithSideEffects", "void"));
@@ -315,7 +314,6 @@
"STATIC: String movetohost.HostOkSideEffects.bar(String)",
"STATIC: String movetohost.HostOkSideEffects.foo()",
"STATIC: String movetohost.MoveToHostTestClass.next()",
- "movetohost.HostOkSideEffects movetohost.HostOkSideEffects.INSTANCE",
"movetohost.HostOkSideEffects movetohost.HostOkSideEffects.INSTANCE"),
references(clazz, "testOkSideEffects", "void"));
