Update class inliner to check if method is eligible for force inlining
Change-Id: I93fffb195153196f1f8961bc4f3cc6f3ebbd1055
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 769923d..38b0b5f 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
@@ -23,7 +23,6 @@
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.Nullability;
import com.android.tools.r8.ir.analysis.type.TypeAnalysis;
@@ -48,7 +47,6 @@
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.conversion.MethodProcessor;
import com.android.tools.r8.ir.optimize.Inliner;
-import com.android.tools.r8.ir.optimize.Inliner.InlineAction;
import com.android.tools.r8.ir.optimize.Inliner.InliningInfo;
import com.android.tools.r8.ir.optimize.Inliner.Reason;
import com.android.tools.r8.ir.optimize.InliningOracle;
@@ -1171,15 +1169,12 @@
// Check if the method is inline-able by standard inliner.
InliningOracle oracle = defaultOracle.get();
- InlineAction inlineAction =
- oracle.computeInlining(
- invoke,
- resolutionResult,
- singleTarget,
- method,
- ClassInitializationAnalysis.trivial(),
- NopWhyAreYouNotInliningReporter.getInstance());
- if (inlineAction == null) {
+ if (!oracle.passesInliningConstraints(
+ invoke,
+ resolutionResult,
+ singleTarget,
+ Reason.SIMPLE,
+ NopWhyAreYouNotInliningReporter.getInstance())) {
return false;
}
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/TreeFixerInterfaceCollisionTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/TreeFixerInterfaceCollisionTest.java
index 6f3c8f8..dbc83c8 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/TreeFixerInterfaceCollisionTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/TreeFixerInterfaceCollisionTest.java
@@ -99,6 +99,7 @@
}
}
+ @NeverClassInline
public static class C extends Parent implements I {
@Override
@NeverInline
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/TreeFixerInterfaceFixedCollisionTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/TreeFixerInterfaceFixedCollisionTest.java
index 627838f..169a59b 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/TreeFixerInterfaceFixedCollisionTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/TreeFixerInterfaceFixedCollisionTest.java
@@ -99,6 +99,7 @@
}
}
+ @NeverClassInline
public static class C extends Parent implements I {
@Override
@NeverInline
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/dispatch/OverrideAbstractMethodWithDefaultTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/dispatch/OverrideAbstractMethodWithDefaultTest.java
index 635ec43..11164b1 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/dispatch/OverrideAbstractMethodWithDefaultTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/dispatch/OverrideAbstractMethodWithDefaultTest.java
@@ -7,6 +7,7 @@
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.MatcherAssert.assertThat;
+import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.NoVerticalClassMerging;
import com.android.tools.r8.TestParameters;
@@ -15,6 +16,7 @@
import org.junit.Test;
public class OverrideAbstractMethodWithDefaultTest extends HorizontalClassMergingTestBase {
+
public OverrideAbstractMethodWithDefaultTest(
TestParameters parameters, boolean enableHorizontalClassMerging) {
super(parameters, enableHorizontalClassMerging);
@@ -29,6 +31,7 @@
options ->
options.horizontalClassMergerOptions().enableIf(enableHorizontalClassMerging))
.enableInliningAnnotations()
+ .enableNeverClassInliningAnnotations()
.enableNoVerticalClassMergingAnnotations()
.setMinApi(parameters.getApiLevel())
.addHorizontallyMergedClassesInspectorIf(
@@ -73,8 +76,10 @@
}
}
+ @NeverClassInline
static class C1 extends B1 implements J {}
+ @NeverClassInline
static class C2 extends B2 {}
static class Main {
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/dispatch/OverrideDefaultMethodTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/dispatch/OverrideDefaultMethodTest.java
index 0e3a098..15ca564 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/dispatch/OverrideDefaultMethodTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/dispatch/OverrideDefaultMethodTest.java
@@ -7,6 +7,7 @@
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.MatcherAssert.assertThat;
+import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.NoVerticalClassMerging;
import com.android.tools.r8.TestParameters;
@@ -29,6 +30,7 @@
options ->
options.horizontalClassMergerOptions().enableIf(enableHorizontalClassMerging))
.enableInliningAnnotations()
+ .enableNeverClassInliningAnnotations()
.enableNoVerticalClassMergingAnnotations()
.setMinApi(parameters.getApiLevel())
.addHorizontallyMergedClassesInspectorIf(
@@ -52,8 +54,10 @@
}
}
+ @NeverClassInline
public static class A implements I {}
+ @NeverClassInline
public static class B implements I {
@NeverInline
@Override
@@ -69,6 +73,7 @@
}
}
+ @NeverClassInline
public static class C extends A implements J {}
public static class Main {
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/dispatch/OverrideDefaultOnSuperMethodTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/dispatch/OverrideDefaultOnSuperMethodTest.java
index fa2dcb2..2aea3d0 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/dispatch/OverrideDefaultOnSuperMethodTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/dispatch/OverrideDefaultOnSuperMethodTest.java
@@ -7,6 +7,7 @@
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.MatcherAssert.assertThat;
+import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.NoUnusedInterfaceRemoval;
import com.android.tools.r8.NoVerticalClassMerging;
@@ -30,6 +31,7 @@
options ->
options.horizontalClassMergerOptions().enableIf(enableHorizontalClassMerging))
.enableInliningAnnotations()
+ .enableNeverClassInliningAnnotations()
.enableNoUnusedInterfaceRemovalAnnotations()
.enableNoVerticalClassMergingAnnotations()
.setMinApi(parameters.getApiLevel())
@@ -58,8 +60,10 @@
public static class Parent implements I {}
+ @NeverClassInline
public static class A extends Parent {}
+ @NeverClassInline
public static class B extends Parent {
@NeverInline
@Override
@@ -68,6 +72,7 @@
}
}
+ @NeverClassInline
@NoUnusedInterfaceRemoval
@NoVerticalClassMerging
interface J extends I {
@@ -76,6 +81,7 @@
}
}
+ @NeverClassInline
public static class C extends A implements J {}
public static class Main {
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/uninstantiatedtypes/InterfaceMethodTest.java b/src/test/java/com/android/tools/r8/ir/optimize/uninstantiatedtypes/InterfaceMethodTest.java
index f489112..209f9f9 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/uninstantiatedtypes/InterfaceMethodTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/uninstantiatedtypes/InterfaceMethodTest.java
@@ -7,6 +7,7 @@
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.MatcherAssert.assertThat;
+import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.NoHorizontalClassMerging;
import com.android.tools.r8.NoVerticalClassMerging;
@@ -48,6 +49,7 @@
.addInnerClasses(InterfaceMethodTest.class)
.addKeepMainRule(TestClass.class)
.enableInliningAnnotations()
+ .enableNeverClassInliningAnnotations()
.enableNoVerticalClassMergingAnnotations()
.enableNoHorizontalClassMergingAnnotations()
.run(TestClass.class)
@@ -84,6 +86,7 @@
Uninstantiated m();
}
+ @NeverClassInline
static class A implements I {
@NeverInline
@@ -97,6 +100,7 @@
// The purpose of this class is merely to avoid that the invoke-interface instruction in
// TestClass.test() gets devirtualized to an invoke-virtual instruction. Otherwise the method
// I.m() would not be present in the output.
+ @NeverClassInline
@NoHorizontalClassMerging
static class B implements I {
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/uninstantiatedtypes/NestedInterfaceMethodTest.java b/src/test/java/com/android/tools/r8/ir/optimize/uninstantiatedtypes/NestedInterfaceMethodTest.java
index 7890640..3cbaed0 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/uninstantiatedtypes/NestedInterfaceMethodTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/uninstantiatedtypes/NestedInterfaceMethodTest.java
@@ -7,6 +7,7 @@
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.MatcherAssert.assertThat;
+import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.NoVerticalClassMerging;
import com.android.tools.r8.TestBase;
@@ -49,6 +50,7 @@
.addInnerClasses(NestedInterfaceMethodTest.class)
.addKeepMainRule(TestClass.class)
.enableInliningAnnotations()
+ .enableNeverClassInliningAnnotations()
.enableNoVerticalClassMergingAnnotations()
.addOptionsModification(
options -> {
@@ -91,6 +93,7 @@
@NoVerticalClassMerging
interface J extends I {}
+ @NeverClassInline
static class A implements J {
@Override
@@ -100,11 +103,13 @@
}
}
+ @NeverClassInline
static class B extends A {}
// The purpose of this class is merely to avoid that the invoke-interface instruction in
// TestClass.test() gets devirtualized to an invoke-virtual instruction. Otherwise the method
// I.m() would not be present in the output.
+ @NeverClassInline
static class C extends A {}
static class Uninstantiated {}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/uninstantiatedtypes/SynchronizedMethodTest.java b/src/test/java/com/android/tools/r8/ir/optimize/uninstantiatedtypes/SynchronizedMethodTest.java
index 88459a5..a04df37 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/uninstantiatedtypes/SynchronizedMethodTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/uninstantiatedtypes/SynchronizedMethodTest.java
@@ -4,11 +4,17 @@
package com.android.tools.r8.ir.optimize.uninstantiatedtypes;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertSame;
+import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.utils.BooleanUtils;
import com.android.tools.r8.utils.StringUtils;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
@@ -18,23 +24,41 @@
import java.util.List;
import java.util.stream.Collectors;
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 SynchronizedMethodTest extends TestBase {
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public SynchronizedMethodTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
@Test
public void test() throws Exception {
String expectedOutput = StringUtils.lines("In A.m()", "Got NullPointerException");
CodeInspector inspector =
- testForR8(Backend.DEX)
+ testForR8(parameters.getBackend())
.addInnerClasses(SynchronizedMethodTest.class)
.addKeepMainRule(TestClass.class)
.enableInliningAnnotations()
- .run(TestClass.class)
+ .enableNeverClassInliningAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .run(parameters.getRuntime(), TestClass.class)
.assertSuccessWithOutput(expectedOutput)
.inspector();
ClassSubject clazz = inspector.clazz(A.class);
MethodSubject method = clazz.uniqueMethodWithName("m");
+ assertThat(method, isPresent());
// The invoke on the uninstantiated turns into a "throw null", and the synchronized method
// already have a throw instruction in the catch all handler ensuring monitor exit is called.
@@ -43,24 +67,30 @@
.streamInstructions()
.filter(InstructionSubject::isThrow)
.collect(Collectors.toList());
- assertEquals(2, throwInstructions.size());
+ assertEquals(1 + BooleanUtils.intValue(parameters.isDexRuntime()), throwInstructions.size());
- // The inserted "throw null" should still be covered by the catch all to ensure monitor exit
- // is called.
- List<InstructionSubject> catchAllCoveredInstructions = new ArrayList<>();
- method.iterateTryCatches().forEachRemaining(tryCatchSubject -> {
- if (tryCatchSubject.hasCatchAll()) {
- catchAllCoveredInstructions.addAll(
- throwInstructions
- .stream()
- .filter(
- throwInstruction ->
- tryCatchSubject.getRange().includes(throwInstruction.getOffset(method)))
- .collect(Collectors.toList()));
- }
- });
- assertEquals(1, catchAllCoveredInstructions.size());
- assertSame(throwInstructions.get(0), catchAllCoveredInstructions.get(0));
+ if (parameters.isDexRuntime()) {
+ // The inserted "throw null" should still be covered by the catch all to ensure monitor exit
+ // is called.
+ List<InstructionSubject> catchAllCoveredInstructions = new ArrayList<>();
+ method
+ .iterateTryCatches()
+ .forEachRemaining(
+ tryCatchSubject -> {
+ if (tryCatchSubject.hasCatchAll()) {
+ catchAllCoveredInstructions.addAll(
+ throwInstructions.stream()
+ .filter(
+ throwInstruction ->
+ tryCatchSubject
+ .getRange()
+ .includes(throwInstruction.getOffset(method)))
+ .collect(Collectors.toList()));
+ }
+ });
+ assertEquals(1, catchAllCoveredInstructions.size());
+ assertSame(throwInstructions.get(0), catchAllCoveredInstructions.get(0));
+ }
}
static class TestClass {
@@ -79,6 +109,7 @@
}
}
+ @NeverClassInline
static class A {
@NeverInline
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/unusedinterfaces/UnusedInterfaceWithDefaultMethodTest.java b/src/test/java/com/android/tools/r8/ir/optimize/unusedinterfaces/UnusedInterfaceWithDefaultMethodTest.java
index 52131b3..768fbd6 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/unusedinterfaces/UnusedInterfaceWithDefaultMethodTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/unusedinterfaces/UnusedInterfaceWithDefaultMethodTest.java
@@ -8,6 +8,7 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
+import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.NoVerticalClassMerging;
import com.android.tools.r8.TestBase;
@@ -40,6 +41,7 @@
.addInnerClasses(UnusedInterfaceWithDefaultMethodTest.class)
.addKeepMainRule(TestClass.class)
.enableInliningAnnotations()
+ .enableNeverClassInliningAnnotations()
.enableNoVerticalClassMergingAnnotations()
.setMinApi(parameters.getApiLevel())
.compile()
@@ -94,5 +96,6 @@
}
}
+ @NeverClassInline
static class A implements J {}
}
diff --git a/src/test/java/com/android/tools/r8/shaking/annotations/AnnotationsOnTargetedMethodTest.java b/src/test/java/com/android/tools/r8/shaking/annotations/AnnotationsOnTargetedMethodTest.java
index ace926f..9246164 100644
--- a/src/test/java/com/android/tools/r8/shaking/annotations/AnnotationsOnTargetedMethodTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/annotations/AnnotationsOnTargetedMethodTest.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.shaking.annotations;
+import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.NoHorizontalClassMerging;
import com.android.tools.r8.NoVerticalClassMerging;
@@ -55,6 +56,7 @@
.addKeepMainRule(TestClass.class)
.addKeepRuntimeVisibleAnnotations()
.enableInliningAnnotations()
+ .enableNeverClassInliningAnnotations()
.enableNoHorizontalClassMergingAnnotations()
.enableNoVerticalClassMergingAnnotations()
.noMinification()
@@ -94,6 +96,7 @@
void targetedMethod();
}
+ @NeverClassInline
@NoHorizontalClassMerging
static class InterfaceImpl implements Interface {
@@ -104,6 +107,7 @@
}
}
+ @NeverClassInline
@NoHorizontalClassMerging
static class OtherInterfaceImpl implements Interface {
diff --git a/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByTwoMethods.java b/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByTwoMethods.java
index 5bb8e48..035ddf7 100644
--- a/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByTwoMethods.java
+++ b/src/test/java/com/android/tools/r8/shaking/keptgraph/KeptByTwoMethods.java
@@ -5,6 +5,7 @@
import static com.android.tools.r8.references.Reference.methodFromMethod;
+import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.NoVerticalClassMerging;
import com.android.tools.r8.TestBase;
@@ -31,6 +32,7 @@
}
}
+ @NeverClassInline
@NoVerticalClassMerging
public static class B extends A {
// Intermediate to A.
@@ -72,6 +74,7 @@
GraphInspector inspector =
testForR8(parameters.getBackend())
.enableGraphInspector()
+ .enableNeverClassInliningAnnotations()
.enableNoVerticalClassMergingAnnotations()
.enableInliningAnnotations()
.addKeepMainRule(TestClass.class)
