Insert assume-not-null instructions based on assume rules Bug: 174285670 Change-Id: Ib8ad1ba1e5e04e8a9133441e336f5e45b4de95a7
diff --git a/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java b/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java index 7ec3198..fa42fcd 100644 --- a/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java +++ b/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java
@@ -64,8 +64,18 @@ public boolean instructionInstanceCanThrow( AppView<?> appView, ProgramMethod context, SideEffectAssumption assumption) { - SuccessfulFieldResolutionResult resolutionResult = - appView.appInfo().resolveField(field, context).asSuccessfulResolution(); + return internalInstructionInstanceCanThrow( + appView, + context, + assumption, + appView.appInfo().resolveField(field, context).asSuccessfulResolution()); + } + + boolean internalInstructionInstanceCanThrow( + AppView<?> appView, + ProgramMethod context, + SideEffectAssumption assumption, + SuccessfulFieldResolutionResult resolutionResult) { if (resolutionResult == null) { return true; }
diff --git a/src/main/java/com/android/tools/r8/ir/code/InstancePut.java b/src/main/java/com/android/tools/r8/ir/code/InstancePut.java index c38c5c0..20377f3 100644 --- a/src/main/java/com/android/tools/r8/ir/code/InstancePut.java +++ b/src/main/java/com/android/tools/r8/ir/code/InstancePut.java
@@ -19,6 +19,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.FieldResolutionResult.SuccessfulFieldResolutionResult; import com.android.tools.r8.graph.ProgramMethod; import com.android.tools.r8.ir.analysis.ClassInitializationAnalysis; import com.android.tools.r8.ir.analysis.ClassInitializationAnalysis.AnalysisAssumption; @@ -121,12 +122,13 @@ AppView<AppInfoWithLiveness> appViewWithLiveness = appView.withLiveness(); AppInfoWithLiveness appInfoWithLiveness = appViewWithLiveness.appInfo(); - if (instructionInstanceCanThrow(appView, context, assumption)) { + SuccessfulFieldResolutionResult resolutionResult = + appInfoWithLiveness.resolveField(getField()).asSuccessfulResolution(); + if (internalInstructionInstanceCanThrow(appView, context, assumption, resolutionResult)) { return true; } - DexEncodedField encodedField = - appInfoWithLiveness.resolveField(getField()).getResolvedField(); + DexEncodedField encodedField = resolutionResult.getResolvedField(); assert encodedField != null : "NoSuchFieldError (resolution failure) should be caught."; if (encodedField.type().isAlwaysNull(appViewWithLiveness)) {
diff --git a/src/main/java/com/android/tools/r8/ir/code/StaticPut.java b/src/main/java/com/android/tools/r8/ir/code/StaticPut.java index 82d67b0..1017f53 100644 --- a/src/main/java/com/android/tools/r8/ir/code/StaticPut.java +++ b/src/main/java/com/android/tools/r8/ir/code/StaticPut.java
@@ -18,6 +18,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.FieldResolutionResult.SuccessfulFieldResolutionResult; import com.android.tools.r8.graph.ProgramMethod; import com.android.tools.r8.ir.analysis.ClassInitializationAnalysis; import com.android.tools.r8.ir.analysis.ClassInitializationAnalysis.AnalysisAssumption; @@ -28,7 +29,6 @@ import com.android.tools.r8.ir.optimize.InliningConstraints; import com.android.tools.r8.ir.regalloc.RegisterAllocator; import com.android.tools.r8.shaking.AppInfoWithLiveness; -import com.android.tools.r8.shaking.ProguardMemberRule; public class StaticPut extends FieldInstruction implements StaticFieldInstruction { @@ -99,22 +99,14 @@ if (appView.appInfo().hasLiveness()) { AppView<AppInfoWithLiveness> appViewWithLiveness = appView.withLiveness(); AppInfoWithLiveness appInfoWithLiveness = appViewWithLiveness.appInfo(); - // MemberValuePropagation will replace the field read only if the target field has bound - // -assumevalues rule whose return value is *single*. - // - // Note that, in principle, class initializer of the field's holder may have side effects. - // However, with -assumevalues, we assume that the developer wants to remove field accesses. - ProguardMemberRule rule = appInfoWithLiveness.assumedValues.get(getField()); - if (rule != null && rule.getReturnValue().isSingleValue()) { - return false; - } - if (instructionInstanceCanThrow(appView, context, assumption)) { + SuccessfulFieldResolutionResult resolutionResult = + appInfoWithLiveness.resolveField(getField()).asSuccessfulResolution(); + if (internalInstructionInstanceCanThrow(appView, context, assumption, resolutionResult)) { return true; } - DexEncodedField encodedField = - appInfoWithLiveness.resolveField(getField()).getResolvedField(); + DexEncodedField encodedField = resolutionResult.getResolvedField(); assert encodedField != null : "NoSuchFieldError (resolution failure) should be caught."; boolean isDeadProtoExtensionField =
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/AssumeInserter.java b/src/main/java/com/android/tools/r8/ir/optimize/AssumeInserter.java index d6eae9a..ddb7ff5 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/AssumeInserter.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/AssumeInserter.java
@@ -6,11 +6,12 @@ import static com.android.tools.r8.ir.analysis.type.Nullability.definitelyNotNull; import static com.google.common.base.Predicates.alwaysTrue; -import com.android.tools.r8.graph.AppInfoWithClassHierarchy; 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.DexMethod; +import com.android.tools.r8.graph.FieldResolutionResult.SuccessfulFieldResolutionResult; +import com.android.tools.r8.graph.ResolutionResult.SingleResolutionResult; import com.android.tools.r8.ir.analysis.type.ClassTypeElement; import com.android.tools.r8.ir.analysis.type.TypeAnalysis; import com.android.tools.r8.ir.analysis.type.TypeElement; @@ -33,7 +34,9 @@ import com.android.tools.r8.ir.code.Value; import com.android.tools.r8.ir.optimize.info.FieldOptimizationInfo; import com.android.tools.r8.ir.optimize.info.MethodOptimizationInfo; -import com.android.tools.r8.utils.InternalOptions; +import com.android.tools.r8.ir.optimize.membervaluepropagation.assume.AssumeInfo; +import com.android.tools.r8.ir.optimize.membervaluepropagation.assume.AssumeInfoLookup; +import com.android.tools.r8.shaking.AppInfoWithLiveness; import com.android.tools.r8.utils.Timing; import com.android.tools.r8.utils.TriConsumer; import com.android.tools.r8.utils.TriFunction; @@ -56,12 +59,10 @@ public class AssumeInserter { - private final AppView<? extends AppInfoWithClassHierarchy> appView; - private final InternalOptions options; + private final AppView<AppInfoWithLiveness> appView; - public AssumeInserter(AppView<? extends AppInfoWithClassHierarchy> appView) { + public AssumeInserter(AppView<AppInfoWithLiveness> appView) { this.appView = appView; - this.options = appView.options(); } public void insertAssumeInstructions(IRCode code, Timing timing) { @@ -225,7 +226,26 @@ private boolean computeAssumedValuesFromSingleTarget( IRCode code, InvokeMethod invoke, AssumedValues.Builder assumedValuesBuilder) { + SingleResolutionResult resolutionResult = + appView + .appInfo() + .unsafeResolveMethodDueToDexFormat(invoke.getInvokedMethod()) + .asSingleResolution(); + if (resolutionResult == null) { + return false; + } + DexClassAndMethod singleTarget = invoke.lookupSingleTarget(appView, code.context()); + if (invoke.hasUsedOutValue() && invoke.getOutType().isReferenceType()) { + AssumeInfo assumeInfo = + AssumeInfoLookup.lookupAssumeInfo(appView, resolutionResult, singleTarget); + if (assumeInfo != null + && assumeInfo.hasReturnInfo() + && assumeInfo.getReturnInfo().isNonNull()) { + assumedValuesBuilder.addNonNullValueKnownToDominateAllUsers(invoke, invoke.outValue()); + } + } + if (singleTarget == null) { return false; } @@ -234,12 +254,11 @@ MethodOptimizationInfo optimizationInfo = singleTarget.getDefinition().getOptimizationInfo(); // Case (2), invocations that are guaranteed to return a non-null value. - Value outValue = invoke.outValue(); - if (outValue != null && outValue.hasNonDebugUsers()) { + if (invoke.hasUsedOutValue()) { needsAssumeInstruction = computeAssumedValuesForOutValue( invoke, - optimizationInfo.getDynamicUpperBoundTypeOrElse(outValue.getType()), + optimizationInfo.getDynamicUpperBoundTypeOrElse(invoke.getOutType()), optimizationInfo.getDynamicLowerBoundType(), assumedValuesBuilder); } @@ -264,20 +283,31 @@ private boolean computeAssumedValuesForFieldGet( FieldInstruction fieldGet, AssumedValues.Builder assumedValuesBuilder) { - Value outValue = fieldGet.outValue(); - if (!outValue.hasNonDebugUsers()) { + if (fieldGet.hasUnusedOutValue()) { return false; } - DexEncodedField field = appView.appInfo().resolveField(fieldGet.getField()).getResolvedField(); - if (field == null) { + SuccessfulFieldResolutionResult resolutionResult = + appView.appInfo().resolveField(fieldGet.getField()).asSuccessfulResolution(); + if (resolutionResult == null) { return false; } - FieldOptimizationInfo optimizationInfo = field.getOptimizationInfo(); + DexClassAndField field = resolutionResult.getResolutionPair(); + + if (field.getType().isReferenceType()) { + AssumeInfo assumeInfo = AssumeInfoLookup.lookupAssumeInfo(appView, field); + if (assumeInfo != null + && assumeInfo.hasReturnInfo() + && assumeInfo.getReturnInfo().isNonNull()) { + assumedValuesBuilder.addNonNullValueKnownToDominateAllUsers(fieldGet, fieldGet.outValue()); + } + } + + FieldOptimizationInfo optimizationInfo = field.getDefinition().getOptimizationInfo(); return computeAssumedValuesForOutValue( fieldGet, - optimizationInfo.getDynamicUpperBoundTypeOrElse(outValue.getType()), + optimizationInfo.getDynamicUpperBoundTypeOrElse(fieldGet.getOutType()), optimizationInfo.getDynamicLowerBoundType(), assumedValuesBuilder); }
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 2950d4c..d92d375 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
@@ -7,19 +7,20 @@ import static com.google.common.base.Predicates.alwaysTrue; 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.DexDefinition; 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.DexMethod; -import com.android.tools.r8.graph.DexReference; 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.ResolutionResult.SingleResolutionResult; 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.SingleValue; +import com.android.tools.r8.ir.analysis.value.UnknownValue; import com.android.tools.r8.ir.code.BasicBlock; import com.android.tools.r8.ir.code.FieldInstruction; import com.android.tools.r8.ir.code.IRCode; @@ -34,8 +35,9 @@ import com.android.tools.r8.ir.code.Value; import com.android.tools.r8.ir.optimize.info.OptimizationFeedback; import com.android.tools.r8.ir.optimize.info.OptimizationFeedbackSimple; +import com.android.tools.r8.ir.optimize.membervaluepropagation.assume.AssumeInfo; +import com.android.tools.r8.ir.optimize.membervaluepropagation.assume.AssumeInfoLookup; 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; @@ -54,48 +56,17 @@ // Fields for which we have reported warnings to due Proguard configuration rules. private final Set<DexField> warnedFields = Sets.newIdentityHashSet(); - private enum RuleType { - NONE, - ASSUME_NO_SIDE_EFFECTS, - ASSUME_VALUES - } - - private static class ProguardMemberRuleLookup { - - final RuleType type; - final ProguardMemberRule rule; - - ProguardMemberRuleLookup(RuleType type, ProguardMemberRule rule) { - this.type = type; - this.rule = rule; - } - - @Override - public boolean equals(Object other) { - if (!(other instanceof ProguardMemberRuleLookup)) { - return false; - } - ProguardMemberRuleLookup otherLookup = (ProguardMemberRuleLookup) other; - return type == otherLookup.type && rule == otherLookup.rule; - } - - @Override - public int hashCode() { - return type.ordinal() * 31 + rule.hashCode(); - } - } - public MemberValuePropagation(AppView<AppInfoWithLiveness> appView) { this.appView = appView; this.reporter = appView.options().reporter; } - private boolean mayPropagateValueFor(DexEncodedField field) { - if (field.isProgramField(appView)) { - return appView.appInfo().mayPropagateValueFor(field.field); + private boolean mayPropagateValueFor(DexClassAndField field) { + if (field.isProgramField()) { + return appView.appInfo().mayPropagateValueFor(field.getReference()); } - return appView.appInfo().assumedValues.containsKey(field.field) - || appView.appInfo().noSideEffects.containsKey(field.field); + return appView.appInfo().assumedValues.containsKey(field.getReference()) + || appView.appInfo().noSideEffects.containsKey(field.getReference()); } private boolean mayPropagateValueFor(DexClassAndMethod method) { @@ -106,36 +77,25 @@ || appView.appInfo().noSideEffects.containsKey(method.getReference()); } - private ProguardMemberRuleLookup lookupMemberRule(DexClassAndMethod method) { - return method != null ? lookupMemberRule(method.getDefinition()) : null; - } - - private ProguardMemberRuleLookup lookupMemberRule(DexDefinition definition) { - if (definition == null) { - return null; - } - DexReference reference = definition.getReference(); - ProguardMemberRule rule = appView.appInfo().noSideEffects.get(reference); - if (rule != null) { - return new ProguardMemberRuleLookup(RuleType.ASSUME_NO_SIDE_EFFECTS, rule); - } - rule = appView.appInfo().assumedValues.get(reference); - if (rule != null) { - return new ProguardMemberRuleLookup(RuleType.ASSUME_VALUES, rule); - } - return null; - } - - private Instruction constantReplacementFromProguardRule( - ProguardMemberRule rule, IRCode code, Instruction instruction) { - if (rule == null || !rule.hasReturnValue()) { + private Instruction createReplacementFromAssumeInfo( + AssumeInfo assumeInfo, IRCode code, Instruction instruction) { + if (!assumeInfo.hasReturnInfo()) { return null; } - ProguardMemberRuleReturnValue returnValueRule = rule.getReturnValue(); + ProguardMemberRuleReturnValue returnValueRule = assumeInfo.getReturnInfo(); // Check if this value can be assumed constant. if (returnValueRule.isSingleValue()) { + if (instruction.getOutType().isReferenceType()) { + if (returnValueRule.getSingleValue() == 0) { + return appView + .abstractValueFactory() + .createNullValue() + .createMaterializingInstruction(appView, code, instruction); + } + return null; + } return appView.abstractValueFactory() .createSingleNumberValue(returnValueRule.getSingleValue()) .createMaterializingInstruction(appView, code, instruction); @@ -177,31 +137,33 @@ return null; } - private void setValueRangeFromProguardRule(ProguardMemberRule rule, Value value) { - if (rule.hasReturnValue() && rule.getReturnValue().isValueRange()) { - assert !rule.getReturnValue().isSingleValue(); - value.setValueRange(rule.getReturnValue().getValueRange()); + private void setValueRangeFromAssumeInfo(AssumeInfo assumeInfo, Value value) { + if (assumeInfo.hasReturnInfo() && assumeInfo.getReturnInfo().isValueRange()) { + assert !assumeInfo.getReturnInfo().isSingleValue(); + value.setValueRange(assumeInfo.getReturnInfo().getValueRange()); } } - private boolean tryConstantReplacementFromProguard( + private boolean applyAssumeInfoIfPossible( IRCode code, Set<Value> affectedValues, ListIterator<BasicBlock> blocks, InstructionListIterator iterator, Instruction current, - ProguardMemberRuleLookup lookup) { - Instruction replacement = constantReplacementFromProguardRule(lookup.rule, code, current); + AssumeInfo assumeInfo) { + Instruction replacement = createReplacementFromAssumeInfo(assumeInfo, code, current); if (replacement == null) { // Check to see if a value range can be assumed. - setValueRangeFromProguardRule(lookup.rule, current.outValue()); + if (current.getOutType().isPrimitiveType()) { + setValueRangeFromAssumeInfo(assumeInfo, current.outValue()); + } return false; } affectedValues.addAll(current.outValue().affectedValues()); - if (lookup.type == RuleType.ASSUME_NO_SIDE_EFFECTS) { + if (assumeInfo.isAssumeNoSideEffects()) { iterator.replaceCurrentInstruction(replacement); } else { - assert lookup.type == RuleType.ASSUME_VALUES; + assert assumeInfo.isAssumeValues(); BasicBlock block = current.getBlock(); Position position = current.getPosition(); if (current.hasOutValue()) { @@ -240,43 +202,33 @@ return; } - DexClassAndMethod singleTarget = invoke.lookupSingleTarget(appView, context); - ProguardMemberRuleLookup lookup = lookupMemberRule(singleTarget); - if (lookup == null) { - // -assumenosideeffects rules are applied to upward visible and overriding methods, but only - // references that have actual definitions are marked by the root set builder. So, here, we - // try again with a resolved target, not the direct definition, which may not exist. - DexEncodedMethod resolutionTarget = - appView.appInfo().unsafeResolveMethodDueToDexFormat(invokedMethod).getSingleTarget(); - lookup = lookupMemberRule(resolutionTarget); - } - - if (lookup != null) { - // Check to see if a constant value can be assumed. - // But, if the current matched rule is -assumenosideeffects without the return value, it won't - // be transformed into a replacement instruction. Check if there is -assumevalues rule bound - // to the target. - if (singleTarget != null - && lookup.type == RuleType.ASSUME_NO_SIDE_EFFECTS - && !lookup.rule.hasReturnValue()) { - ProguardMemberRule rule = appView.appInfo().assumedValues.get(singleTarget.getReference()); - if (rule != null) { - lookup = new ProguardMemberRuleLookup(RuleType.ASSUME_VALUES, rule); - } - } - if (tryConstantReplacementFromProguard( - code, affectedValues, blocks, iterator, invoke, lookup)) { - return; - } - } - - // No Proguard rule could replace the instruction check for knowledge about the return value. - if (singleTarget == null || !mayPropagateValueFor(singleTarget)) { + SingleResolutionResult resolutionResult = + appView.appInfo().unsafeResolveMethodDueToDexFormat(invokedMethod).asSingleResolution(); + if (resolutionResult == null) { return; } - AbstractValue abstractReturnValue = - singleTarget.getDefinition().getOptimizationInfo().getAbstractReturnValue(); + DexClassAndMethod singleTarget = invoke.lookupSingleTarget(appView, context); + AssumeInfo lookup = AssumeInfoLookup.lookupAssumeInfo(appView, resolutionResult, singleTarget); + if (lookup != null + && applyAssumeInfoIfPossible(code, affectedValues, blocks, iterator, invoke, lookup)) { + return; + } + + // No Proguard rule could replace the instruction check for knowledge about the return value. + if (singleTarget != null && !mayPropagateValueFor(singleTarget)) { + return; + } + + AbstractValue abstractReturnValue; + if (invokedMethod.getReturnType().isAlwaysNull(appView)) { + abstractReturnValue = appView.abstractValueFactory().createNullValue(); + } else if (singleTarget != null) { + abstractReturnValue = + singleTarget.getDefinition().getOptimizationInfo().getAbstractReturnValue(); + } else { + abstractReturnValue = UnknownValue.getInstance(); + } if (abstractReturnValue.isSingleValue()) { SingleValue singleReturnValue = abstractReturnValue.asSingleValue(); @@ -293,7 +245,7 @@ if (invoke.isInvokeMethodWithReceiver()) { iterator.replaceCurrentInstructionByNullCheckIfPossible(appView, context); - } else if (invoke.isInvokeStatic()) { + } else if (invoke.isInvokeStatic() && singleTarget != null) { iterator.replaceCurrentInstructionByInitClassIfPossible( appView, code, singleTarget.getHolderType()); } @@ -308,7 +260,10 @@ } else { iterator.add(replacement); } - singleTarget.getDefinition().getMutableOptimizationInfo().markAsPropagated(); + + if (singleTarget != null) { + singleTarget.getDefinition().getMutableOptimizationInfo().markAsPropagated(); + } } } } @@ -322,8 +277,9 @@ DexField field = current.getField(); // TODO(b/123857022): Should be able to use definitionFor(). - DexEncodedField target = appView.appInfo().resolveField(field).getResolvedField(); - if (target == null) { + SuccessfulFieldResolutionResult resolutionResult = + appView.appInfo().resolveField(field).asSuccessfulResolution(); + if (resolutionResult == null) { boolean replaceCurrentInstructionWithConstNull = appView.withGeneratedExtensionRegistryShrinker( shrinker -> shrinker.wasRemoved(field), false); @@ -333,7 +289,9 @@ return; } - if (target.isStatic() != current.isStaticGet()) { + DexClassAndField target = resolutionResult.getResolutionPair(); + DexEncodedField definition = target.getDefinition(); + if (definition.isStatic() != current.isStaticGet()) { return; } @@ -342,33 +300,35 @@ } // Check if there is a Proguard configuration rule that specifies the value of the field. - ProguardMemberRuleLookup lookup = lookupMemberRule(target); + AssumeInfo lookup = AssumeInfoLookup.lookupAssumeInfo(appView, target); if (lookup != null - && tryConstantReplacementFromProguard( - code, affectedValues, blocks, iterator, current, lookup)) { + && applyAssumeInfoIfPossible(code, affectedValues, blocks, iterator, current, lookup)) { return; } AbstractValue abstractValue; if (field.getType().isAlwaysNull(appView)) { abstractValue = appView.abstractValueFactory().createSingleNumberValue(0); - } else if (appView.appInfo().isFieldWrittenByFieldPutInstruction(target)) { - abstractValue = target.getOptimizationInfo().getAbstractValue(); - if (abstractValue.isUnknown() && !target.isStatic()) { + } else if (appView.appInfo().isFieldWrittenByFieldPutInstruction(definition)) { + abstractValue = definition.getOptimizationInfo().getAbstractValue(); + if (abstractValue.isUnknown() && !definition.isStatic()) { AbstractValue abstractReceiverValue = current.asInstanceGet().object().getAbstractValue(appView, code.context()); if (abstractReceiverValue.isSingleFieldValue()) { abstractValue = - abstractReceiverValue.asSingleFieldValue().getState().getAbstractFieldValue(target); + abstractReceiverValue + .asSingleFieldValue() + .getState() + .getAbstractFieldValue(definition); } } - } else if (target.isStatic()) { + } else if (definition.isStatic()) { // This is guaranteed to read the static value of the field. - abstractValue = target.getStaticValue().toAbstractValue(appView.abstractValueFactory()); + abstractValue = definition.getStaticValue().toAbstractValue(appView.abstractValueFactory()); // Verify that the optimization info is consistent with the static value. - assert target.getOptimizationInfo().getAbstractValue().isUnknown() - || !target.hasExplicitStaticValue() - || abstractValue == target.getOptimizationInfo().getAbstractValue(); + assert definition.getOptimizationInfo().getAbstractValue().isUnknown() + || !definition.hasExplicitStaticValue() + || abstractValue == definition.getOptimizationInfo().getAbstractValue(); } else { // This is guaranteed to read the default value of the field. abstractValue = appView.abstractValueFactory().createSingleNumberValue(0); @@ -411,7 +371,8 @@ } else { iterator.add(replacement); } - feedback.markFieldAsPropagated(target); + + feedback.markFieldAsPropagated(definition); } } }
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/membervaluepropagation/assume/AssumeInfo.java b/src/main/java/com/android/tools/r8/ir/optimize/membervaluepropagation/assume/AssumeInfo.java new file mode 100644 index 0000000..a704abb --- /dev/null +++ b/src/main/java/com/android/tools/r8/ir/optimize/membervaluepropagation/assume/AssumeInfo.java
@@ -0,0 +1,67 @@ +// Copyright (c) 2021, 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.membervaluepropagation.assume; + +import com.android.tools.r8.shaking.ProguardMemberRule; +import com.android.tools.r8.shaking.ProguardMemberRuleReturnValue; + +public class AssumeInfo { + + public enum AssumeType { + ASSUME_NO_SIDE_EFFECTS, + ASSUME_VALUES; + + AssumeType meet(AssumeType type) { + return this == ASSUME_NO_SIDE_EFFECTS || type == ASSUME_NO_SIDE_EFFECTS + ? ASSUME_NO_SIDE_EFFECTS + : ASSUME_VALUES; + } + } + + private final AssumeType type; + private final ProguardMemberRule rule; + + public AssumeInfo(AssumeType type, ProguardMemberRule rule) { + this.type = type; + this.rule = rule; + } + + public boolean hasReturnInfo() { + return rule.hasReturnValue(); + } + + public ProguardMemberRuleReturnValue getReturnInfo() { + return rule.getReturnValue(); + } + + public boolean isAssumeNoSideEffects() { + return type == AssumeType.ASSUME_NO_SIDE_EFFECTS; + } + + public boolean isAssumeValues() { + return type == AssumeType.ASSUME_VALUES; + } + + public AssumeInfo meet(AssumeInfo lookup) { + return new AssumeInfo(type.meet(lookup.type), rule.hasReturnValue() ? rule : lookup.rule); + } + + @Override + public boolean equals(Object other) { + if (other == null) { + return false; + } + if (!(other instanceof AssumeInfo)) { + return false; + } + AssumeInfo assumeInfo = (AssumeInfo) other; + return type == assumeInfo.type && rule == assumeInfo.rule; + } + + @Override + public int hashCode() { + return type.ordinal() * 31 + rule.hashCode(); + } +}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/membervaluepropagation/assume/AssumeInfoLookup.java b/src/main/java/com/android/tools/r8/ir/optimize/membervaluepropagation/assume/AssumeInfoLookup.java new file mode 100644 index 0000000..5ddba94 --- /dev/null +++ b/src/main/java/com/android/tools/r8/ir/optimize/membervaluepropagation/assume/AssumeInfoLookup.java
@@ -0,0 +1,51 @@ +// Copyright (c) 2021, 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.membervaluepropagation.assume; + +import com.android.tools.r8.graph.AppView; +import com.android.tools.r8.graph.DexClassAndMember; +import com.android.tools.r8.graph.DexClassAndMethod; +import com.android.tools.r8.graph.DexMember; +import com.android.tools.r8.graph.ResolutionResult.SingleResolutionResult; +import com.android.tools.r8.ir.optimize.membervaluepropagation.assume.AssumeInfo.AssumeType; +import com.android.tools.r8.shaking.AppInfoWithLiveness; +import com.android.tools.r8.shaking.ProguardMemberRule; + +public class AssumeInfoLookup { + + public static AssumeInfo lookupAssumeInfo( + AppView<AppInfoWithLiveness> appView, + SingleResolutionResult resolutionResult, + DexClassAndMethod singleTarget) { + AssumeInfo resolutionLookup = lookupAssumeInfo(appView, resolutionResult.getResolutionPair()); + if (resolutionLookup == null) { + return singleTarget != null ? lookupAssumeInfo(appView, singleTarget) : null; + } + AssumeInfo singleTargetLookup = + singleTarget != null ? lookupAssumeInfo(appView, singleTarget) : null; + return singleTargetLookup != null + ? resolutionLookup.meet(singleTargetLookup) + : resolutionLookup; + } + + public static AssumeInfo lookupAssumeInfo( + AppView<AppInfoWithLiveness> appView, DexClassAndMember<?, ?> member) { + DexMember<?, ?> reference = member.getReference(); + ProguardMemberRule assumeNoSideEffectsRule = appView.appInfo().noSideEffects.get(reference); + ProguardMemberRule assumeValuesRule = appView.appInfo().assumedValues.get(reference); + if (assumeNoSideEffectsRule == null && assumeValuesRule == null) { + return null; + } + AssumeType type = + assumeNoSideEffectsRule != null + ? AssumeType.ASSUME_NO_SIDE_EFFECTS + : AssumeType.ASSUME_VALUES; + if ((assumeNoSideEffectsRule != null && assumeNoSideEffectsRule.hasReturnValue()) + || assumeValuesRule == null) { + return new AssumeInfo(type, assumeNoSideEffectsRule); + } + return new AssumeInfo(type, assumeValuesRule); + } +}
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardMemberRuleReturnValue.java b/src/main/java/com/android/tools/r8/shaking/ProguardMemberRuleReturnValue.java index c8545c7..11b8868 100644 --- a/src/main/java/com/android/tools/r8/shaking/ProguardMemberRuleReturnValue.java +++ b/src/main/java/com/android/tools/r8/shaking/ProguardMemberRuleReturnValue.java
@@ -60,6 +60,10 @@ return type == Type.FIELD; } + public boolean isNonNull() { + return isValueRange() && getValueRange().getMin() > 0; + } + public boolean isNull() { return type == Type.NULL; }
diff --git a/src/test/java/com/android/tools/r8/ir/analysis/type/NullabilityTest.java b/src/test/java/com/android/tools/r8/ir/analysis/type/NullabilityTest.java index a4c9785..3a6f0e4 100644 --- a/src/test/java/com/android/tools/r8/ir/analysis/type/NullabilityTest.java +++ b/src/test/java/com/android/tools/r8/ir/analysis/type/NullabilityTest.java
@@ -13,7 +13,6 @@ import com.android.tools.r8.TestParameters; import com.android.tools.r8.TestParametersCollection; -import com.android.tools.r8.graph.AppInfoWithClassHierarchy; import com.android.tools.r8.graph.AppView; import com.android.tools.r8.graph.DexType; import com.android.tools.r8.ir.code.Argument; @@ -33,6 +32,7 @@ import com.android.tools.r8.ir.optimize.nonnull.NonNullAfterFieldAccess; import com.android.tools.r8.ir.optimize.nonnull.NonNullAfterInvoke; import com.android.tools.r8.naming.MemberNaming.MethodSignature; +import com.android.tools.r8.shaking.AppInfoWithLiveness; import com.android.tools.r8.utils.DescriptorUtils; import com.android.tools.r8.utils.Timing; import com.android.tools.r8.utils.codeinspector.CodeInspector; @@ -63,7 +63,7 @@ boolean npeCaught, BiConsumer<AppView<?>, IRCode> inspector) throws Exception { - AppView<? extends AppInfoWithClassHierarchy> appView = build(mainClass); + AppView<AppInfoWithLiveness> appView = build(mainClass); CodeInspector codeInspector = new CodeInspector(appView.appInfo().app()); MethodSubject fooSubject = codeInspector.clazz(mainClass.getName()).method(signature); IRCode irCode = fooSubject.buildIR();
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/NonNullTrackerTest.java b/src/test/java/com/android/tools/r8/ir/optimize/NonNullTrackerTest.java index daa21f4..f5431bb 100644 --- a/src/test/java/com/android/tools/r8/ir/optimize/NonNullTrackerTest.java +++ b/src/test/java/com/android/tools/r8/ir/optimize/NonNullTrackerTest.java
@@ -9,7 +9,6 @@ import com.android.tools.r8.TestParameters; import com.android.tools.r8.TestParametersCollection; -import com.android.tools.r8.graph.AppInfoWithClassHierarchy; import com.android.tools.r8.graph.AppView; import com.android.tools.r8.ir.code.IRCode; import com.android.tools.r8.ir.code.InstancePut; @@ -22,6 +21,7 @@ import com.android.tools.r8.ir.optimize.nonnull.NonNullAfterInvoke; import com.android.tools.r8.ir.optimize.nonnull.NonNullAfterNullCheck; import com.android.tools.r8.naming.MemberNaming.MethodSignature; +import com.android.tools.r8.shaking.AppInfoWithLiveness; import com.android.tools.r8.utils.Timing; import com.android.tools.r8.utils.codeinspector.CodeInspector; import com.android.tools.r8.utils.codeinspector.MethodSubject; @@ -49,7 +49,7 @@ int expectedNumberOfNonNull, Consumer<IRCode> testAugmentedIRCode) throws Exception { - AppView<? extends AppInfoWithClassHierarchy> appView = build(testClass); + AppView<AppInfoWithLiveness> appView = build(testClass); CodeInspector codeInspector = new CodeInspector(appView.appInfo().app()); MethodSubject fooSubject = codeInspector.clazz(testClass.getName()).method(signature); IRCode code = fooSubject.buildIR();
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/NonNullTrackerTestBase.java b/src/test/java/com/android/tools/r8/ir/optimize/NonNullTrackerTestBase.java index 6b5f0cb..7d5e23e 100644 --- a/src/test/java/com/android/tools/r8/ir/optimize/NonNullTrackerTestBase.java +++ b/src/test/java/com/android/tools/r8/ir/optimize/NonNullTrackerTestBase.java
@@ -5,13 +5,12 @@ import com.android.tools.r8.TestBase; import com.android.tools.r8.ToolHelper; -import com.android.tools.r8.graph.AppInfoWithClassHierarchy; import com.android.tools.r8.graph.AppView; +import com.android.tools.r8.shaking.AppInfoWithLiveness; public abstract class NonNullTrackerTestBase extends TestBase { - protected AppView<? extends AppInfoWithClassHierarchy> build(Class<?> mainClass) - throws Exception { - return computeAppViewWithSubtyping(buildAndroidApp(ToolHelper.getClassAsBytes(mainClass))); + protected AppView<AppInfoWithLiveness> build(Class<?> mainClass) throws Exception { + return computeAppViewWithLiveness(buildAndroidApp(ToolHelper.getClassAsBytes(mainClass))); } }
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/membervaluepropagation/AssumeNotNullTest.java b/src/test/java/com/android/tools/r8/ir/optimize/membervaluepropagation/AssumeNotNullTest.java new file mode 100644 index 0000000..fda891e --- /dev/null +++ b/src/test/java/com/android/tools/r8/ir/optimize/membervaluepropagation/AssumeNotNullTest.java
@@ -0,0 +1,130 @@ +// Copyright (c) 2021, 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.membervaluepropagation; + +import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import com.android.tools.r8.NeverInline; +import com.android.tools.r8.TestBase; +import com.android.tools.r8.TestParameters; +import com.android.tools.r8.utils.codeinspector.ClassSubject; +import com.android.tools.r8.utils.codeinspector.InstructionSubject; +import com.android.tools.r8.utils.codeinspector.MethodSubject; +import com.google.common.collect.ImmutableList; +import java.util.List; +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 AssumeNotNullTest extends TestBase { + + private final String flavor; + private final TestParameters parameters; + + @Parameters(name = "{1}, flavor: {0}") + public static List<Object[]> data() { + return buildParameters( + ImmutableList.of("assumenosideeffects", "assumevalues"), + getTestParameters().withAllRuntimesAndApiLevels().build()); + } + + public AssumeNotNullTest(String flavor, TestParameters parameters) { + this.flavor = flavor; + this.parameters = parameters; + } + + @Test + public void test() throws Exception { + testForR8(parameters.getBackend()) + .addInnerClasses(getClass()) + .addKeepMainRule(Main.class) + .addKeepRules( + "-" + flavor + " class " + Factory.class.getTypeName() + " {", + " java.lang.Object create() return 1;", + "}", + "-" + flavor + " class " + Singleton.class.getTypeName() + " {", + " java.lang.Object INSTANCE return 1;", + "}") + .enableInliningAnnotations() + .setMinApi(parameters.getApiLevel()) + .compile() + .inspect( + inspector -> { + MethodSubject mainMethodSubject = inspector.clazz(Main.class).mainMethod(); + assertThat(mainMethodSubject, isPresent()); + if (flavor.equals("assumenosideeffects")) { + // With -assumenosideeffects, the method should become empty. + assertTrue( + mainMethodSubject + .streamInstructions() + .allMatch(InstructionSubject::isReturnVoid)); + } else { + // With -assumevalues, the Singleton.INSTANCE access should remain along with the + // Factory.create() invoke. + ClassSubject factoryClassSubject = inspector.clazz(Factory.class); + assertThat(factoryClassSubject, isPresent()); + + ClassSubject singletonClassSubject = inspector.clazz(Singleton.class); + assertThat(singletonClassSubject, isPresent()); + + assertEquals( + 2, + mainMethodSubject + .streamInstructions() + .filter( + instruction -> instruction.isFieldAccess() || instruction.isInvoke()) + .count()); + assertTrue( + mainMethodSubject + .streamInstructions() + .anyMatch( + instruction -> + instruction.isStaticGet() + && instruction.getField().getHolderType() + == singletonClassSubject.getDexProgramClass().getType())); + assertTrue( + mainMethodSubject + .streamInstructions() + .filter(InstructionSubject::isInvoke) + .anyMatch( + instruction -> + instruction.getMethod().getHolderType() + == factoryClassSubject.getDexProgramClass().getType())); + } + }) + .run(parameters.getRuntime(), Main.class) + .assertSuccessWithEmptyOutput(); + } + + static class Main { + + public static void main(String[] args) { + if (Singleton.INSTANCE == null) { + System.out.println("Foo"); + } + if (Factory.create() == null) { + System.out.println("Bar"); + } + } + } + + static class Factory { + + @NeverInline + public static Object create() { + return System.currentTimeMillis() > 0 ? new Object() : null; + } + } + + static class Singleton { + + public static Object INSTANCE = System.currentTimeMillis() > 0 ? new Object() : null; + } +}