Canonicalize effectively final fields that are guaranteed to be written
Change-Id: I675119ee7e357a5d1440ed8bf4551fbf051eb77b
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/FieldValueAnalysis.java b/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/FieldValueAnalysis.java
index 17b91f3..f7d3bc0 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/FieldValueAnalysis.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/fieldvalueanalysis/FieldValueAnalysis.java
@@ -278,9 +278,12 @@
// Abstract value.
Value root = value.getAliasedValue();
AbstractValue abstractValue = computeAbstractValue(root);
- if (!abstractValue.isUnknown()) {
- feedback.recordFieldHasAbstractValue(field, appView, abstractValue);
- }
+ feedback.recordFieldHasAbstractValue(
+ field,
+ appView,
+ abstractValue.isUnknown()
+ ? appView.abstractValueFactory().createSingleFieldValue(field.field)
+ : abstractValue);
// Dynamic upper bound type.
TypeLatticeElement fieldType =
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 f560b29..2bd87e0 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
@@ -28,6 +28,14 @@
return null;
}
+ public boolean isSingleFieldValue() {
+ return false;
+ }
+
+ public SingleFieldValue asSingleFieldValue() {
+ return null;
+ }
+
public boolean isSingleNumberValue() {
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 7e34815..887a171 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
@@ -11,6 +11,8 @@
public class AbstractValueFactory {
private ConcurrentHashMap<DexField, SingleEnumValue> singleEnumValues = new ConcurrentHashMap<>();
+ private ConcurrentHashMap<DexField, SingleFieldValue> singleFieldValues =
+ new ConcurrentHashMap<>();
private ConcurrentHashMap<Long, SingleNumberValue> singleNumberValues = new ConcurrentHashMap<>();
private ConcurrentHashMap<DexString, SingleStringValue> singleStringValues =
new ConcurrentHashMap<>();
@@ -19,6 +21,10 @@
return singleEnumValues.computeIfAbsent(field, SingleEnumValue::new);
}
+ public SingleFieldValue createSingleFieldValue(DexField field) {
+ return singleFieldValues.computeIfAbsent(field, SingleFieldValue::new);
+ }
+
public SingleNumberValue createSingleNumberValue(long value) {
return singleNumberValues.computeIfAbsent(value, SingleNumberValue::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
index 1f7a76b..313f977 100644
--- 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
@@ -4,28 +4,13 @@
package com.android.tools.r8.ir.analysis.value;
-import static com.android.tools.r8.ir.analysis.type.Nullability.maybeNull;
-import static com.android.tools.r8.optimize.MemberRebindingAnalysis.isMemberVisibleFromOriginalContext;
-
-import com.android.tools.r8.graph.AppInfoWithSubtyping;
-import com.android.tools.r8.graph.AppView;
-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.ir.analysis.type.TypeLatticeElement;
-import com.android.tools.r8.ir.code.IRCode;
-import com.android.tools.r8.ir.code.Instruction;
-import com.android.tools.r8.ir.code.StaticGet;
-import com.android.tools.r8.ir.code.TypeAndLocalInfoSupplier;
-import com.android.tools.r8.ir.code.Value;
-public class SingleEnumValue extends SingleValue {
-
- private final DexField field;
+public class SingleEnumValue extends SingleFieldValue {
/** Intentionally package private, use {@link AbstractValueFactory} instead. */
SingleEnumValue(DexField field) {
- this.field = field;
+ super(field);
}
@Override
@@ -39,35 +24,7 @@
}
@Override
- public boolean equals(Object o) {
- return this == o;
- }
-
- @Override
- public int hashCode() {
- return System.identityHashCode(this);
- }
-
- @Override
public String toString() {
- return "SingleEnumValue(" + field.toSourceString() + ")";
- }
-
- @Override
- public Instruction createMaterializingInstruction(
- AppView<? extends AppInfoWithSubtyping> appView,
- IRCode code,
- TypeAndLocalInfoSupplier info) {
- TypeLatticeElement type = TypeLatticeElement.fromDexType(field.type, maybeNull(), appView);
- assert type.lessThanOrEqual(info.getTypeLattice(), appView);
- Value outValue = code.createValue(type, info.getLocalInfo());
- return new StaticGet(outValue, field);
- }
-
- @Override
- public boolean isMaterializableInContext(AppView<?> appView, DexType context) {
- DexEncodedField encodedField = appView.appInfo().resolveField(field);
- return isMemberVisibleFromOriginalContext(
- appView, context, encodedField.field.holder, encodedField.accessFlags);
+ return "SingleEnumValue(" + getField().toSourceString() + ")";
}
}
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
new file mode 100644
index 0000000..0adfe65
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/ir/analysis/value/SingleFieldValue.java
@@ -0,0 +1,75 @@
+// Copyright (c) 2020, 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 static com.android.tools.r8.ir.analysis.type.Nullability.maybeNull;
+import static com.android.tools.r8.optimize.MemberRebindingAnalysis.isMemberVisibleFromOriginalContext;
+
+import com.android.tools.r8.graph.AppInfoWithSubtyping;
+import com.android.tools.r8.graph.AppView;
+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.ir.analysis.type.TypeLatticeElement;
+import com.android.tools.r8.ir.code.IRCode;
+import com.android.tools.r8.ir.code.Instruction;
+import com.android.tools.r8.ir.code.StaticGet;
+import com.android.tools.r8.ir.code.TypeAndLocalInfoSupplier;
+import com.android.tools.r8.ir.code.Value;
+
+public class SingleFieldValue extends SingleValue {
+
+ private final DexField field;
+
+ /** Intentionally package private, use {@link AbstractValueFactory} instead. */
+ SingleFieldValue(DexField field) {
+ this.field = field;
+ }
+
+ public DexField getField() {
+ return field;
+ }
+
+ @Override
+ public boolean isSingleFieldValue() {
+ return true;
+ }
+
+ @Override
+ public SingleFieldValue asSingleFieldValue() {
+ return this;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ return this == o;
+ }
+
+ @Override
+ public int hashCode() {
+ return System.identityHashCode(this);
+ }
+
+ @Override
+ public String toString() {
+ return "SingleFieldValue(" + field.toSourceString() + ")";
+ }
+
+ @Override
+ public Instruction createMaterializingInstruction(
+ AppView<? extends AppInfoWithSubtyping> appView, IRCode code, TypeAndLocalInfoSupplier info) {
+ TypeLatticeElement type = TypeLatticeElement.fromDexType(field.type, maybeNull(), appView);
+ assert type.lessThanOrEqual(info.getTypeLattice(), appView);
+ Value outValue = code.createValue(type, info.getLocalInfo());
+ return new StaticGet(outValue, field);
+ }
+
+ @Override
+ public boolean isMaterializableInContext(AppView<?> appView, DexType context) {
+ DexEncodedField encodedField = appView.appInfo().resolveField(field);
+ return isMemberVisibleFromOriginalContext(
+ appView, context, encodedField.field.holder, encodedField.accessFlags);
+ }
+}
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 1382317..48f7b09 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
@@ -12,6 +12,7 @@
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.ir.analysis.value.AbstractValue;
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.ConstClass;
import com.android.tools.r8.ir.code.ConstNumber;
@@ -44,7 +45,7 @@
private int numberOfConstStringCanonicalization = 0;
private int numberOfDexItemBasedConstStringCanonicalization = 0;
private int numberOfConstClassCanonicalization = 0;
- private int numberOfEnumCanonicalization = 0;
+ private int numberOfEffectivelyFinalFieldCanonicalization = 0;
private final Object2IntMap<Long> histogramOfCanonicalizationCandidatesPerMethod;
public ConstantCanonicalizer() {
@@ -66,7 +67,10 @@
numberOfDexItemBasedConstStringCanonicalization);
Log.info(getClass(),
"# const-class canonicalization: %s", numberOfConstClassCanonicalization);
- Log.info(getClass(), "# enum canonicalization: %s", numberOfEnumCanonicalization);
+ Log.info(
+ getClass(),
+ "# effectively final field canonicalization: %s",
+ numberOfEffectivelyFinalFieldCanonicalization);
assert histogramOfCanonicalizationCandidatesPerMethod != null;
Log.info(getClass(), "------ histogram of constant canonicalization candidates ------");
histogramOfCanonicalizationCandidatesPerMethod.forEach((length, count) -> {
@@ -129,9 +133,11 @@
&& code.metadata().mayHaveMonitorInstruction()) {
continue;
}
- // Only canonicalize enum values. This is only OK if the instruction cannot have side effects.
+ // Canonicalize effectively final fields that are guaranteed to be written before they are
+ // read. This is only OK if the instruction cannot have side effects.
if (current.isStaticGet()) {
- if (!current.outValue().getAbstractValue(appView, context).isSingleEnumValue()) {
+ AbstractValue abstractValue = current.outValue().getAbstractValue(appView, context);
+ if (!abstractValue.isSingleFieldValue()) {
continue;
}
if (current.instructionMayHaveSideEffects(appView, context)) {
@@ -212,7 +218,7 @@
break;
case STATIC_GET:
if (Log.ENABLED) {
- numberOfEnumCanonicalization++;
+ numberOfEffectivelyFinalFieldCanonicalization++;
}
newConst = StaticGet.copyOf(code, canonicalizedConstant.asStaticGet());
break;
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/canonicalization/EffectivelyFinalFieldCanonicalizationTest.java b/src/test/java/com/android/tools/r8/ir/optimize/canonicalization/EffectivelyFinalFieldCanonicalizationTest.java
new file mode 100644
index 0000000..4e73445
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/canonicalization/EffectivelyFinalFieldCanonicalizationTest.java
@@ -0,0 +1,84 @@
+// Copyright (c) 2020, 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.canonicalization;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.NeverInline;
+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;
+
+@RunWith(Parameterized.class)
+public class EffectivelyFinalFieldCanonicalizationTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withDexRuntimes().withAllApiLevels().build();
+ }
+
+ public EffectivelyFinalFieldCanonicalizationTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(EffectivelyFinalFieldCanonicalizationTest.class)
+ .addKeepMainRule(TestClass.class)
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspect(this::inspect)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("[R8] Foo", "[R8] Bar");
+ }
+
+ private void inspect(CodeInspector inspector) {
+ ClassSubject testClassSubject = inspector.clazz(TestClass.class);
+ assertThat(testClassSubject, isPresent());
+
+ MethodSubject mainMethodSubject = testClassSubject.mainMethod();
+ assertThat(mainMethodSubject, isPresent());
+ assertEquals(
+ 1, mainMethodSubject.streamInstructions().filter(InstructionSubject::isStaticGet).count());
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ Printer.INSTANCE.print("Foo");
+ Printer.INSTANCE.print("Bar");
+ }
+ }
+
+ @NeverClassInline
+ static class Printer {
+
+ static Printer INSTANCE = new Printer("[R8]");
+
+ private String tag;
+
+ Printer(String tag) {
+ this.tag = tag;
+ }
+
+ @NeverInline
+ void print(String message) {
+ System.out.println(tag + " " + message);
+ }
+ }
+}
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 d0a4064..5ab6053 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
@@ -170,6 +170,7 @@
"STATIC: SimpleWithThrowingGetter SimpleWithThrowingGetter.getInstance()",
"STATIC: SimpleWithThrowingGetter SimpleWithThrowingGetter.getInstance()",
"STATIC: String TrivialTestClass.next()",
+ "SimpleWithThrowingGetter SimpleWithThrowingGetter.INSTANCE",
"VIRTUAL: String SimpleWithThrowingGetter.bar(String)",
"VIRTUAL: String SimpleWithThrowingGetter.foo()"),
references(clazz, "testSimpleWithThrowingGetter", "void"));