Add a test for inlining of library method overrides
Change-Id: I204a5823b1b5aaba42bf49ee2cbd1c71c10a60e6
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CallSiteInformation.java b/src/main/java/com/android/tools/r8/ir/conversion/CallSiteInformation.java
index 36b5a83..937b0ca 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/CallSiteInformation.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/CallSiteInformation.java
@@ -54,8 +54,12 @@
// For non-pinned methods and methods that override library methods we do not know the exact
// number of call sites.
- if (appView.appInfo().isPinned(method)
- || encodedMethod.isLibraryMethodOverride().isTrue()) {
+ if (appView.appInfo().isPinned(method)) {
+ continue;
+ }
+
+ if (appView.options().disableInliningOfLibraryMethodOverrides
+ && encodedMethod.isLibraryMethodOverride().isTrue()) {
continue;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
index c45d136..d9e8efe 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
@@ -76,11 +76,10 @@
}
private DexEncodedMethod validateCandidate(InvokeMethod invoke, DexMethod invocationContext) {
- DexEncodedMethod candidate =
- invoke.lookupSingleTarget(inliner.appView, invocationContext.holder);
+ DexEncodedMethod candidate = invoke.lookupSingleTarget(appView, invocationContext.holder);
if ((candidate == null)
|| (candidate.getCode() == null)
- || inliner.appView.definitionFor(candidate.method.holder).isNotProgramClass()) {
+ || appView.definitionFor(candidate.method.holder).isNotProgramClass()) {
if (info != null) {
info.exclude(invoke, "No inlinee");
}
@@ -100,16 +99,17 @@
private Reason computeInliningReason(DexEncodedMethod target) {
if (target.getOptimizationInfo().forceInline()
- || (inliner.appView.appInfo().hasLiveness()
- && inliner.appView.withLiveness().appInfo().forceInline.contains(target.method))) {
+ || (appView.appInfo().hasLiveness()
+ && appView.withLiveness().appInfo().forceInline.contains(target.method))) {
assert !appView.appInfo().neverInline.contains(target.method);
return Reason.FORCE;
}
- if (inliner.appView.appInfo().hasLiveness()
- && inliner.appView.withLiveness().appInfo().alwaysInline.contains(target.method)) {
+ if (appView.appInfo().hasLiveness()
+ && appView.withLiveness().appInfo().alwaysInline.contains(target.method)) {
return Reason.ALWAYS;
}
- if (target.isLibraryMethodOverride().isTrue()) {
+ if (appView.options().disableInliningOfLibraryMethodOverrides
+ && target.isLibraryMethodOverride().isTrue()) {
// This method will always have an implicit call site from the library, so we won't be able to
// remove it after inlining even if we have single or dual call site information from the
// program.
@@ -140,7 +140,7 @@
if (appView.appInfo().isSubtype(method.method.holder, targetHolder)) {
return true;
}
- DexClass clazz = inliner.appView.definitionFor(targetHolder);
+ DexClass clazz = appView.definitionFor(targetHolder);
assert clazz != null;
if (target.getOptimizationInfo().triggersClassInitBeforeAnySideEffect()) {
return true;
@@ -226,7 +226,7 @@
return false;
}
- DexClass holder = inliner.appView.definitionFor(candidate.method.holder);
+ DexClass holder = appView.definitionFor(candidate.method.holder);
if (holder.isInterface()) {
// Art978_virtual_interfaceTest correctly expects an IncompatibleClassChangeError exception at
@@ -335,7 +335,7 @@
}
Reason reason = computeInliningReason(candidate);
- if (!candidate.isInliningCandidate(method, reason, inliner.appView.appInfo())) {
+ if (!candidate.isInliningCandidate(method, reason, appView.appInfo())) {
// Abort inlining attempt if the single target is not an inlining candidate.
if (info != null) {
info.exclude(invoke, "target is not identified for inlining");
@@ -409,7 +409,7 @@
Reason reason = computeInliningReason(candidate);
// Determine if this should be inlined no matter how big it is.
- if (!candidate.isInliningCandidate(method, reason, inliner.appView.appInfo())) {
+ if (!candidate.isInliningCandidate(method, reason, appView.appInfo())) {
// Abort inlining attempt if the single target is not an inlining candidate.
if (info != null) {
info.exclude(invoke, "target is not identified for inlining");
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index e43b307..1f19169 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -199,6 +199,7 @@
System.getProperty("com.android.tools.r8.disableInliningOfInvokesWithDefinitelyNullReceivers")
== null;
public boolean enableInliningOfInvokesWithNullableReceivers = true;
+ public boolean disableInliningOfLibraryMethodOverrides = true;
public boolean enableClassInlining = true;
public boolean enableClassStaticizer = true;
public boolean enableInitializedClassesAnalysis = true;
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/inliner/LibraryOverrideInliningTest.java b/src/test/java/com/android/tools/r8/ir/optimize/inliner/LibraryOverrideInliningTest.java
new file mode 100644
index 0000000..1a70cde
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/inliner/LibraryOverrideInliningTest.java
@@ -0,0 +1,104 @@
+// 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.inliner;
+
+import static com.android.tools.r8.utils.codeinspector.CodeMatchers.invokesMethod;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.NeverClassInline;
+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.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 LibraryOverrideInliningTest extends TestBase {
+
+ private final boolean disableInliningOfLibraryMethodOverrides;
+ private final TestParameters parameters;
+
+ @Parameters(name = "{1}, disableInliningOfLibraryMethodOverrides: {0}")
+ public static List<Object[]> data() {
+ return buildParameters(BooleanUtils.values(), getTestParameters().withAllRuntimes().build());
+ }
+
+ public LibraryOverrideInliningTest(
+ boolean disableInliningOfLibraryMethodOverrides, TestParameters parameters) {
+ this.disableInliningOfLibraryMethodOverrides = disableInliningOfLibraryMethodOverrides;
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(LibraryOverrideInliningTest.class)
+ .addKeepMainRule(TestClass.class)
+ .addOptionsModification(
+ options ->
+ options.disableInliningOfLibraryMethodOverrides =
+ disableInliningOfLibraryMethodOverrides)
+ .enableClassInliningAnnotations()
+ .setMinApi(parameters.getRuntime())
+ .compile()
+ .inspect(
+ inspector -> {
+ ClassSubject greeterClassSubject = inspector.clazz(Greeter.class);
+ assertThat(greeterClassSubject, isPresent());
+
+ MethodSubject toStringMethodSubject =
+ greeterClassSubject.uniqueMethodWithName("toString");
+ assertThat(toStringMethodSubject, isPresent());
+
+ ClassSubject testClassSubject = inspector.clazz(TestClass.class);
+ assertThat(testClassSubject, isPresent());
+
+ MethodSubject mainMethodSubject = testClassSubject.mainMethod();
+ assertThat(mainMethodSubject, isPresent());
+
+ if (disableInliningOfLibraryMethodOverrides) {
+ assertThat(mainMethodSubject, invokesMethod(toStringMethodSubject));
+ } else {
+ assertThat(mainMethodSubject, not(invokesMethod(toStringMethodSubject)));
+ }
+ })
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("Hello world!");
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ System.out.println(new Greeter().toString());
+ }
+ }
+
+ @NeverClassInline
+ static class Greeter {
+
+ final char[] greeting = new char["Hello world!".length()];
+
+ // A method that returns "Hello world!" and is not a 'SIMPLE' inlining candidate.
+ @Override
+ public String toString() {
+ String greeting = "Hello world!";
+ for (int i = 0; i < greeting.length(); i++) {
+ this.greeting[i] = greeting.charAt(i);
+ }
+ StringBuilder result = new StringBuilder();
+ for (char c : greeting.toCharArray()) {
+ result.append(c);
+ }
+ return result.toString();
+ }
+ }
+}
