Fix class inliner bug that leads to unnecessary bail-outs for candidates not inheriting from Object
Change-Id: I2b35965264493083bd0e56e868c90c60a9cd227f
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
index c2fdaaf..7ef077a 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -1094,12 +1094,23 @@
private TrivialInitializer() {
}
+ public boolean isTrivialInstanceInitializer() {
+ return false;
+ }
+
// Defines instance trivial initialized, see details in comments
// to CodeRewriter::computeInstanceInitializerInfo(...)
public static final class TrivialInstanceInitializer extends TrivialInitializer {
public static final TrivialInstanceInitializer INSTANCE =
new TrivialInstanceInitializer();
+
+ private TrivialInstanceInitializer() {}
+
+ @Override
+ public boolean isTrivialInstanceInitializer() {
+ return true;
+ }
}
// Defines class trivial initialized, see details in comments
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 be28c03..2883353 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
@@ -11,7 +11,6 @@
import com.android.tools.r8.graph.DexEncodedMethod.ClassInlinerEligibility;
import com.android.tools.r8.graph.DexEncodedMethod.TrivialInitializer;
import com.android.tools.r8.graph.DexEncodedMethod.TrivialInitializer.TrivialClassInitializer;
-import com.android.tools.r8.graph.DexEncodedMethod.TrivialInitializer.TrivialInstanceInitializer;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexType;
@@ -557,10 +556,23 @@
// any class initializers in superclass chain or in superinterfaces, see
// details in ClassInliner::computeClassEligible(...).
if (eligibleClassDefinition.superType != appView.dexItemFactory().objectType) {
- TrivialInitializer info = singleTarget.getOptimizationInfo().getTrivialInitializerInfo();
- if (!(info instanceof TrivialInstanceInitializer)) {
+ DexClass superClass = appView.definitionFor(eligibleClassDefinition.superType);
+ if (superClass == null || !superClass.isProgramClass()) {
return null;
}
+
+ // At this point, we don't know which constructor in the super type that is invoked from the
+ // method. Therefore, we just check if all of the constructors in the super type are trivial.
+ for (DexEncodedMethod method : superClass.directMethods()) {
+ if (method.isInstanceInitializer()) {
+ TrivialInitializer trivialInitializerInfo =
+ method.getOptimizationInfo().getTrivialInitializerInfo();
+ if (trivialInitializerInfo == null
+ || !trivialInitializerInfo.isTrivialInstanceInitializer()) {
+ return null;
+ }
+ }
+ }
}
return singleTarget.getOptimizationInfo().getClassInlinerEligibility() != null
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerWithSimpleSuperTypeTest.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerWithSimpleSuperTypeTest.java
new file mode 100644
index 0000000..4cdbc84
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerWithSimpleSuperTypeTest.java
@@ -0,0 +1,107 @@
+// 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.classinliner;
+
+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.NeverMerge;
+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 ClassInlinerWithSimpleSuperTypeTest extends TestBase {
+
+ private final boolean enableClassInlining;
+ private final TestParameters parameters;
+
+ @Parameters(name = "{1}, enable class inlining: {0}")
+ public static List<Object[]> data() {
+ return buildParameters(BooleanUtils.values(), getTestParameters().withAllRuntimes().build());
+ }
+
+ public ClassInlinerWithSimpleSuperTypeTest(
+ boolean enableClassInlining, TestParameters parameters) {
+ this.enableClassInlining = enableClassInlining;
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(ClassInlinerWithSimpleSuperTypeTest.class)
+ .addKeepMainRule(TestClass.class)
+ .addOptionsModification(options -> options.enableClassInlining = enableClassInlining)
+ .enableInliningAnnotations()
+ .enableMergeAnnotations()
+ .setMinApi(parameters.getRuntime())
+ .compile()
+ .inspect(this::verifyCandidateIsClassInlined)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("Hello world!");
+ }
+
+ private void verifyCandidateIsClassInlined(CodeInspector inspector) {
+ ClassSubject testClassSubject = inspector.clazz(TestClass.class);
+ assertThat(testClassSubject, isPresent());
+
+ MethodSubject mainMethodSubject = testClassSubject.mainMethod();
+ assertThat(mainMethodSubject, isPresent());
+ assertNotEquals(
+ enableClassInlining,
+ mainMethodSubject.streamInstructions().anyMatch(InstructionSubject::isNewInstance));
+
+ ClassSubject aClassSubject = inspector.clazz(A.class);
+ assertNotEquals(enableClassInlining, aClassSubject.isPresent());
+
+ ClassSubject bClassSubject = inspector.clazz(B.class);
+ assertNotEquals(enableClassInlining, bClassSubject.isPresent());
+
+ ClassSubject cClassSubject = inspector.clazz(C.class);
+ assertNotEquals(enableClassInlining, cClassSubject.isPresent());
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ new C("Hello world!").method();
+ }
+ }
+
+ @NeverMerge
+ static class A {}
+
+ @NeverMerge
+ static class B extends A {}
+
+ static class C extends B {
+
+ String greeting;
+
+ C(String greeting) {
+ if (greeting == null) {
+ throw new RuntimeException();
+ }
+ this.greeting = greeting;
+ }
+
+ @NeverInline
+ void method() {
+ System.out.println(greeting);
+ }
+ }
+}
