Do not compute optimization info summary for kept methods
Bug: 131619590
Change-Id: Ia8ba0b16c0801a05db6d4a9bac080c9bc63d7413
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 7395d2b..2d8a5de 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
@@ -1115,22 +1115,6 @@
previous =
printMethod(code, "IR after idempotent function call canonicalization (SSA)", previous);
- codeRewriter.identifyReturnsArgument(method, code, feedback);
- if (options.enableInlining && inliner != null) {
- codeRewriter.identifyInvokeSemanticsForInlining(method, code, appView, feedback);
- }
-
- if (appView.enableWholeProgramOptimizations()) {
- // Track usage of parameters and compute their nullability and possibility of NPE.
- if (method.getOptimizationInfo().getNonNullParamOrThrow() == null) {
- computeNonNullParamHints(feedback, method, code);
- }
-
- computeDynamicReturnType(feedback, method, code);
- computeInitializedClassesOnNormalExit(feedback, method, code);
- computeMayHaveSideEffects(feedback, method, code);
- }
-
// Insert code to log arguments if requested.
if (options.methodMatchesLogArgumentsFilter(method)) {
codeRewriter.logArgumentTypes(method, code);
@@ -1139,19 +1123,32 @@
previous = printMethod(code, "IR after argument type logging (SSA)", previous);
- // Analysis must be done after method is rewritten by logArgumentTypes()
- codeRewriter.identifyClassInlinerEligibility(method, code, feedback);
-
- previous = printMethod(code, "IR after class inliner eligibility (SSA)", previous);
-
- if (method.isInstanceInitializer() || method.isClassInitializer()) {
- codeRewriter.identifyTrivialInitializer(method, code, feedback);
- }
- codeRewriter.identifyParameterUsages(method, code, feedback);
if (classStaticizer != null) {
classStaticizer.examineMethodCode(method, code);
}
+ // Compute optimization info summary for the current method unless it is pinned (in that case,
+ // we should not be making any assumptions about the behavior of the method).
+ if (appView.enableWholeProgramOptimizations()
+ && !appView.appInfo().withLiveness().isPinned(method.method)) {
+ codeRewriter.identifyClassInlinerEligibility(method, code, feedback);
+ codeRewriter.identifyParameterUsages(method, code, feedback);
+ codeRewriter.identifyReturnsArgument(method, code, feedback);
+ codeRewriter.identifyTrivialInitializer(method, code, feedback);
+
+ if (options.enableInlining && inliner != null) {
+ codeRewriter.identifyInvokeSemanticsForInlining(method, code, appView, feedback);
+ }
+
+ computeDynamicReturnType(feedback, method, code);
+ computeInitializedClassesOnNormalExit(feedback, method, code);
+ computeMayHaveSideEffects(feedback, method, code);
+ computeNonNullParamOrThrow(feedback, method, code);
+ }
+
+ previous =
+ printMethod(code, "IR after computation of optimization info summary (SSA)", previous);
+
if (options.canHaveNumberConversionRegisterAllocationBug()) {
codeRewriter.workaroundNumberConversionRegisterAllocationBug(code);
}
@@ -1171,8 +1168,13 @@
finalizeIR(method, code, feedback);
}
- private void computeNonNullParamHints(
- OptimizationFeedback feedback, DexEncodedMethod method, IRCode code) {
+ // Track usage of parameters and compute their nullability and possibility of NPE.
+ private void computeNonNullParamOrThrow(
+ OptimizationFeedback feedback, DexEncodedMethod method, IRCode code) {
+ if (method.getOptimizationInfo().getNonNullParamOrThrow() != null) {
+ return;
+ }
+
List<Value> arguments = code.collectArguments();
BitSet paramsCheckedForNull = new BitSet();
for (int index = 0; index < arguments.size(); index++) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
index 87c28bf..f214373 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
@@ -11,7 +11,6 @@
import com.android.tools.r8.dex.Constants;
import com.android.tools.r8.errors.CompilationError;
import com.android.tools.r8.errors.Unreachable;
-import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DebugLocalInfo;
import com.android.tools.r8.graph.DexClass;
@@ -1169,22 +1168,23 @@
public void identifyTrivialInitializer(
DexEncodedMethod method, IRCode code, OptimizationFeedback feedback) {
- boolean isInstanceInitializer = method.isInstanceInitializer();
- boolean isClassInitializer = method.isClassInitializer();
- assert isInstanceInitializer || isClassInitializer;
+ if (!method.isInstanceInitializer() && !method.isClassInitializer()) {
+ return;
+ }
+
if (method.accessFlags.isNative()) {
return;
}
- AppInfo appInfo = appView.appInfo();
- DexClass clazz = appInfo.definitionFor(method.method.holder);
+ DexClass clazz = appView.appInfo().definitionFor(method.method.holder);
if (clazz == null) {
return;
}
- feedback.setTrivialInitializer(method,
- isInstanceInitializer
- ? computeInstanceInitializerInfo(code, clazz, appInfo::definitionFor)
+ feedback.setTrivialInitializer(
+ method,
+ method.isInstanceInitializer()
+ ? computeInstanceInitializerInfo(code, clazz, appView::definitionFor)
: computeClassInitializerInfo(code, clazz));
}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/OptimizationSummaryForKeptMethodTest.java b/src/test/java/com/android/tools/r8/ir/optimize/OptimizationSummaryForKeptMethodTest.java
new file mode 100644
index 0000000..7dc327c
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/OptimizationSummaryForKeptMethodTest.java
@@ -0,0 +1,94 @@
+// Copyright (c) 2019, 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;
+
+import static com.android.tools.r8.utils.codeinspector.CodeMatchers.invokesMethod;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertNotEquals;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.utils.BooleanUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.InstructionSubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+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 OptimizationSummaryForKeptMethodTest extends TestBase {
+
+ private final boolean enableExtraKeepRule;
+ private final TestParameters parameters;
+
+ @Parameters(name = "{1}, enable extra keep rule: {0}")
+ public static List<Object[]> params() {
+ return buildParameters(BooleanUtils.values(), getTestParameters().withAllRuntimes().build());
+ }
+
+ public OptimizationSummaryForKeptMethodTest(
+ boolean enableExtraKeepRule, TestParameters parameters) {
+ this.enableExtraKeepRule = enableExtraKeepRule;
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(OptimizationSummaryForKeptMethodTest.class)
+ .addKeepMainRule(TestClass.class)
+ .addKeepClassAndMembersRules(
+ (Class<?>[]) (enableExtraKeepRule ? new Class<?>[] {KeptClass.class} : new Class<?>[0]))
+ .enableInliningAnnotations()
+ .setMinApi(parameters.getRuntime())
+ .compile()
+ .inspect(this::verifyOutput);
+ }
+
+ private void verifyOutput(CodeInspector inspector) {
+ ClassSubject testClassSubject = inspector.clazz(TestClass.class);
+ assertThat(testClassSubject, isPresent());
+
+ MethodSubject mainMethodSubject = testClassSubject.mainMethod();
+ assertThat(mainMethodSubject, isPresent());
+
+ // Main method invokes noNormalExits() since it is not allowed to be inlined.
+ assertThat(
+ mainMethodSubject,
+ invokesMethod(inspector.clazz(KeptClass.class).uniqueMethodWithName("noNormalExits")));
+
+ // The fact that noNormalExits() never returns normally has only been exploited if it is not
+ // kept.
+ assertNotEquals(
+ enableExtraKeepRule,
+ mainMethodSubject.streamInstructions().anyMatch(InstructionSubject::isThrow));
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ try {
+ KeptClass.noNormalExits();
+ System.out.println(" world!");
+ } catch (Exception e) {
+ System.out.println("Caught " + e.getClass().getName());
+ }
+ }
+ }
+
+ static class KeptClass {
+
+ @NeverInline
+ public static void noNormalExits() {
+ throw new RuntimeException();
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/neverreturnsnormally/NeverReturnsNormallyTest.java b/src/test/java/com/android/tools/r8/neverreturnsnormally/NeverReturnsNormallyTest.java
index d6ca6e1..9df1d87 100644
--- a/src/test/java/com/android/tools/r8/neverreturnsnormally/NeverReturnsNormallyTest.java
+++ b/src/test/java/com/android/tools/r8/neverreturnsnormally/NeverReturnsNormallyTest.java
@@ -12,20 +12,24 @@
import com.android.tools.r8.ForceInline;
import com.android.tools.r8.R8Command;
import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.cf.code.CfStackInstruction.Opcode;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.AndroidApp;
+import com.android.tools.r8.utils.BooleanUtils;
import com.android.tools.r8.utils.codeinspector.CfInstructionSubject;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.android.tools.r8.utils.codeinspector.DexInstructionSubject;
+import com.android.tools.r8.utils.codeinspector.FoundMethodSubject;
import com.android.tools.r8.utils.codeinspector.InstructionSubject;
import com.android.tools.r8.utils.codeinspector.InstructionSubject.JumboStringMode;
import com.android.tools.r8.utils.codeinspector.InvokeInstructionSubject;
import com.android.tools.r8.utils.codeinspector.MethodSubject;
import com.google.common.collect.ImmutableList;
import java.util.Iterator;
+import java.util.List;
import java.util.function.BiConsumer;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -35,15 +39,17 @@
@RunWith(Parameterized.class)
public class NeverReturnsNormallyTest extends TestBase {
- private Backend backend;
+ private boolean keepOuterTrivial;
+ private TestParameters parameters;
- @Parameters(name = "Backend: {0}")
- public static Backend[] data() {
- return ToolHelper.getBackends();
+ @Parameters(name = "{1}, keep outerTrivial: {0}")
+ public static List<Object[]> data() {
+ return buildParameters(BooleanUtils.values(), getTestParameters().withAllRuntimes().build());
}
- public NeverReturnsNormallyTest(Backend backend) {
- this.backend = backend;
+ public NeverReturnsNormallyTest(boolean keepOuterTrivial, TestParameters parameters) {
+ this.keepOuterTrivial = keepOuterTrivial;
+ this.parameters = parameters;
}
private void runTest(
@@ -52,53 +58,65 @@
R8Command.Builder builder = R8Command.builder();
builder.addProgramFiles(ToolHelper.getClassFileForTestClass(ForceInline.class));
builder.addProgramFiles(ToolHelper.getClassFileForTestClass(TestClass.class));
- builder.setProgramConsumer(emptyConsumer(backend));
- builder.addLibraryFiles(runtimeJar(backend));
+ builder.setProgramConsumer(emptyConsumer(parameters.getBackend()));
+ builder.addLibraryFiles(runtimeJar(parameters.getBackend()));
builder.setMode(mode);
builder.addProguardConfiguration(
ImmutableList.of(
"-forceinline class * { @com.android.tools.r8.ForceInline *; }",
- "-keep class " + TestClass.class.getCanonicalName() + "{",
+ "-keep class " + TestClass.class.getTypeName() + " {",
" public static void main(java.lang.String[]);",
" *** test*(...);",
- " *** outerTrivial(...);",
- "}",
- "",
- "-checkdiscard class " + TestClass.class.getCanonicalName() + "{",
- " *** assertRemoved(...);",
"}",
"",
"-dontobfuscate",
"-allowaccessmodification"),
Origin.unknown());
+ if (keepOuterTrivial) {
+ builder.addProguardConfiguration(
+ ImmutableList.of(
+ "-keep class " + TestClass.class.getTypeName() + " {",
+ " *** outerTrivial(...);",
+ "}"),
+ Origin.unknown());
+ } else {
+ builder.addProguardConfiguration(
+ ImmutableList.of(
+ "-checkdiscard class " + TestClass.class.getTypeName() + " {",
+ " *** assertRemoved(...);",
+ "}"),
+ Origin.unknown());
+ }
ToolHelper.allowTestProguardOptions(builder);
AndroidApp app =
ToolHelper.runR8(builder.build(), opts -> opts.enableClassInlining = enableClassInliner);
inspection.accept(new CodeInspector(app), mode);
- if (backend == Backend.DEX) {
+ if (parameters.isDexRuntime()) {
// Run on Art to check generated code against verifier.
runOnArt(app, TestClass.class);
} else {
- assert backend == Backend.CF;
+ assert parameters.isCfRuntime();
runOnJava(app, TestClass.class);
}
}
private void validate(CodeInspector inspector, CompilationMode mode) {
- assert (backend == Backend.DEX || backend == Backend.CF);
+ assert parameters.isCfRuntime() || parameters.isDexRuntime();
ClassSubject clazz = inspector.clazz(TestClass.class);
assertTrue(clazz.isPresent());
// All calls to 'assertRemoved' are to be removed.
- clazz.forAllMethods(method -> {
- Iterator<InstructionSubject> instructions =
- method.iterateInstructions(InstructionSubject::isInvoke);
- while (instructions.hasNext()) {
- InvokeInstructionSubject invoke = (InvokeInstructionSubject) instructions.next();
- assertFalse(invoke.invokedMethod().name.toString().equals("assertRemoved"));
- }
- });
+ int numberOfCallsToAssertRemoved = 0;
+ for (FoundMethodSubject method : clazz.allMethods()) {
+ numberOfCallsToAssertRemoved +=
+ method
+ .streamInstructions()
+ .filter(InstructionSubject::isInvoke)
+ .filter(invoke -> invoke.getMethod().name.toString().equals("assertRemoved"))
+ .count();
+ }
+ assertEquals(keepOuterTrivial ? 2 : 0, numberOfCallsToAssertRemoved);
// Check the instruction used for testInlinedIntoVoidMethod
MethodSubject methodThrowToBeInlined =
@@ -140,16 +158,17 @@
verifyTrailingPattern(instructions);
// Check the instruction used for outerTrivial
- MethodSubject methodOuterTrivial =
- clazz.method("int", "outerTrivial", ImmutableList.of());
- assertTrue(methodOuterTrivial.isPresent());
- instructions = methodOuterTrivial.iterateInstructions();
- // Call, followed by [nop, goto]
- insn = nextInstructionSkippingCfPositionAndLabel(instructions);
- assertTrue(insn.isInvoke());
- assertTrue(((InvokeInstructionSubject) insn)
- .invokedMethod().name.toString().equals("innerNotReachable"));
- verifyTrailingPattern(instructions);
+ MethodSubject methodOuterTrivial = clazz.method("int", "outerTrivial", ImmutableList.of());
+ assertEquals(keepOuterTrivial || mode == CompilationMode.DEBUG, methodOuterTrivial.isPresent());
+
+ if (methodOuterTrivial.isPresent()) {
+ instructions = methodOuterTrivial.iterateInstructions();
+ // Call, followed by [nop, goto]
+ insn = nextInstructionSkippingCfPositionAndLabel(instructions);
+ assertTrue(insn.isInvoke());
+ assertEquals("innerNotReachable", insn.getMethod().name.toString());
+ verifyTrailingPattern(instructions);
+ }
}
private InstructionSubject nextInstruction(Iterator<InstructionSubject> instructions) {
@@ -175,7 +194,7 @@
private void verifyTrailingPattern(Iterator<InstructionSubject> instructions) {
InstructionSubject insn = nextInstruction(instructions);
- if (backend == Backend.DEX) {
+ if (parameters.isDexRuntime()) {
assertTrue(
insn instanceof DexInstructionSubject && ((DexInstructionSubject) insn).isConst4());
} else {
