Defer redundant block removal to end of class inliner
Change-Id: If1e6fd3f1a8041f629312ec800548df31e465b15
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index 8b5d9ee..cf010f5 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -729,7 +729,7 @@
// Class inliner should work before lambda merger, so if it inlines the
// lambda, it does not get collected by merger.
classInliner.processMethodCode(
- context, code, feedback, methodProcessor, methodProcessingContext);
+ context, code, feedback, methodProcessor, methodProcessingContext, timing);
timing.end();
code.removeRedundantBlocks();
assert code.isConsistentSSA(appView);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
index 76250ff..57efcdf 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
@@ -134,7 +134,8 @@
IRCode code,
OptimizationFeedback feedback,
MethodProcessor methodProcessor,
- MethodProcessingContext methodProcessingContext) {
+ MethodProcessingContext methodProcessingContext,
+ Timing timing) {
LazyBox<InliningOracle> defaultOracle =
new LazyBox<>(
() ->
@@ -146,14 +147,17 @@
code));
// Collect all the new-instance and static-get instructions in the code before inlining.
+ timing.begin("Compute roots");
List<Instruction> roots =
Lists.newArrayList(code.instructions(insn -> insn.isNewInstance() || insn.isStaticGet()));
+ timing.end();
// We loop inlining iterations until there was no inlining, but still use same set
// of roots to avoid infinite inlining. Looping makes possible for some roots to
// become eligible after other roots are inlined.
boolean anyInlinedGeneratedMessageLiteBuilders = false;
boolean anyInlinedMethods = false;
+ boolean changed = false;
boolean repeat;
do {
repeat = false;
@@ -165,40 +169,46 @@
new InlineCandidateProcessor(appView, code, inliner, methodProcessor, method, root);
// Assess eligibility of instance and class.
- EligibilityStatus status = processor.isInstanceEligible();
- if (status != EligibilityStatus.ELIGIBLE) {
- // This root will never be inlined.
- rootsIterator.remove();
- continue;
- }
- status = isClassEligible(processor.getEligibleClass());
- if (status != EligibilityStatus.ELIGIBLE) {
- // This root will never be inlined.
- rootsIterator.remove();
- continue;
+ try (Timing t0 = timing.begin("Compute instance eligibility")) {
+ EligibilityStatus status = processor.isInstanceEligible();
+ if (status != EligibilityStatus.ELIGIBLE) {
+ // This root will never be inlined.
+ rootsIterator.remove();
+ continue;
+ }
+ status = isClassEligible(processor.getEligibleClass());
+ if (status != EligibilityStatus.ELIGIBLE) {
+ // This root will never be inlined.
+ rootsIterator.remove();
+ continue;
+ }
}
// Assess users eligibility and compute inlining of direct calls and extra methods needed.
- InstructionOrPhi ineligibleUser = processor.areInstanceUsersEligible(defaultOracle);
- if (ineligibleUser != null) {
- // This root may succeed if users change in future.
- continue;
+ try (Timing t0 = timing.begin("Compute users eligibility")) {
+ InstructionOrPhi ineligibleUser = processor.areInstanceUsersEligible(defaultOracle);
+ if (ineligibleUser != null) {
+ // This root may succeed if users change in future.
+ continue;
+ }
}
// Is inlining allowed.
InliningIRProvider inliningIRProvider =
new InliningIRProvider(
appView, method, code, inliner.getLensCodeRewriter(), methodProcessor);
- ClassInlinerCostAnalysis costAnalysis =
- new ClassInlinerCostAnalysis(appView, inliningIRProvider, processor.getReceivers());
- if (costAnalysis.willExceedInstructionBudget(
- code,
- processor.getEligibleClass(),
- processor.getDirectInlinees(),
- processor.getIndirectInlinees())) {
- // This root is unlikely to be inlined in the future.
- rootsIterator.remove();
- continue;
+ try (Timing t0 = timing.begin("Analyze cost")) {
+ ClassInlinerCostAnalysis costAnalysis =
+ new ClassInlinerCostAnalysis(appView, inliningIRProvider, processor.getReceivers());
+ if (costAnalysis.willExceedInstructionBudget(
+ code,
+ processor.getEligibleClass(),
+ processor.getDirectInlinees(),
+ processor.getIndirectInlinees())) {
+ // This root is unlikely to be inlined in the future.
+ rootsIterator.remove();
+ continue;
+ }
}
if (appView.protoShrinker() != null && root.isNewInstance()) {
@@ -212,28 +222,33 @@
// Inline the class instance.
AffectedValues affectedValues = new AffectedValues();
- try {
- anyInlinedMethods |=
- processor.processInlining(code, affectedValues, feedback, inliningIRProvider);
- } catch (IllegalClassInlinerStateException e) {
- // We introduced a user that we cannot handle in the class inliner as a result of force
- // inlining. Abort gracefully from class inlining without removing the instance.
- //
- // Alternatively we would need to collect additional information about the behavior of
- // methods (which is bad for memory), or we would need to analyze the called methods
- // before inlining them. The latter could be good solution, since we are going to build IR
- // for the methods that need to be inlined anyway.
- assert false;
- anyInlinedMethods = true;
+ try (Timing t0 = timing.begin("Perform inlining")) {
+ try {
+ anyInlinedMethods |=
+ processor.processInlining(
+ code, affectedValues, feedback, inliningIRProvider, timing);
+ } catch (IllegalClassInlinerStateException e) {
+ // We introduced a user that we cannot handle in the class inliner as a result of force
+ // inlining. Abort gracefully from class inlining without removing the instance.
+ //
+ // Alternatively we would need to collect additional information about the behavior of
+ // methods (which is bad for memory), or we would need to analyze the called methods
+ // before inlining them. The latter could be good solution, since we are going to build
+ // IR for the methods that need to be inlined anyway.
+ assert false;
+ anyInlinedMethods = true;
+ }
}
+ changed = true;
// Restore normality.
- code.removeAllDeadAndTrivialPhis(affectedValues);
- affectedValues.narrowingWithAssumeRemoval(appView, code);
- code.removeRedundantBlocks();
- assert code.isConsistentSSA(appView);
- rootsIterator.remove();
- repeat = true;
+ try (Timing t0 = timing.begin("Restore consistent SSA")) {
+ code.removeAllDeadAndTrivialPhis(affectedValues);
+ affectedValues.narrowingWithAssumeRemoval(appView, code);
+ assert code.isConsistentSSAAllowingRedundantBlocks(appView);
+ rootsIterator.remove();
+ repeat = true;
+ }
}
} while (repeat);
@@ -245,6 +260,11 @@
method, code, feedback, methodProcessor, methodProcessingContext, inliner));
}
+ if (changed) {
+ // Ensure no redundant blocks.
+ code.removeRedundantBlocks();
+ }
+
if (anyInlinedMethods) {
// If a method was inlined we may be able to remove check-cast instructions because we may
// have more information about the types of the arguments at the call site. This is
@@ -259,6 +279,8 @@
.libraryMethodOptimizer()
.optimize(code, feedback, methodProcessor, methodProcessingContext);
}
+
+ assert code.isConsistentSSA(appView);
}
private EligibilityStatus isClassEligible(DexProgramClass clazz) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
index 51881ef..5365575 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
@@ -375,43 +375,52 @@
IRCode code,
AffectedValues affectedValues,
OptimizationFeedback feedback,
- InliningIRProvider inliningIRProvider)
+ InliningIRProvider inliningIRProvider,
+ Timing timing)
throws IllegalClassInlinerStateException {
// Verify that `eligibleInstance` is not aliased.
assert eligibleInstance == eligibleInstance.getAliasedValue();
- boolean anyInlinedMethods =
- forceInlineDirectMethodInvocations(code, feedback, inliningIRProvider);
- anyInlinedMethods |= forceInlineIndirectMethodInvocations(code, feedback, inliningIRProvider);
+ boolean anyInlinedMethods;
+ try (Timing t0 = timing.begin("Force inline direct methods")) {
+ anyInlinedMethods =
+ forceInlineDirectMethodInvocations(code, feedback, inliningIRProvider, timing);
+ }
+ try (Timing t0 = timing.begin("Force inline indirect methods")) {
+ anyInlinedMethods |=
+ forceInlineIndirectMethodInvocations(code, feedback, inliningIRProvider, timing);
+ }
- rebindIndirectEligibleInstanceUsersFromPhis();
- removeMiscUsages(code, affectedValues);
- removeFieldReads(code, affectedValues);
- removeFieldWrites();
- removeInstruction(root);
+ try (Timing t0 = timing.begin("Process users")) {
+ rebindIndirectEligibleInstanceUsersFromPhis();
+ removeMiscUsages(code, affectedValues);
+ removeFieldReads(code, affectedValues);
+ removeFieldWrites();
+ removeInstruction(root);
+ }
return anyInlinedMethods;
}
@SuppressWarnings("ReferenceEquality")
private boolean forceInlineDirectMethodInvocations(
- IRCode code, OptimizationFeedback feedback, InliningIRProvider inliningIRProvider)
+ IRCode code,
+ OptimizationFeedback feedback,
+ InliningIRProvider inliningIRProvider,
+ Timing timing)
throws IllegalClassInlinerStateException {
if (directMethodCalls.isEmpty()) {
return false;
}
+ timing.begin("Inline initial");
inliner.performForcedInlining(
- method,
- code,
- directMethodCalls,
- feedback,
- inliningIRProvider,
- methodProcessor,
- Timing.empty());
+ method, code, directMethodCalls, feedback, inliningIRProvider, methodProcessor, timing);
+ timing.end();
// In case we are class inlining an object allocation that does not inherit directly from
// java.lang.Object, we need keep force inlining the constructor until we reach
// java.lang.Object.<init>().
+ timing.begin("Inline constructors");
if (root.isNewInstance()) {
do {
directMethodCalls.clear();
@@ -460,17 +469,21 @@
feedback,
inliningIRProvider,
methodProcessor,
- Timing.empty());
+ timing);
}
} while (!directMethodCalls.isEmpty());
}
+ timing.end();
return true;
}
@SuppressWarnings("ReferenceEquality")
private boolean forceInlineIndirectMethodInvocations(
- IRCode code, OptimizationFeedback feedback, InliningIRProvider inliningIRProvider)
+ IRCode code,
+ OptimizationFeedback feedback,
+ InliningIRProvider inliningIRProvider,
+ Timing timing)
throws IllegalClassInlinerStateException {
if (indirectMethodCallsOnInstance.isEmpty()) {
return false;
@@ -548,7 +561,7 @@
feedback,
inliningIRProvider,
methodProcessor,
- Timing.empty());
+ timing);
} else {
// TODO(b/315284776): Diagnose if this should be removed or reenabled.
/*assert indirectMethodCallsOnInstance.stream()
