"Reland "Allow inlining when the optimize flag is set""""
This reverts commit b4d07e706dffa41f2bb7691697b7a7ff296d6a3d.
Pass don't optimize to jctf compilation to avoid doing inlining of simple methods
Bug: 198113932
Bug: 199142666
Change-Id: I5f036766fcda11b0d945194edbdb46ad4c5a2f66
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CallGraphBuilderBase.java b/src/main/java/com/android/tools/r8/ir/conversion/CallGraphBuilderBase.java
index 1e5a344..2be47ca 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/CallGraphBuilderBase.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/CallGraphBuilderBase.java
@@ -144,10 +144,11 @@
// We don't care about calls to native methods.
return;
}
- if (appView.appInfo().isPinned(callee.getReference())) {
- // Since the callee is kept, we cannot inline it into the caller, and we also cannot collect
- // any optimization info for the method. Therefore, we drop the call edge to reduce the
- // total number of call graph edges, which should lead to fewer call graph cycles.
+ if (!appView.getKeepInfo(callee).isInliningAllowed(appView.options())) {
+ // Since the callee is kept and optimizations are disallowed, we cannot inline it into the
+ // caller, and we also cannot collect any optimization info for the method. Therefore, we
+ // drop the call edge to reduce the total number of call graph edges, which should lead to
+ // fewer call graph cycles.
return;
}
getOrCreateNode(callee).addCallerConcurrently(currentMethod, likelySpuriousCallEdge);
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 0235660..5bde78e 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
@@ -1288,6 +1288,7 @@
.libraryMethodOptimizer()
.optimize(code, feedback, methodProcessor, methodProcessingContext);
timing.end();
+ previous = printMethod(code, "IR after class library method optimizer (SSA)", previous);
assert code.isConsistentSSA();
}
@@ -1298,6 +1299,7 @@
timing.begin("Devirtualize invoke interface");
devirtualizer.devirtualizeInvokeInterface(code);
timing.end();
+ previous = printMethod(code, "IR after devirtualizer (SSA)", previous);
}
assert code.verifyTypes(appView);
@@ -1374,12 +1376,14 @@
timing.begin("Rewrite throw NPE");
codeRewriter.rewriteThrowNullPointerException(code);
timing.end();
+ previous = printMethod(code, "IR after rewrite throw null (SSA)", previous);
}
timing.begin("Optimize class initializers");
ClassInitializerDefaultsResult classInitializerDefaultsResult =
classInitializerDefaultsOptimization.optimize(code, feedback);
timing.end();
+ previous = printMethod(code, "IR after class initializer optimisation (SSA)", previous);
if (Log.ENABLED) {
Log.debug(getClass(), "Intermediate (SSA) flow graph for %s:\n%s",
@@ -1391,7 +1395,7 @@
deadCodeRemover.run(code, timing);
assert code.isConsistentSSA();
- previous = printMethod(code, "IR after lambda desugaring (SSA)", previous);
+ previous = printMethod(code, "IR after dead code removal (SSA)", previous);
assert code.verifyTypes(appView);
@@ -1572,7 +1576,7 @@
timing.end();
}
- if (appView.getKeepInfo().getMethodInfo(code.context()).isPinned(options)) {
+ if (appView.getKeepInfo(code.context()).isPinned(options)) {
return;
}
@@ -1694,8 +1698,7 @@
|| definition.getOptimizationInfo().isReachabilitySensitive()) {
return false;
}
- if (appView.appInfo().hasLiveness()
- && appView.appInfo().withLiveness().isPinned(method.getReference())) {
+ if (!appView.getKeepInfo(method).isInliningAllowed(options)) {
return false;
}
return true;
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
index 0ddc915..7bf0e38 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
@@ -118,7 +118,7 @@
WhyAreYouNotInliningReporter whyAreYouNotInliningReporter) {
AppInfoWithLiveness appInfo = appView.appInfo();
DexMethod singleTargetReference = singleTarget.getReference();
- if (appInfo.isPinned(singleTargetReference)) {
+ if (!appView.getKeepInfo(singleTarget).isInliningAllowed(appView.options())) {
whyAreYouNotInliningReporter.reportPinned();
return true;
}
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 a46bf38..242a08c 100644
--- a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
+++ b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
@@ -937,7 +937,8 @@
if (!options().enableValuePropagation || neverPropagateValue.contains(method)) {
return false;
}
- if (isPinned(method) && !method.getReturnType().isAlwaysNull(this)) {
+ if (!method.getReturnType().isAlwaysNull(this)
+ && !getKeepInfo().getMethodInfo(method, this).isInliningAllowed(options())) {
return false;
}
return true;
@@ -1319,7 +1320,11 @@
if (refinedReceiverClass.isProgramClass()) {
DexClassAndMethod clazzAndMethod =
resolution.lookupVirtualDispatchTarget(refinedReceiverClass.asProgramClass(), this);
- if (clazzAndMethod == null || isPinned(clazzAndMethod.getDefinition().getReference())) {
+ if (clazzAndMethod == null
+ || (clazzAndMethod.isProgramMethod()
+ && !getKeepInfo()
+ .getMethodInfo(clazzAndMethod.asProgramMethod())
+ .isOptimizationAllowed(options()))) {
// TODO(b/150640456): We should maybe only consider program methods.
return DexEncodedMethod.SENTINEL;
}
diff --git a/src/main/java/com/android/tools/r8/shaking/KeepMethodInfo.java b/src/main/java/com/android/tools/r8/shaking/KeepMethodInfo.java
index dd4226c..79fac0a 100644
--- a/src/main/java/com/android/tools/r8/shaking/KeepMethodInfo.java
+++ b/src/main/java/com/android/tools/r8/shaking/KeepMethodInfo.java
@@ -54,6 +54,10 @@
return this.equals(bottom());
}
+ public boolean isInliningAllowed(GlobalKeepInfoConfiguration configuration) {
+ return isOptimizationAllowed(configuration);
+ }
+
public static class Builder extends KeepInfo.Builder<Builder, KeepMethodInfo> {
private Builder() {
diff --git a/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java b/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
index 2af9e9a..c81cdfc 100644
--- a/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
+++ b/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
@@ -1901,6 +1901,7 @@
.setDisableTreeShaking(true)
.setDisableMinification(true)
.setDisableDesugaring(compilationOptions.disableDesugaring)
+ .addProguardConfiguration(ImmutableList.of("-dontoptimize"), Origin.unknown())
.addProguardConfiguration(ImmutableList.of("-keepattributes *"), Origin.unknown())
.addProguardConfiguration(compilationOptions.keepRules, Origin.unknown())
.setOutput(
diff --git a/src/test/java/com/android/tools/r8/desugar/lambdas/LambdaInStacktraceTest.java b/src/test/java/com/android/tools/r8/desugar/lambdas/LambdaInStacktraceTest.java
index 640796d..31f87d9 100644
--- a/src/test/java/com/android/tools/r8/desugar/lambdas/LambdaInStacktraceTest.java
+++ b/src/test/java/com/android/tools/r8/desugar/lambdas/LambdaInStacktraceTest.java
@@ -102,6 +102,7 @@
.addKeepAttributeSourceFile()
.addKeepRules("-renamesourcefileattribute SourceFile")
.noTreeShaking()
+ .addDontOptimize()
.run(parameters.getRuntime(), TestRunner.class, Boolean.toString(isDalvik))
.assertSuccess()
.getStdOut();
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/inliner/InliningOptimize.java b/src/test/java/com/android/tools/r8/ir/optimize/inliner/InliningOptimize.java
new file mode 100644
index 0000000..0c0bac6
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/inliner/InliningOptimize.java
@@ -0,0 +1,47 @@
+// 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.inliner;
+
+import static org.junit.Assert.assertFalse;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.utils.codeinspector.InstructionSubject;
+import org.junit.Test;
+
+class Foobar {
+ public static void main(String[] args) {
+ System.out.println("Value: " + (new Bar()).returnPlusConstant(42));
+ }
+}
+
+class Bar {
+ public int returnPlusConstant(int value) {
+ return value + 42;
+ }
+}
+
+public class InliningOptimize extends TestBase {
+
+ @Test
+ public void test() throws Exception {
+ testForR8(Backend.DEX)
+ .addProgramClasses(Bar.class, Foobar.class)
+ .addKeepRules("-keep,allowoptimization class ** {\n" + "*;\n" + "}")
+ .compile()
+ .inspect(
+ inspector -> {
+ inspector
+ .clazz(Foobar.class)
+ .mainMethod()
+ .iterateInstructions(InstructionSubject::isInvoke)
+ .forEachRemaining(
+ invoke -> {
+ assertFalse(
+ invoke.getMethod().name.toString().contains("returnPlusConstant"));
+ });
+ })
+ .run(Foobar.class)
+ .assertSuccessWithOutputLines("Value: 84");
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/maindexlist/MainDexTracingTest.java b/src/test/java/com/android/tools/r8/maindexlist/MainDexTracingTest.java
index bcea2fe..42cd647 100644
--- a/src/test/java/com/android/tools/r8/maindexlist/MainDexTracingTest.java
+++ b/src/test/java/com/android/tools/r8/maindexlist/MainDexTracingTest.java
@@ -391,6 +391,7 @@
.setMinApi(minSdk)
.noMinification()
.noTreeShaking()
+ .addDontOptimize()
.setMainDexListConsumer(ToolHelper.consumeString(r8MainDexListOutput::set))
.allowDiagnosticMessages()
.compileWithExpectedDiagnostics(
diff --git a/src/test/java/com/android/tools/r8/retrace/RetraceLambdaTest.java b/src/test/java/com/android/tools/r8/retrace/RetraceLambdaTest.java
index 252dcc5..d68447d 100644
--- a/src/test/java/com/android/tools/r8/retrace/RetraceLambdaTest.java
+++ b/src/test/java/com/android/tools/r8/retrace/RetraceLambdaTest.java
@@ -105,6 +105,7 @@
.addKeepMainRule(Main.class)
.addKeepPackageNamesRule(getClass().getPackage())
.noTreeShaking()
+ .addDontOptimize()
.addKeepAttributeSourceFile()
.addKeepAttributeLineNumberTable()
.setMinApi(parameters.getApiLevel())
diff --git a/src/test/java/com/android/tools/r8/smali/OutlineTest.java b/src/test/java/com/android/tools/r8/smali/OutlineTest.java
index 21501d0..c9d30fd 100644
--- a/src/test/java/com/android/tools/r8/smali/OutlineTest.java
+++ b/src/test/java/com/android/tools/r8/smali/OutlineTest.java
@@ -85,6 +85,8 @@
return options -> {
// Disable inlining to make sure that code looks as expected.
options.enableInlining = false;
+ // Disable the devirtulizer to not remove the super calls
+ options.enableDevirtualization = false;
// Disable string concatenation optimization to not bother outlining of StringBuilder usage.
options.enableStringConcatenationOptimization = false;
// Also apply outline options.
