Fix inlining of instance initializers
Change-Id: Ica254cb4b06901a2ca45a3a8f3a997a2dfa3c95d
diff --git a/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java b/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
index 661371c..fa9384a 100644
--- a/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
+++ b/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
@@ -42,6 +42,7 @@
import java.util.ListIterator;
import java.util.Map;
import java.util.Map.Entry;
+import java.util.NoSuchElementException;
import java.util.Set;
import java.util.WeakHashMap;
import java.util.function.Consumer;
@@ -566,6 +567,40 @@
return () -> iterator(instruction);
}
+ public Iterable<Instruction> instructionsBefore(Instruction instruction) {
+ return () ->
+ new Iterator<Instruction>() {
+
+ private InstructionIterator iterator = iterator();
+ private Instruction next = advance();
+
+ private Instruction advance() {
+ if (iterator.hasNext()) {
+ Instruction next = iterator.next();
+ if (next != instruction) {
+ return next;
+ }
+ }
+ return null;
+ }
+
+ @Override
+ public boolean hasNext() {
+ return next != null;
+ }
+
+ @Override
+ public Instruction next() {
+ Instruction result = next;
+ if (result == null) {
+ throw new NoSuchElementException();
+ }
+ next = advance();
+ return result;
+ }
+ };
+ }
+
public boolean isEmpty() {
return instructions.isEmpty();
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/IRCode.java b/src/main/java/com/android/tools/r8/ir/code/IRCode.java
index c4180a3..fee2c94 100644
--- a/src/main/java/com/android/tools/r8/ir/code/IRCode.java
+++ b/src/main/java/com/android/tools/r8/ir/code/IRCode.java
@@ -1167,7 +1167,7 @@
* <p>Note: It is the responsibility of the caller to return the marking color.
*/
public void markTransitivePredecessors(BasicBlock subject, int color) {
- assert isMarkingColorInUse(color) && !anyBlocksMarkedWithColor(color);
+ assert isMarkingColorInUse(color);
Queue<BasicBlock> worklist = new ArrayDeque<>();
worklist.add(subject);
while (!worklist.isEmpty()) {
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 f755108..d23805d 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
@@ -15,7 +15,6 @@
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.Instruction;
-import com.android.tools.r8.ir.code.InstructionIterator;
import com.android.tools.r8.ir.code.InvokeDirect;
import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.ir.code.InvokeMethodWithReceiver;
@@ -33,6 +32,7 @@
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.IteratorUtils;
import com.google.common.collect.Sets;
+import java.util.ArrayList;
import java.util.BitSet;
import java.util.List;
import java.util.ListIterator;
@@ -460,7 +460,6 @@
@Override
public boolean canInlineInstanceInitializer(
- InvokeDirect invoke,
IRCode inlinee,
WhyAreYouNotInliningReporter whyAreYouNotInliningReporter) {
// In the Java VM Specification section "4.10.2.4. Instance Initialization Methods and
@@ -473,68 +472,88 @@
// Allow inlining a constructor into a constructor of the same class, as the constructor code
// is expected to adhere to the VM specification.
DexType callerMethodHolder = method.method.holder;
- boolean callerMethodIsConstructor = method.isInstanceInitializer();
- DexType calleeMethodHolder = invoke.asInvokeMethod().getInvokedMethod().holder;
+ DexType calleeMethodHolder = inlinee.method.method.holder;
// Calling a constructor on the same class from a constructor can always be inlined.
- if (callerMethodIsConstructor && callerMethodHolder == calleeMethodHolder) {
+ if (method.isInstanceInitializer() && callerMethodHolder == calleeMethodHolder) {
return true;
}
- // We cannot invoke <init> on other values than |this| on Dalvik 4.4.4. Compute whether
- // the receiver to the call was the this value at the call-site.
- boolean receiverOfInnerCallIsThisOfOuter = invoke.getReceiver().isThis();
+ // Only allow inlining a constructor into a non-constructor if:
+ // (1) the first use of the uninitialized object is the receiver of an invoke of <init>(),
+ // (2) the constructor does not initialize any final fields, as such is only allowed from within
+ // a constructor of the corresponding class, and
+ // (3) the constructors own <init>() call is on the same class.
+ //
+ // Note that, due to (3), we do allow inlining of `A(int x)` into another class, but not the
+ // default constructor `A()`, since the default constructor invokes Object.<init>().
+ //
+ // class A {
+ // A() { ... }
+ // A(int x) {
+ // this()
+ // ...
+ // }
+ // }
+ Value thisValue = inlinee.entryBlock().entry().asArgument().outValue();
- // Don't allow inlining a constructor into a non-constructor if the first use of the
- // un-initialized object is not an argument of an invoke of <init>.
- // Also, we cannot inline a constructor if it initializes final fields, as such is only allowed
- // from within a constructor of the corresponding class.
- // Lastly, we can only inline a constructor, if its own <init> call is on the method's class. If
- // we inline into a constructor, calls to super.<init> are also OK if the receiver of the
- // super.<init> call is the this argument.
- InstructionIterator iterator = inlinee.instructionIterator();
- Instruction instruction = iterator.next();
- // A constructor always has the un-initialized object as the first argument.
- assert instruction.isArgument();
- Value unInitializedObject = instruction.outValue();
- boolean seenSuperInvoke = false;
- while (iterator.hasNext()) {
- instruction = iterator.next();
- if (instruction.inValues().contains(unInitializedObject)) {
- if (instruction.isInvokeDirect() && !seenSuperInvoke) {
- DexMethod target = instruction.asInvokeDirect().getInvokedMethod();
- seenSuperInvoke = appView.dexItemFactory().isConstructor(target);
- boolean callOnConstructorThatCallsConstructorSameClass =
- calleeMethodHolder == target.holder;
- boolean callOnSupertypeOfThisInConstructor =
- appView.appInfo().isDirectSubtype(callerMethodHolder, target.holder)
- && instruction.asInvokeDirect().getReceiver() == unInitializedObject
- && receiverOfInnerCallIsThisOfOuter
- && callerMethodIsConstructor;
- if (seenSuperInvoke
- // Calls to init on same class than the called constructor are OK.
- && !callOnConstructorThatCallsConstructorSameClass
- // If we are inlining into a constructor, calls to superclass init are only OK on the
- // |this| value in the outer context.
- && !callOnSupertypeOfThisInConstructor) {
- return false;
+ List<InvokeDirect> initCallsOnThis = new ArrayList<>();
+ for (Instruction instruction : inlinee.instructions()) {
+ if (instruction.isInvokeDirect()) {
+ InvokeDirect initCall = instruction.asInvokeDirect();
+ DexMethod invokedMethod = initCall.getInvokedMethod();
+ if (appView.dexItemFactory().isConstructor(invokedMethod)) {
+ Value receiver = initCall.getReceiver().getAliasedValue();
+ if (receiver == thisValue) {
+ // The <init>() call of the constructor must be on the same class.
+ if (calleeMethodHolder != invokedMethod.holder) {
+ return false;
+ }
+ initCallsOnThis.add(initCall);
}
}
- if (!seenSuperInvoke) {
- return false;
- }
- }
- if (instruction.isInstancePut()) {
- // Fields may not be initialized outside of a constructor.
- if (!callerMethodIsConstructor) {
- return false;
- }
+ } else if (instruction.isInstancePut()) {
+ // Final fields may not be initialized outside of a constructor in the enclosing class.
DexField field = instruction.asInstancePut().getField();
DexEncodedField target = appView.appInfo().lookupInstanceTarget(field.holder, field);
- if (target != null && target.accessFlags.isFinal()) {
+ if (target == null || target.accessFlags.isFinal()) {
return false;
}
}
}
+
+ // Check that there are no uses of the uninitialized object before it gets initialized.
+ int markingColor = inlinee.reserveMarkingColor();
+ for (InvokeDirect initCallOnThis : initCallsOnThis) {
+ BasicBlock block = initCallOnThis.getBlock();
+ for (Instruction instruction : block.instructionsBefore(initCallOnThis)) {
+ for (Value inValue : instruction.inValues()) {
+ Value root = inValue.getAliasedValue();
+ if (root == thisValue) {
+ inlinee.returnMarkingColor(markingColor);
+ return false;
+ }
+ }
+ }
+ for (BasicBlock predecessor : block.getPredecessors()) {
+ inlinee.markTransitivePredecessors(predecessor, markingColor);
+ }
+ }
+
+ for (BasicBlock block : inlinee.blocks) {
+ if (block.isMarked(markingColor)) {
+ for (Instruction instruction : block.getInstructions()) {
+ for (Value inValue : instruction.inValues()) {
+ Value root = inValue.getAliasedValue();
+ if (root == thisValue) {
+ inlinee.returnMarkingColor(markingColor);
+ return false;
+ }
+ }
+ }
+ }
+ }
+
+ inlinee.returnMarkingColor(markingColor);
return true;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/ForcedInliningOracle.java b/src/main/java/com/android/tools/r8/ir/optimize/ForcedInliningOracle.java
index 4f34bd3..b44e32c 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/ForcedInliningOracle.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/ForcedInliningOracle.java
@@ -11,7 +11,6 @@
import com.android.tools.r8.ir.analysis.ClassInitializationAnalysis;
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.IRCode;
-import com.android.tools.r8.ir.code.InvokeDirect;
import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.ir.code.InvokeMethodWithReceiver;
import com.android.tools.r8.ir.code.InvokeStatic;
@@ -97,7 +96,7 @@
@Override
public boolean canInlineInstanceInitializer(
- InvokeDirect invoke, IRCode code, WhyAreYouNotInliningReporter whyAreYouNotInliningReporter) {
+ IRCode code, WhyAreYouNotInliningReporter whyAreYouNotInliningReporter) {
return true;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
index 9c5a539..b6ae500 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
@@ -776,7 +776,7 @@
assert !singleTarget.isClassInitializer();
if (singleTarget.isInstanceInitializer()
&& !strategy.canInlineInstanceInitializer(
- invoke.asInvokeDirect(), inlinee.code, whyAreYouNotInliningReporter)) {
+ inlinee.code, whyAreYouNotInliningReporter)) {
// TODO(b/142108662): Enable assertion once reporting is complete.
// assert whyAreYouNotInliningReporter.verifyReasonHasBeenReported();
continue;
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/InliningStrategy.java b/src/main/java/com/android/tools/r8/ir/optimize/InliningStrategy.java
index 5d7bdc2..21b060a 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/InliningStrategy.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/InliningStrategy.java
@@ -8,7 +8,6 @@
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.IRCode;
-import com.android.tools.r8.ir.code.InvokeDirect;
import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.ir.optimize.Inliner.InlineAction;
import com.android.tools.r8.ir.optimize.Inliner.InlineeWithReason;
@@ -19,7 +18,7 @@
interface InliningStrategy {
boolean canInlineInstanceInitializer(
- InvokeDirect invoke, IRCode code, WhyAreYouNotInliningReporter whyAreYouNotInliningReporter);
+ IRCode code, WhyAreYouNotInliningReporter whyAreYouNotInliningReporter);
/** Return true if there is still budget for inlining into this method. */
boolean stillHasBudget(
