Fix class inliner bail-out after force inlining
Bug: 145090972
Change-Id: Ifde30391c6839374649957b4b6f350e7fb1924ed
diff --git a/src/main/java/com/android/tools/r8/graph/AppInfo.java b/src/main/java/com/android/tools/r8/graph/AppInfo.java
index 6eb6a94..3b51579 100644
--- a/src/main/java/com/android/tools/r8/graph/AppInfo.java
+++ b/src/main/java/com/android/tools/r8/graph/AppInfo.java
@@ -535,9 +535,12 @@
public DexEncodedField resolveFieldOn(DexType type, DexField desc) {
assert checkIfObsolete();
DexClass holder = definitionFor(type);
- if (holder == null) {
- return null;
- }
+ return holder != null ? resolveFieldOn(holder, desc) : null;
+ }
+
+ public DexEncodedField resolveFieldOn(DexClass holder, DexField desc) {
+ assert checkIfObsolete();
+ assert holder != null;
// Step 1: Class declares the field.
DexEncodedField result = holder.lookupField(desc);
if (result != null) {
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 2f98fa7..36112c9 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
@@ -4,9 +4,11 @@
package com.android.tools.r8.ir.optimize.classinliner;
+
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexClass;
+import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexMethod;
@@ -20,6 +22,7 @@
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.If;
import com.android.tools.r8.ir.code.InstanceGet;
+import com.android.tools.r8.ir.code.InstancePut;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InstructionOrPhi;
import com.android.tools.r8.ir.code.Invoke.Type;
@@ -305,14 +308,12 @@
if (user.isInstanceGet()
|| (user.isInstancePut()
&& receivers.addIllegalReceiverAlias(user.asInstancePut().value()))) {
- DexField field = user.asFieldInstruction().getField();
- if (field.holder == eligibleClass
- && eligibleClassDefinition.lookupInstanceField(field) != null) {
- // Since class inliner currently only supports classes directly extending
- // java.lang.Object, we don't need to worry about fields defined in superclasses.
- continue;
+ DexEncodedField field =
+ appView.appInfo().resolveField(user.asFieldInstruction().getField());
+ if (field == null || field.isStatic()) {
+ return user; // Not eligible.
}
- return user; // Not eligible.
+ continue;
}
if (user.isInvokeMethod()) {
@@ -467,10 +468,44 @@
if (methodCallsOnInstance.isEmpty()) {
return false;
}
+
assert methodCallsOnInstance.keySet().stream()
.map(InvokeMethodWithReceiver::getReceiver)
.allMatch(receivers::isReceiverAlias);
+
inliner.performForcedInlining(method, code, methodCallsOnInstance, inliningIRProvider);
+
+ // In case we are class inlining an object allocation that does not inherit directly from
+ // java.lang.Object, we need keep force inlining the constructor until we reach
+ // java.lang.Object.<init>().
+ if (root.isNewInstance()) {
+ do {
+ methodCallsOnInstance.clear();
+ for (Instruction instruction : eligibleInstance.uniqueUsers()) {
+ if (instruction.isInvokeDirect()) {
+ InvokeDirect invoke = instruction.asInvokeDirect();
+ Value receiver = invoke.getReceiver();
+ if (receiver == eligibleInstance) {
+ DexMethod invokedMethod = invoke.getInvokedMethod();
+ if (appView.dexItemFactory().isConstructor(invokedMethod)
+ && invokedMethod != appView.dexItemFactory().objectMethods.constructor) {
+ methodCallsOnInstance.put(
+ invoke,
+ new InliningInfo(
+ appView.definitionFor(invokedMethod), root.asNewInstance().clazz));
+ break;
+ }
+ } else {
+ assert receiver.getAliasedValue() != eligibleInstance;
+ }
+ }
+ }
+ if (!methodCallsOnInstance.isEmpty()) {
+ inliner.performForcedInlining(method, code, methodCallsOnInstance, inliningIRProvider);
+ }
+ } while (!methodCallsOnInstance.isEmpty());
+ }
+
return true;
}
@@ -495,13 +530,14 @@
private void removeMiscUsages(IRCode code) {
boolean needToRemoveUnreachableBlocks = false;
for (Instruction user : eligibleInstance.uniqueUsers()) {
- // Remove the call to superclass constructor.
- if (root.isNewInstance()
- && user.isInvokeDirect()
- && appView.dexItemFactory().isConstructor(user.asInvokeDirect().getInvokedMethod())
- && user.asInvokeDirect().getInvokedMethod().holder == eligibleClassDefinition.superType) {
- removeInstruction(user);
- continue;
+ // Remove the call to java.lang.Object.<init>().
+ if (user.isInvokeDirect()) {
+ InvokeDirect invoke = user.asInvokeDirect();
+ if (root.isNewInstance()
+ && invoke.getInvokedMethod() == appView.dexItemFactory().objectMethods.constructor) {
+ removeInstruction(invoke);
+ continue;
+ }
}
if (user.isIf()) {
@@ -615,7 +651,10 @@
+ "` after field reads removed: "
+ user);
}
- if (user.asInstancePut().getField().holder != eligibleClass) {
+ InstancePut instancePut = user.asInstancePut();
+ DexEncodedField field =
+ appView.appInfo().resolveFieldOn(eligibleClassDefinition, instancePut.getField());
+ if (field == null) {
throw new Unreachable(
"Unexpected field write left in method `"
+ method.method.toSourceString()
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java b/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java
index 3d5f386..ee49c97 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java
@@ -444,7 +444,7 @@
if (insn.isInstancePut()) {
InstancePut instancePut = insn.asInstancePut();
- DexEncodedField field = clazz.lookupInstanceField(instancePut.getField());
+ DexEncodedField field = appView.appInfo().resolveFieldOn(clazz, instancePut.getField());
if (field == null
|| instancePut.object() != receiver
|| (instancePut.value() != receiver && !instancePut.value().isArgument())) {
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/BuilderWithInheritanceTest.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/BuilderWithInheritanceTest.java
index 02eb602..a1042ce 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/BuilderWithInheritanceTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/BuilderWithInheritanceTest.java
@@ -5,6 +5,7 @@
package com.android.tools.r8.ir.optimize.classinliner;
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.NeverInline;
@@ -46,7 +47,7 @@
.run(parameters.getRuntime(), TestClass.class)
.assertSuccessWithOutput("42")
.inspector();
- assertThat(inspector.clazz(Builder.class), isPresent());
+ assertThat(inspector.clazz(Builder.class), not(isPresent()));
}
static class TestClass {
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/EscapeFromParentConstructorTest.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/EscapeFromParentConstructorTest.java
new file mode 100644
index 0000000..2062667
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/EscapeFromParentConstructorTest.java
@@ -0,0 +1,84 @@
+// 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 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.TestParametersCollection;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+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 EscapeFromParentConstructorTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public EscapeFromParentConstructorTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(EscapeFromParentConstructorTest.class)
+ .addKeepMainRule(TestClass.class)
+ .enableInliningAnnotations()
+ .enableMergeAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspect(this::inspect)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("Hello world!");
+ }
+
+ private void inspect(CodeInspector inspector) {
+ // The receiver escapes from BuilderBase.<init>(), and therefore, Builder is not considered
+ // eligible for class inlining.
+ assertThat(inspector.clazz(BuilderBase.class), isPresent());
+ assertThat(inspector.clazz(Builder.class), isPresent());
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ System.out.println(new Builder().build());
+ }
+ }
+
+ @NeverMerge
+ static class BuilderBase {
+
+ String greeting;
+
+ BuilderBase() {
+ initialize();
+ }
+
+ @NeverInline
+ void initialize() {
+ this.greeting = "Hello world!";
+ }
+ }
+
+ static class Builder extends BuilderBase {
+
+ String build() {
+ return greeting;
+ }
+ }
+}
