Remove references to pruned methods in argument propagation
This fixes a bug where argument propagation proves that a direct method is dead. Argument propagation then removes the method from the app, but does not forcefully remove all calls to the method (which indirectly have been proved to also be dead).
This CL makes an update to argument propagation so that the removal of the dead methods is deferred until all calls to these methods have been removed.
Change-Id: Id71255467e2daae1395550e5d5bb15870d4fd396
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagator.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagator.java
index 20741e0..c4c931f 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagator.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagator.java
@@ -4,12 +4,13 @@
package com.android.tools.r8.optimize.argumentpropagation;
+import static com.android.tools.r8.utils.MapUtils.ignoreKey;
+
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexMethodSignature;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.ImmediateProgramSubtypingInfo;
import com.android.tools.r8.graph.ProgramMethod;
-import com.android.tools.r8.graph.PrunedItems;
import com.android.tools.r8.ir.analysis.path.PathConstraintSupplier;
import com.android.tools.r8.ir.code.AbstractValueSupplier;
import com.android.tools.r8.ir.code.IRCode;
@@ -30,6 +31,7 @@
import com.android.tools.r8.utils.ThreadUtils;
import com.android.tools.r8.utils.Timing;
import com.android.tools.r8.utils.collections.DexMethodSignatureSet;
+import com.android.tools.r8.utils.collections.ProgramMethodSet;
import java.util.IdentityHashMap;
import java.util.List;
import java.util.Map;
@@ -209,19 +211,20 @@
// Set the optimization info on each method.
Map<Set<DexProgramClass>, DexMethodSignatureSet> interfaceDispatchOutsideProgram =
new IdentityHashMap<>();
- populateParameterOptimizationInfo(
- converter,
- immediateSubtypingInfo,
- stronglyConnectedProgramComponents,
- (stronglyConnectedProgramComponent, signature) -> {
- interfaceDispatchOutsideProgram
- .computeIfAbsent(
- stronglyConnectedProgramComponent, (unused) -> DexMethodSignatureSet.create())
- .add(signature);
- },
- postMethodProcessorBuilder,
- executorService,
- timing);
+ ProgramMethodSet prunedMethods =
+ populateParameterOptimizationInfo(
+ converter,
+ immediateSubtypingInfo,
+ stronglyConnectedProgramComponents,
+ (stronglyConnectedProgramComponent, signature) -> {
+ interfaceDispatchOutsideProgram
+ .computeIfAbsent(
+ stronglyConnectedProgramComponent, (unused) -> DexMethodSignatureSet.create())
+ .add(signature);
+ },
+ postMethodProcessorBuilder,
+ executorService,
+ timing);
// Using the computed optimization info, build a graph lens that describes the mapping from
// methods with constant parameters to methods with the constant parameters removed.
@@ -232,11 +235,14 @@
.run(stronglyConnectedProgramComponents, affectedClasses::add, executorService, timing);
// Find all the code objects that need reprocessing.
- new ArgumentPropagatorMethodReprocessingEnqueuer(appView, reprocessingCriteriaCollection)
+ new ArgumentPropagatorMethodReprocessingEnqueuer(
+ appView, prunedMethods, reprocessingCriteriaCollection)
.enqueueAndPrepareMethodsForReprocessing(
graphLens, postMethodProcessorBuilder, executorService, timing);
reprocessingCriteriaCollection = null;
+ removePrunedMethods(converter, postMethodProcessorBuilder, prunedMethods, executorService);
+
// Finally, apply the graph lens to the program (i.e., remove constant parameters from method
// definitions).
new ArgumentPropagatorApplicationFixer(appView, graphLens)
@@ -254,7 +260,7 @@
* Called by {@link IRConverter} *after* the primary optimization pass to populate the parameter
* optimization info.
*/
- private void populateParameterOptimizationInfo(
+ private ProgramMethodSet populateParameterOptimizationInfo(
PrimaryR8IRConverter converter,
ImmediateProgramSubtypingInfo immediateSubtypingInfo,
List<Set<DexProgramClass>> stronglyConnectedProgramComponents,
@@ -287,16 +293,40 @@
classesWithSingleCallerInlinedInstanceInitializers, executorService, timing);
classesWithSingleCallerInlinedInstanceInitializers = null;
- PrunedItems prunedItems =
+ ProgramMethodSet prunedMethods =
new ArgumentPropagatorOptimizationInfoPopulator(
appView, converter, fieldStates, methodStates, postMethodProcessorBuilder)
.populateOptimizationInfo(executorService, timing);
timing.end();
timing.begin("Compute unused arguments");
- effectivelyUnusedArgumentsAnalysis.computeEffectivelyUnusedArguments(prunedItems);
+ effectivelyUnusedArgumentsAnalysis.computeEffectivelyUnusedArguments(prunedMethods);
effectivelyUnusedArgumentsAnalysis = null;
timing.end();
+
+ return prunedMethods;
+ }
+
+ private void removePrunedMethods(
+ PrimaryR8IRConverter converter,
+ PostMethodProcessor.Builder postMethodProcessorBuilder,
+ ProgramMethodSet prunedMethods,
+ ExecutorService executorService)
+ throws ExecutionException {
+ Map<DexProgramClass, ProgramMethodSet> prunedMethodsPerClass = new IdentityHashMap<>();
+ prunedMethods.forEach(
+ prunedMethod ->
+ prunedMethodsPerClass
+ .computeIfAbsent(prunedMethod.getHolder(), ignoreKey(ProgramMethodSet::create))
+ .add(prunedMethod));
+ prunedMethodsPerClass.forEach(
+ (clazz, prunedMethodsInClass) ->
+ clazz.getMethodCollection().removeMethods(prunedMethodsInClass.toDefinitionSet()));
+ for (ProgramMethod prunedMethod : prunedMethods) {
+ converter.onMethodPruned(prunedMethod);
+ postMethodProcessorBuilder.remove(prunedMethod, appView.graphLens());
+ }
+ converter.pruneItems(executorService);
}
/**
@@ -316,9 +346,9 @@
|| method.getDefinition().belongsToDirectPool()
|| method.getAccessFlags().wasPrivate();
}
-
- assert effectivelyUnusedArgumentsAnalysis != null;
- effectivelyUnusedArgumentsAnalysis.onMethodPruned(method);
+ if (effectivelyUnusedArgumentsAnalysis != null) {
+ effectivelyUnusedArgumentsAnalysis.onMethodPruned(method);
+ }
}
public void onMethodCodePruned(ProgramMethod method) {
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorMethodReprocessingEnqueuer.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorMethodReprocessingEnqueuer.java
index 94a7584..4873821 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorMethodReprocessingEnqueuer.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorMethodReprocessingEnqueuer.java
@@ -24,7 +24,9 @@
import com.android.tools.r8.ir.code.BasicBlockIterator;
import com.android.tools.r8.ir.code.FieldInstruction;
import com.android.tools.r8.ir.code.IRCode;
+import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InstructionListIterator;
+import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.ir.conversion.IRConverter;
import com.android.tools.r8.ir.conversion.IRToLirFinalizer;
import com.android.tools.r8.ir.conversion.PostMethodProcessor;
@@ -41,6 +43,7 @@
import com.android.tools.r8.utils.ObjectUtils;
import com.android.tools.r8.utils.ThreadUtils;
import com.android.tools.r8.utils.Timing;
+import com.android.tools.r8.utils.collections.ProgramMethodSet;
import com.google.common.collect.Sets;
import java.util.ArrayList;
import java.util.Collection;
@@ -53,12 +56,15 @@
public class ArgumentPropagatorMethodReprocessingEnqueuer {
private final AppView<AppInfoWithLiveness> appView;
+ private final ProgramMethodSet prunedMethods;
private final ArgumentPropagatorReprocessingCriteriaCollection reprocessingCriteriaCollection;
public ArgumentPropagatorMethodReprocessingEnqueuer(
AppView<AppInfoWithLiveness> appView,
+ ProgramMethodSet prunedMethods,
ArgumentPropagatorReprocessingCriteriaCollection reprocessingCriteriaCollection) {
this.appView = appView;
+ this.prunedMethods = prunedMethods;
this.reprocessingCriteriaCollection = reprocessingCriteriaCollection;
}
@@ -91,7 +97,7 @@
timing.end();
timing.begin("Eliminate dead field accesses");
- eliminateDeadFieldAccesses(executorService);
+ eliminateDeadFieldAndMethodAccesses(executorService);
timing.end();
timing.end();
@@ -170,36 +176,35 @@
postMethodProcessorBuilder.addAll(methodsToReprocessInClass, currentGraphLens));
}
- private void eliminateDeadFieldAccesses(ExecutorService executorService)
+ private void eliminateDeadFieldAndMethodAccesses(ExecutorService executorService)
throws ExecutionException {
ThreadUtils.processItems(
appView.appInfo().classes(),
- clazz -> {
- clazz.forEachProgramMethodMatching(
- m -> m.hasCode() && !m.getCode().isSharedCodeObject(),
- this::eliminateDeadFieldAccesses);
- },
+ clazz ->
+ clazz.forEachProgramMethodMatching(
+ m -> m.hasCode() && !m.getCode().isSharedCodeObject(),
+ this::eliminateDeadFieldAndMethodAccesses),
appView.options().getThreadingModule(),
executorService);
}
- private void eliminateDeadFieldAccesses(ProgramMethod method) {
+ private void eliminateDeadFieldAndMethodAccesses(ProgramMethod method) {
Code code = method.getDefinition().getCode();
if (!code.isLirCode()) {
assert appView.isCfByteCodePassThrough(method);
return;
}
LirCode<Integer> lirCode = code.asLirCode();
- LirCode<Integer> rewrittenLirCode = eliminateDeadFieldAccesses(method, lirCode);
+ LirCode<Integer> rewrittenLirCode = eliminateDeadFieldAndMethodAccesses(method, lirCode);
if (ObjectUtils.notIdentical(lirCode, rewrittenLirCode)) {
method.setCode(rewrittenLirCode, appView);
}
}
- private LirCode<Integer> eliminateDeadFieldAccesses(
+ private LirCode<Integer> eliminateDeadFieldAndMethodAccesses(
ProgramMethod method, LirCode<Integer> lirCode) {
if (ArrayUtils.none(
- lirCode.getConstantPool(), constant -> isDeadFieldAccess(constant, method))) {
+ lirCode.getConstantPool(), constant -> isDeadFieldOrMethodAccess(constant, method))) {
return lirCode;
}
IRCode irCode = lirCode.buildIR(method, appView);
@@ -213,22 +218,34 @@
}
InstructionListIterator instructionIterator = block.listIterator();
while (instructionIterator.hasNext()) {
- FieldInstruction fieldInstruction = instructionIterator.next().asFieldInstruction();
- if (fieldInstruction == null) {
- continue;
- }
- ProgramField resolvedField =
- fieldInstruction.resolveField(appView, method).getProgramField();
- if (resolvedField == null || !isDeadFieldAccess(resolvedField)) {
- continue;
- }
- if (fieldInstruction.isStaticFieldInstruction()
- != resolvedField.getAccessFlags().isStatic()) {
- // Preserve the ICCE.
- instructionIterator.next();
- instructionIterator.replaceCurrentInstructionWithThrowNull(
- appView, irCode, blocks, blocksToRemove, affectedValues);
- } else {
+ Instruction instruction = instructionIterator.next();
+ if (instruction.isFieldInstruction()) {
+ FieldInstruction fieldInstruction = instruction.asFieldInstruction();
+ if (fieldInstruction == null) {
+ continue;
+ }
+ ProgramField resolvedField =
+ fieldInstruction.resolveField(appView, method).getProgramField();
+ if (resolvedField == null || !isDeadFieldAccess(resolvedField)) {
+ continue;
+ }
+ if (fieldInstruction.isStaticFieldInstruction()
+ != resolvedField.getAccessFlags().isStatic()) {
+ // Preserve the ICCE.
+ instructionIterator.next();
+ instructionIterator.replaceCurrentInstructionWithThrowNull(
+ appView, irCode, blocks, blocksToRemove, affectedValues);
+ } else {
+ instructionIterator.replaceCurrentInstructionWithThrowNull(
+ appView, irCode, blocks, blocksToRemove, affectedValues);
+ }
+ } else if (instruction.isInvokeMethod()) {
+ InvokeMethod invoke = instruction.asInvokeMethod();
+ ProgramMethod resolvedMethod =
+ invoke.resolveMethod(appView, method).getResolvedProgramMethod();
+ if (resolvedMethod == null || !isDeadMethodAccess(resolvedMethod)) {
+ continue;
+ }
instructionIterator.replaceCurrentInstructionWithThrowNull(
appView, irCode, blocks, blocksToRemove, affectedValues);
}
@@ -241,12 +258,15 @@
.finalizeCode(irCode, BytecodeMetadataProvider.empty(), Timing.empty());
}
- private boolean isDeadFieldAccess(LirConstant constant, ProgramMethod context) {
- if (!(constant instanceof DexField)) {
- return false;
+ private boolean isDeadFieldOrMethodAccess(LirConstant constant, ProgramMethod context) {
+ if (constant instanceof DexField) {
+ DexField fieldReference = (DexField) constant;
+ return isDeadFieldAccess(fieldReference, context);
+ } else if (constant instanceof DexMethod) {
+ DexMethod methodReference = (DexMethod) constant;
+ return isDeadMethodAccess(methodReference);
}
- DexField fieldReference = (DexField) constant;
- return isDeadFieldAccess(fieldReference, context);
+ return false;
}
private boolean isDeadFieldAccess(DexField fieldReference, ProgramMethod context) {
@@ -258,6 +278,19 @@
return field.getOptimizationInfo().getAbstractValue().isBottom();
}
+ private boolean isDeadMethodAccess(DexMethod methodReference) {
+ ProgramMethod resolvedMethod =
+ appView
+ .appInfo()
+ .unsafeResolveMethodDueToDexFormat(methodReference)
+ .getResolvedProgramMethod();
+ return resolvedMethod != null && isDeadMethodAccess(resolvedMethod);
+ }
+
+ private boolean isDeadMethodAccess(ProgramMethod method) {
+ return prunedMethods.contains(method);
+ }
+
static class AffectedMethodUseRegistry
extends DefaultUseRegistryWithResult<Boolean, ProgramMethod> {
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorOptimizationInfoPopulator.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorOptimizationInfoPopulator.java
index f18dc04..1228139 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorOptimizationInfoPopulator.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/ArgumentPropagatorOptimizationInfoPopulator.java
@@ -11,7 +11,6 @@
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.ProgramField;
import com.android.tools.r8.graph.ProgramMethod;
-import com.android.tools.r8.graph.PrunedItems;
import com.android.tools.r8.ir.analysis.type.DynamicType;
import com.android.tools.r8.ir.analysis.type.DynamicTypeWithUpperBound;
import com.android.tools.r8.ir.analysis.type.Nullability;
@@ -37,7 +36,6 @@
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.ListUtils;
import com.android.tools.r8.utils.OptionalBool;
-import com.android.tools.r8.utils.SetUtils;
import com.android.tools.r8.utils.ThreadUtils;
import com.android.tools.r8.utils.Timing;
import com.android.tools.r8.utils.collections.ProgramMethodSet;
@@ -77,18 +75,18 @@
* Computes an over-approximation of each parameter's value and type and stores the result in
* {@link com.android.tools.r8.ir.optimize.info.MethodOptimizationInfo}.
*/
- PrunedItems populateOptimizationInfo(ExecutorService executorService, Timing timing)
+ ProgramMethodSet populateOptimizationInfo(ExecutorService executorService, Timing timing)
throws ExecutionException {
// The information stored on each method is now sound, and can be used as optimization info.
timing.begin("Set optimization info");
- PrunedItems prunedItems = setOptimizationInfo(executorService);
+ ProgramMethodSet prunedMethods = setOptimizationInfo(executorService);
timing.end();
assert methodStates.isEmpty();
- return prunedItems;
+ return prunedMethods;
}
- private PrunedItems setOptimizationInfo(ExecutorService executorService)
+ private ProgramMethodSet setOptimizationInfo(ExecutorService executorService)
throws ExecutionException {
ProgramMethodSet prunedMethods = ProgramMethodSet.createConcurrent();
ThreadUtils.processItems(
@@ -96,23 +94,13 @@
clazz -> prunedMethods.addAll(setOptimizationInfo(clazz)),
appView.options().getThreadingModule(),
executorService);
- for (ProgramMethod prunedMethod : prunedMethods) {
- converter.onMethodPruned(prunedMethod);
- postMethodProcessorBuilder.remove(prunedMethod, appView.graphLens());
- }
- converter.pruneItems(executorService);
- converter.waveDone(ProgramMethodSet.empty(), executorService);
- return PrunedItems.builder()
- .setPrunedApp(appView.app())
- .setRemovedMethods(prunedMethods.toReferenceSet(SetUtils::newIdentityHashSet))
- .build();
+ return prunedMethods;
}
private ProgramMethodSet setOptimizationInfo(DexProgramClass clazz) {
ProgramMethodSet prunedMethods = ProgramMethodSet.create();
clazz.forEachProgramField(this::setOptimizationInfo);
clazz.forEachProgramMethod(method -> setOptimizationInfo(method, prunedMethods));
- clazz.getMethodCollection().removeMethods(prunedMethods.toDefinitionSet());
return prunedMethods;
}
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/unusedarguments/EffectivelyUnusedArgumentsAnalysis.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/unusedarguments/EffectivelyUnusedArgumentsAnalysis.java
index 8c88cfd..8ba3ce7 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/unusedarguments/EffectivelyUnusedArgumentsAnalysis.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/unusedarguments/EffectivelyUnusedArgumentsAnalysis.java
@@ -10,7 +10,6 @@
import com.android.tools.r8.graph.DexMethodSignature;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.ProgramMethod;
-import com.android.tools.r8.graph.PrunedItems;
import com.android.tools.r8.ir.analysis.path.PathConstraintSupplier;
import com.android.tools.r8.ir.analysis.path.state.ConcretePathConstraintAnalysisState;
import com.android.tools.r8.ir.analysis.path.state.PathConstraintKind;
@@ -147,11 +146,11 @@
new Analyzer(method, code, pathConstraintSupplier).analyze();
}
- public void computeEffectivelyUnusedArguments(PrunedItems prunedItems) {
+ public void computeEffectivelyUnusedArguments(ProgramMethodSet prunedMethods) {
// Build a graph where nodes are method parameters and there is an edge from method parameter p0
// to method parameter p1 if the removal of p0 depends on the removal of p1.
EffectivelyUnusedArgumentsGraph dependenceGraph =
- EffectivelyUnusedArgumentsGraph.create(appView, conditions, constraints, prunedItems);
+ EffectivelyUnusedArgumentsGraph.create(appView, conditions, constraints, prunedMethods);
// Remove all unoptimizable method parameters from the graph, as well as all nodes that depend
// on a node that is unoptimable.
diff --git a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/unusedarguments/EffectivelyUnusedArgumentsGraph.java b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/unusedarguments/EffectivelyUnusedArgumentsGraph.java
index c2fa1f0..a837362 100644
--- a/src/main/java/com/android/tools/r8/optimize/argumentpropagation/unusedarguments/EffectivelyUnusedArgumentsGraph.java
+++ b/src/main/java/com/android/tools/r8/optimize/argumentpropagation/unusedarguments/EffectivelyUnusedArgumentsGraph.java
@@ -8,7 +8,6 @@
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.ProgramMethod;
-import com.android.tools.r8.graph.PrunedItems;
import com.android.tools.r8.ir.analysis.value.AbstractValue;
import com.android.tools.r8.ir.optimize.info.MethodOptimizationInfo;
import com.android.tools.r8.optimize.argumentpropagation.codescanner.FlowGraphStateProvider;
@@ -16,6 +15,7 @@
import com.android.tools.r8.optimize.argumentpropagation.computation.ComputationTreeNode;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.utils.WorkList;
+import com.android.tools.r8.utils.collections.ProgramMethodSet;
import com.android.tools.r8.utils.dfs.DFSStack;
import com.android.tools.r8.utils.dfs.DFSWorklistItem;
import com.android.tools.r8.utils.dfs.DFSWorklistItem.NewlyVisitedDFSWorklistItem;
@@ -43,10 +43,13 @@
AppView<AppInfoWithLiveness> appView,
Map<MethodParameter, ComputationTreeNode> conditions,
Map<MethodParameter, Set<MethodParameter>> constraints,
- PrunedItems prunedItems) {
+ ProgramMethodSet prunedMethods) {
EffectivelyUnusedArgumentsGraph graph = new EffectivelyUnusedArgumentsGraph(appView);
conditions.forEach(
(methodParameter, condition) -> {
+ if (prunedMethods.contains(methodParameter.getMethod())) {
+ return;
+ }
ProgramMethod method =
asProgramMethodOrNull(appView.definitionFor(methodParameter.getMethod()));
if (method == null) {
@@ -71,9 +74,12 @@
});
constraints.forEach(
(methodParameter, constraintsForMethodParameter) -> {
+ if (prunedMethods.contains(methodParameter.getMethod())) {
+ return;
+ }
EffectivelyUnusedArgumentsGraphNode node = graph.getOrCreateNode(methodParameter);
for (MethodParameter constraint : constraintsForMethodParameter) {
- graph.addConstraintEdge(node, constraint, constraints, prunedItems);
+ graph.addConstraintEdge(node, constraint, constraints, prunedMethods);
}
});
return graph;
@@ -83,8 +89,8 @@
EffectivelyUnusedArgumentsGraphNode node,
MethodParameter constraint,
Map<MethodParameter, Set<MethodParameter>> constraints,
- PrunedItems prunedItems) {
- if (prunedItems.isRemoved(constraint.getMethod())) {
+ ProgramMethodSet prunedMethods) {
+ if (prunedMethods.contains(constraint.getMethod())) {
// The current parameter node is an argument to a method that has been removed by argument
// propagation. Therefore, this usage constraint is no longer blocking the removal of the
// parameter.
