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"));