Leverage member value propagation for optimizing methods that return null
Bug: 150269949
Change-Id: I0984d9bb11ec688c9addea543401892e8582b1f8
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeMethodWithReceiver.java b/src/main/java/com/android/tools/r8/ir/code/InvokeMethodWithReceiver.java
index f3a889f..a83c964 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokeMethodWithReceiver.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokeMethodWithReceiver.java
@@ -12,7 +12,7 @@
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.ProgramMethod;
-import com.android.tools.r8.graph.ResolutionResult;
+import com.android.tools.r8.graph.ResolutionResult.SingleResolutionResult;
import com.android.tools.r8.ir.analysis.ClassInitializationAnalysis;
import com.android.tools.r8.ir.analysis.type.ClassTypeElement;
import com.android.tools.r8.ir.analysis.type.TypeAnalysis;
@@ -209,9 +209,12 @@
assert appView.appInfo().hasLiveness();
AppView<AppInfoWithLiveness> appViewWithLiveness = appView.withLiveness();
- ResolutionResult resolutionResult =
- appViewWithLiveness.appInfo().resolveMethod(getInvokedMethod(), getInterfaceBit());
- if (resolutionResult.isFailedResolution()) {
+ SingleResolutionResult resolutionResult =
+ appViewWithLiveness
+ .appInfo()
+ .resolveMethod(getInvokedMethod(), getInterfaceBit())
+ .asSingleResolution();
+ if (resolutionResult == null) {
return true;
}
@@ -222,6 +225,12 @@
return true;
}
+ DexEncodedMethod resolvedMethod = resolutionResult.getResolvedMethod();
+ if (appViewWithLiveness.appInfo().noSideEffects.containsKey(getInvokedMethod())
+ || appViewWithLiveness.appInfo().noSideEffects.containsKey(resolvedMethod.getReference())) {
+ return false;
+ }
+
// Find the target and check if the invoke may have side effects.
DexEncodedMethod target = lookupSingleTarget(appViewWithLiveness, context);
if (target == null) {
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 51c1c01..8ca93eb 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
@@ -230,21 +230,23 @@
Set<Value> affectedValues,
ListIterator<BasicBlock> blocks,
InstructionListIterator iterator,
- InvokeMethod current) {
- DexMethod invokedMethod = current.getInvokedMethod();
+ InvokeMethod invoke) {
+ DexMethod invokedMethod = invoke.getInvokedMethod();
+ if (invokedMethod.proto.returnType.isVoidType()) {
+ return;
+ }
+
+ if (!invoke.hasOutValue() || !invoke.outValue().hasNonDebugUsers()) {
+ return;
+ }
+
DexType invokedHolder = invokedMethod.holder;
if (!invokedHolder.isClassType()) {
return;
}
- DexEncodedMethod target = current.lookupSingleTarget(appView, context);
- if (target != null && target.isInstanceInitializer()) {
- // Member value propagation does not apply to constructors. Removing a call to a constructor
- // that is marked as having no side effects could lead to verification errors, due to
- // uninitialized instances being used.
- return;
- }
- ProguardMemberRuleLookup lookup = lookupMemberRule(target);
+ DexEncodedMethod 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
@@ -253,60 +255,56 @@
appView.appInfo().unsafeResolveMethodDueToDexFormat(invokedMethod).getSingleTarget();
lookup = lookupMemberRule(resolutionTarget);
}
- boolean invokeReplaced = false;
- if (lookup != null) {
- boolean hasUsedOutValue = current.hasOutValue() && current.outValue().isUsed();
- if (!hasUsedOutValue) {
- if (lookup.type == RuleType.ASSUME_NO_SIDE_EFFECTS) {
- // Remove invoke if marked as having no side effects and the return value is not used.
- iterator.removeOrReplaceByDebugLocalRead();
- }
- return;
- }
+ 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 (target != null
+ if (singleTarget != null
&& lookup.type == RuleType.ASSUME_NO_SIDE_EFFECTS
&& !lookup.rule.hasReturnValue()) {
- ProguardMemberRule rule = appView.appInfo().assumedValues.get(target.toReference());
+ ProguardMemberRule rule = appView.appInfo().assumedValues.get(singleTarget.toReference());
if (rule != null) {
lookup = new ProguardMemberRuleLookup(RuleType.ASSUME_VALUES, rule);
}
}
- invokeReplaced =
- tryConstantReplacementFromProguard(
- code, affectedValues, blocks, iterator, current, lookup);
+ if (tryConstantReplacementFromProguard(
+ code, affectedValues, blocks, iterator, invoke, lookup)) {
+ return;
+ }
}
- if (invokeReplaced || !current.hasOutValue()) {
- return;
- }
+
// No Proguard rule could replace the instruction check for knowledge about the return value.
- if (target == null || !mayPropagateValueFor(target)) {
+ if (singleTarget == null || !mayPropagateValueFor(singleTarget)) {
return;
}
- AbstractValue abstractReturnValue = target.getOptimizationInfo().getAbstractReturnValue();
+ AbstractValue abstractReturnValue;
+ if (singleTarget.returnType().isAlwaysNull(appView)) {
+ abstractReturnValue = appView.abstractValueFactory().createSingleNumberValue(0);
+ } else {
+ abstractReturnValue = singleTarget.getOptimizationInfo().getAbstractReturnValue();
+ }
+
if (abstractReturnValue.isSingleValue()) {
SingleValue singleReturnValue = abstractReturnValue.asSingleValue();
if (singleReturnValue.isMaterializableInContext(appView, context)) {
- BasicBlock block = current.getBlock();
- Position position = current.getPosition();
+ BasicBlock block = invoke.getBlock();
+ Position position = invoke.getPosition();
Instruction replacement =
- singleReturnValue.createMaterializingInstruction(appView, code, current);
- affectedValues.addAll(current.outValue().affectedValues());
- current.moveDebugValues(replacement);
- current.outValue().replaceUsers(replacement.outValue());
- current.setOutValue(null);
+ singleReturnValue.createMaterializingInstruction(appView, code, invoke);
+ affectedValues.addAll(invoke.outValue().affectedValues());
+ invoke.moveDebugValues(replacement);
+ invoke.outValue().replaceUsers(replacement.outValue());
+ invoke.setOutValue(null);
- if (current.isInvokeMethodWithReceiver()) {
- replaceInstructionByNullCheckIfPossible(current, iterator, context);
- } else if (current.isInvokeStatic()) {
+ if (invoke.isInvokeMethodWithReceiver()) {
+ replaceInstructionByNullCheckIfPossible(invoke, iterator, context);
+ } else if (invoke.isInvokeStatic()) {
replaceInstructionByInitClassIfPossible(
- current, target.holder(), code, iterator, context);
+ invoke, singleTarget.holder(), code, iterator, context);
}
// Insert the definition of the replacement.
@@ -316,7 +314,7 @@
} else {
iterator.add(replacement);
}
- target.getMutableOptimizationInfo().markAsPropagated();
+ singleTarget.getMutableOptimizationInfo().markAsPropagated();
}
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java b/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java
index da5994e..11245e7 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java
@@ -326,7 +326,6 @@
}
public void rewrite(IRCode code) {
- AssumeRemover assumeRemover = new AssumeRemover(appView, code);
Set<BasicBlock> blocksToBeRemoved = Sets.newIdentityHashSet();
ListIterator<BasicBlock> blockIterator = code.listIterator();
Set<Value> affectedValues = Sets.newIdentityHashSet();
@@ -352,13 +351,11 @@
blockIterator,
instructionIterator,
code,
- assumeRemover,
affectedValues,
blocksToBeRemoved);
}
}
}
- assumeRemover.removeMarkedInstructions(blocksToBeRemoved).finish();
code.removeBlocks(blocksToBeRemoved);
code.removeAllDeadAndTrivialPhis(affectedValues);
code.removeUnreachableBlocks();
@@ -398,7 +395,6 @@
ListIterator<BasicBlock> blockIterator,
InstructionListIterator instructionIterator,
IRCode code,
- AssumeRemover assumeRemover,
Set<Value> affectedValues,
Set<BasicBlock> blocksToBeRemoved) {
DexEncodedMethod target = invoke.lookupSingleTarget(appView, code.context());
@@ -418,30 +414,5 @@
}
}
}
-
- DexType returnType = target.method.proto.returnType;
- if (returnType.isAlwaysNull(appView)) {
- replaceOutValueByNull(invoke, instructionIterator, code, assumeRemover, affectedValues);
- }
- }
-
- private void replaceOutValueByNull(
- Instruction instruction,
- InstructionListIterator instructionIterator,
- IRCode code,
- AssumeRemover assumeRemover,
- Set<Value> affectedValues) {
- assert instructionIterator.peekPrevious() == instruction;
- if (instruction.hasOutValue()) {
- Value outValue = instruction.outValue();
- if (outValue.numberOfAllUsers() > 0) {
- assumeRemover.markAssumeDynamicTypeUsersForRemoval(outValue);
- instructionIterator.previous();
- affectedValues.addAll(outValue.affectedValues());
- outValue.replaceUsers(
- instructionIterator.insertConstNullInstruction(code, appView.options()));
- instructionIterator.next();
- }
- }
}
}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/uninstantiatedtypes/B146957343.java b/src/test/java/com/android/tools/r8/ir/optimize/uninstantiatedtypes/B146957343.java
index 289e095..3940548 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/uninstantiatedtypes/B146957343.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/uninstantiatedtypes/B146957343.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.ir.optimize.uninstantiatedtypes;
+import com.android.tools.r8.NeverInline;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
@@ -46,7 +47,7 @@
.addProgramClasses(I.class, J.class, Main.class)
.addProgramClassFileData(getAimplementsI())
.addKeepMainRule(Main.class)
- .addKeepRules("-keep class **A { createA(); }")
+ .enableInliningAnnotations()
.setMinApi(parameters.getApiLevel())
.addOptionsModification(
options -> options.enableUninstantiatedTypeOptimizationForInterfaces = true)
@@ -62,7 +63,7 @@
.addProgramClasses(I.class, J.class, Main.class)
.addProgramClassFileData(getAimplementsI())
.addKeepMainRule(Main.class)
- .addKeepRules("-keep class **A { createA(); }")
+ .enableInliningAnnotations()
.setMinApi(parameters.getApiLevel())
.addOptionsModification(
options -> options.enableUninstantiatedTypeOptimizationForInterfaces = false)
@@ -76,10 +77,13 @@
public interface J extends I {}
public static class A implements J {
+
+ @NeverInline
public static J createA() {
return new A();
}
+ @NeverInline
public void f() {
System.out.println("In A.f()");
}
diff --git a/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsPropagationWithoutMatchingDefinitionTest.java b/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsPropagationWithoutMatchingDefinitionTest.java
index bc842e5..9c38214 100644
--- a/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsPropagationWithoutMatchingDefinitionTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsPropagationWithoutMatchingDefinitionTest.java
@@ -116,7 +116,9 @@
@NeverInline
private static void testInvokeInterface(LoggerInterface logger, String message) {
- logger.debug(TAG, message);
+ if (logger != null) {
+ logger.debug(TAG, message);
+ }
}
@NeverInline
diff --git a/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsWithMultipleTargetsTest.java b/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsWithMultipleTargetsTest.java
index 29e4588..34b1b33 100644
--- a/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsWithMultipleTargetsTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsWithMultipleTargetsTest.java
@@ -138,7 +138,9 @@
@NeverInline
private static void testInvokeInterface(TestLogger logger, String message) {
- logger.info(TAG, message);
+ if (logger != null) {
+ logger.info(TAG, message);
+ }
}
public static void main(String... args) {
