Member value propagation for null's
Change-Id: I3a6f234d1d66db70c77133f06121e79b47304a20
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 7b57fa2..670e510 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedField.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedField.java
@@ -5,10 +5,10 @@
import com.android.tools.r8.dex.IndexedItemCollection;
import com.android.tools.r8.dex.MixedSectionCollection;
-import com.android.tools.r8.ir.code.Instruction;
+import com.android.tools.r8.ir.code.ConstInstruction;
+import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
-import com.android.tools.r8.utils.InternalOptions;
public class DexEncodedField extends KeyedDexItem<DexField> {
public static final DexEncodedField[] EMPTY_ARRAY = {};
@@ -141,21 +141,49 @@
return staticValue == null ? DexValue.defaultForType(field.type) : staticValue;
}
- // Returns a const instructions if this field is a compile time final const.
- public Instruction valueAsConstInstruction(
- AppInfoWithLiveness appInfo, Value dest, InternalOptions options) {
- // The only way to figure out whether the DexValue contains the final value
- // is ensure the value is not the default or check <clinit> is not present.
- boolean isEffectivelyFinal =
- (accessFlags.isFinal() || !appInfo.isFieldWritten(this)) && !appInfo.isPinned(field);
- if (!isEffectivelyFinal) {
- return null;
+ /**
+ * Returns a const instructions if this field is a compile time final const.
+ *
+ * <p>NOTE: It is the responsibility of the caller to check if this field is pinned or not.
+ */
+ public ConstInstruction valueAsConstInstruction(
+ IRCode code, Value dest, AppView<AppInfoWithLiveness> appView) {
+ // If it is a static field, we can only propagate the value if class initialization does not
+ // have side effects.
+ if (isStatic()) {
+ DexClass clazz = appView.definitionFor(field.holder);
+ if (clazz == null) {
+ return null;
+ }
+ DexType context = code.method.method.holder;
+ if (clazz.classInitializationMayHaveSideEffects(
+ appView,
+ // Types that are a super type of the current context are guaranteed to be initialized
+ // already.
+ type -> appView.isSubtype(context, type).isTrue())) {
+ return null;
+ }
}
- if (accessFlags.isStatic()) {
- DexClass clazz = appInfo.definitionFor(field.holder);
- assert clazz != null : "Class for the field must be present";
- return getStaticValue().asConstInstruction(clazz.hasClassInitializer(), dest, options);
+
+ boolean isWritten = appView.appInfo().isFieldWrittenByFieldPutInstruction(this);
+ if (!isWritten) {
+ // Since the field is not written, we can simply return the default value for the type.
+ return getStaticValue().asConstInstruction(code, dest, appView.options());
}
+
+ // The only way to figure out whether the DexValue contains the final value is ensure the value
+ // is not the default or check that <clinit> is not present.
+ if (accessFlags.isFinal() && isStatic()) {
+ DexClass clazz = appView.definitionFor(field.holder);
+ if (clazz == null || clazz.hasClassInitializer()) {
+ return null;
+ }
+ DexValue staticValue = getStaticValue();
+ if (!staticValue.isDefault(field.type)) {
+ return staticValue.asConstInstruction(code, dest, appView.options());
+ }
+ }
+
return null;
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexValue.java b/src/main/java/com/android/tools/r8/graph/DexValue.java
index 782f04a..3752054 100644
--- a/src/main/java/com/android/tools/r8/graph/DexValue.java
+++ b/src/main/java/com/android/tools/r8/graph/DexValue.java
@@ -13,6 +13,7 @@
import com.android.tools.r8.ir.code.ConstNumber;
import com.android.tools.r8.ir.code.ConstString;
import com.android.tools.r8.ir.code.DexItemBasedConstString;
+import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.optimize.ReflectionOptimizer.ClassNameComputationInfo;
import com.android.tools.r8.utils.EncodedValueUtils;
@@ -137,9 +138,8 @@
public abstract Object getBoxedValue();
- // Returns a const instruction for the non default value.
- public ConstInstruction asConstInstruction(
- boolean hasClassInitializer, Value dest, InternalOptions options) {
+ /** Returns an instruction that can be used to materialize this {@link DexValue} (or null). */
+ public ConstInstruction asConstInstruction(IRCode code, Value dest, InternalOptions options) {
return null;
}
@@ -214,8 +214,7 @@
}
@Override
- public ConstInstruction asConstInstruction(
- boolean hasClassInitializer, Value dest, InternalOptions options) {
+ public ConstInstruction asConstInstruction(IRCode code, Value dest, InternalOptions options) {
return null;
}
}
@@ -302,9 +301,8 @@
}
@Override
- public ConstInstruction asConstInstruction(
- boolean hasClassInitializer, Value dest, InternalOptions options) {
- return (this == DEFAULT && hasClassInitializer) ? null : new ConstNumber(dest, value);
+ public ConstInstruction asConstInstruction(IRCode code, Value dest, InternalOptions options) {
+ return new ConstNumber(dest, value);
}
}
@@ -359,9 +357,8 @@
}
@Override
- public ConstInstruction asConstInstruction(
- boolean hasClassInitializer, Value dest, InternalOptions options) {
- return (this == DEFAULT && hasClassInitializer) ? null : new ConstNumber(dest, value);
+ public ConstInstruction asConstInstruction(IRCode code, Value dest, InternalOptions options) {
+ return new ConstNumber(dest, value);
}
}
@@ -420,9 +417,8 @@
}
@Override
- public ConstInstruction asConstInstruction(
- boolean hasClassInitializer, Value dest, InternalOptions options) {
- return (this == DEFAULT && hasClassInitializer) ? null : new ConstNumber(dest, value);
+ public ConstInstruction asConstInstruction(IRCode code, Value dest, InternalOptions options) {
+ return new ConstNumber(dest, value);
}
}
@@ -477,9 +473,8 @@
}
@Override
- public ConstInstruction asConstInstruction(
- boolean hasClassInitializer, Value dest, InternalOptions options) {
- return (this == DEFAULT && hasClassInitializer) ? null : new ConstNumber(dest, value);
+ public ConstInstruction asConstInstruction(IRCode code, Value dest, InternalOptions options) {
+ return new ConstNumber(dest, value);
}
}
@@ -534,9 +529,8 @@
}
@Override
- public ConstInstruction asConstInstruction(
- boolean hasClassInitializer, Value dest, InternalOptions options) {
- return (this == DEFAULT && hasClassInitializer) ? null : new ConstNumber(dest, value);
+ public ConstInstruction asConstInstruction(IRCode code, Value dest, InternalOptions options) {
+ return new ConstNumber(dest, value);
}
}
@@ -739,8 +733,7 @@
}
@Override
- public ConstInstruction asConstInstruction(
- boolean hasClassInitializer, Value dest, InternalOptions options) {
+ public ConstInstruction asConstInstruction(IRCode code, Value dest, InternalOptions options) {
ConstString instruction =
new ConstString(dest, value, ThrowingInfo.defaultForConstString(options));
if (!instruction.instructionInstanceCanThrow()) {
@@ -784,8 +777,7 @@
}
@Override
- public ConstInstruction asConstInstruction(
- boolean hasClassInitializer, Value dest, InternalOptions options) {
+ public ConstInstruction asConstInstruction(IRCode code, Value dest, InternalOptions options) {
DexItemBasedConstString instruction =
new DexItemBasedConstString(
dest, value, ThrowingInfo.defaultForConstString(options), classNameComputationInfo);
@@ -1122,9 +1114,8 @@
}
@Override
- public ConstInstruction asConstInstruction(
- boolean hasClassInitializer, Value dest, InternalOptions options) {
- return (this == DEFAULT && hasClassInitializer) ? null : new ConstNumber(dest, value ? 1 : 0);
+ public ConstInstruction asConstInstruction(IRCode code, Value dest, InternalOptions options) {
+ return new ConstNumber(dest, value ? 1 : 0);
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java b/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
index df73ae8..ccd346b 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
@@ -3,7 +3,6 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.ir.optimize;
-import com.android.tools.r8.errors.CompilationError;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DebugLocalInfo;
import com.android.tools.r8.graph.DexClass;
@@ -36,6 +35,8 @@
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.shaking.ProguardMemberRule;
import com.android.tools.r8.shaking.ProguardMemberRuleReturnValue;
+import com.android.tools.r8.utils.Reporter;
+import com.android.tools.r8.utils.StringDiagnostic;
import com.google.common.collect.Sets;
import java.util.ListIterator;
import java.util.Set;
@@ -44,6 +45,10 @@
public class MemberValuePropagation {
private final AppView<AppInfoWithLiveness> appView;
+ private final Reporter reporter;
+
+ // Fields for which we have reported warnings to due Proguard configuration rules.
+ private final Set<DexField> warnedFields = Sets.newIdentityHashSet();
private enum RuleType {
NONE,
@@ -78,6 +83,7 @@
public MemberValuePropagation(AppView<AppInfoWithLiveness> appView) {
this.appView = appView;
+ this.reporter = appView.options().reporter;
}
private boolean mayPropagateValueFor(DexEncodedField field) {
@@ -134,13 +140,30 @@
DexEncodedField staticField = appView.appInfo().lookupStaticTarget(field.holder, field);
if (staticField == null) {
- throw new CompilationError(field.holder.toSourceString() + "." + field.name.toString()
- + " used in assumevalues rule does not exist.");
+ if (warnedFields.add(field)) {
+ reporter.warning(
+ new StringDiagnostic(
+ "Field `"
+ + field.toSourceString()
+ + "` is used in an -assumevalues rule but does not exist.",
+ code.origin));
+ }
+ return null;
}
Value value = code.createValue(typeLattice, instruction.getLocalInfo());
- ConstInstruction replacement =
- staticField.getStaticValue().asConstInstruction(false, value, appView.options());
+ ConstInstruction replacement = staticField.valueAsConstInstruction(code, value, appView);
+ if (replacement == null) {
+ reporter.warning(
+ new StringDiagnostic(
+ "Unable to apply the rule `"
+ + returnValueRule.toString()
+ + "`: Could not determine the value of field `"
+ + field.toSourceString()
+ + "`",
+ code.origin));
+ return null;
+ }
if (replacement.isDexItemBasedConstString()) {
code.method.getMutableOptimizationInfo().markUseIdentifierNameString();
}
@@ -309,18 +332,21 @@
if (target == null || !mayPropagateValueFor(target)) {
return;
}
+
// Check if a this value is known const.
- Instruction replacement =
- target.valueAsConstInstruction(appView.appInfo(), current.dest(), appView.options());
- if (replacement != null) {
- affectedValues.add(replacement.outValue());
- iterator.replaceCurrentInstruction(replacement);
- if (replacement.isDexItemBasedConstString()) {
- code.method.getMutableOptimizationInfo().markUseIdentifierNameString();
+ if (!appView.appInfo().isPinned(target.field)) {
+ ConstInstruction replacement = target.valueAsConstInstruction(code, current.dest(), appView);
+ if (replacement != null) {
+ affectedValues.add(replacement.outValue());
+ iterator.replaceCurrentInstruction(replacement);
+ if (replacement.isDexItemBasedConstString()) {
+ code.method.getMutableOptimizationInfo().markUseIdentifierNameString();
+ }
+ target.getMutableOptimizationInfo().markAsPropagated();
+ return;
}
- target.getMutableOptimizationInfo().markAsPropagated();
- return;
}
+
ProguardMemberRuleLookup lookup = lookupMemberRule(target);
if (lookup != null
&& tryConstantReplacementFromProguard(
diff --git a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
index 92783d2..77ea68b 100644
--- a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
+++ b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
@@ -639,16 +639,26 @@
public boolean isFieldWritten(DexEncodedField encodedField) {
assert checkIfObsolete();
+ return isFieldWrittenByFieldPutInstruction(encodedField) || isPinned(encodedField.field);
+ }
+
+ public boolean isFieldWrittenByFieldPutInstruction(DexEncodedField encodedField) {
+ assert checkIfObsolete();
DexField field = encodedField.field;
FieldAccessInfoImpl info = fieldAccessInfoCollection.get(field);
if (info != null && info.isWritten()) {
+ // The field is written directly by the program itself.
return true;
}
- return isPinned(field)
- // Fields in the class that is synthesized by D8/R8 would be used soon.
- || field.holder.isD8R8SynthesizedClassType()
- // For library classes we don't know whether a field is rewritten.
- || isLibraryOrClasspathField(encodedField);
+ if (field.holder.isD8R8SynthesizedClassType()) {
+ // Fields in the class that is synthesized by D8/R8 would be used soon.
+ return true;
+ }
+ if (isLibraryOrClasspathField(encodedField)) {
+ // For library classes we don't know whether a field is rewritten.
+ return true;
+ }
+ return false;
}
public boolean isStaticFieldWrittenOnlyInEnclosingStaticInitializer(DexEncodedField field) {
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/string/StringInMonitorTest.java b/src/test/java/com/android/tools/r8/ir/optimize/string/StringInMonitorTest.java
index a8dfd4c..e9195fd 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/string/StringInMonitorTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/string/StringInMonitorTest.java
@@ -11,6 +11,7 @@
import com.android.tools.r8.D8TestRunResult;
import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NeverPropagateValue;
import com.android.tools.r8.R8TestRunResult;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
@@ -34,7 +35,7 @@
class StringInMonitorTestMain {
static final Object lock = new Object();
- private static int foo = 0;
+ @NeverPropagateValue private static int foo = 0;
@NeverInline
private static synchronized void sync() throws OutOfMemoryError {
@@ -140,11 +141,28 @@
}
MethodSubject sync = mainClass.uniqueMethodWithName("sync");
+ System.out.println(sync.getMethod().getCode());
assertThat(sync, isPresent());
count = Streams.stream(sync.iterateInstructions(
i -> i.isConstString("", JumboStringMode.ALLOW))).count();
assertEquals(expectedConstStringCount2, count);
+ /*.limit stack 2
+ .limit locals 1
+ 0: ldc bar
+ 1: ldc
+ 2: if_acmpeq L0
+ 3: return
+ 4: L0: ; locals:
+ 5: ; frame: [] []
+ 6: getstatic java/lang/System/out Ljava/io/PrintStream;
+ 7: ldc bar
+ 8: invokevirtual java.io.PrintStream.println(Ljava/lang/String;)V
+ 9: new java/lang/OutOfMemoryError
+ 10: dup
+ 11: invokespecial java.lang.OutOfMemoryError.<init>()V
+ 12: athrow*/
+
// In CF, we don't explicitly add monitor-{enter|exit} and catch-all for synchronized methods.
if (parameters.isDexRuntime()) {
Iterator<InstructionSubject> constStringIterator =
@@ -203,6 +221,7 @@
testForR8(parameters.getBackend())
.addProgramClasses(MAIN)
.enableInliningAnnotations()
+ .enableMemberValuePropagationAnnotations()
.addKeepMainRule(MAIN)
.setMinApi(parameters.getRuntime())
.run(parameters.getRuntime(), MAIN)
