Version 1.6.34
Cherry-pick: Do not inline methods on definitely null receivers
CL: https://r8-review.googlesource.com/c/r8/+/43609
Bug: 140851070
Change-Id: If5c1caef21aefb174a3cd6184df80388c93d2aa5
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 753d1b1..203966b 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
// This field is accessed from release scripts using simple pattern matching.
// Therefore, changing this field could break our release scripts.
- public static final String LABEL = "1.6.33";
+ public static final String LABEL = "1.6.34";
private Version() {
}
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 3e3ed2b..90c2a58 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -600,11 +600,6 @@
return this;
}
- public IRCode buildEmptyThrowingIRCode(AppView<?> appView, Origin origin) {
- DexCode emptyThrowingDexCode = buildEmptyThrowingDexCode();
- return emptyThrowingDexCode.buildIR(this, appView, origin);
- }
-
/**
* Generates a {@link DexCode} object for the given instructions.
*/
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 0c4e409..f90c1b6 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
@@ -351,46 +351,37 @@
info.include(invoke.getType(), candidate);
}
+ Value receiver = invoke.getReceiver();
+ if (receiver.getTypeLattice().isDefinitelyNull()) {
+ // A definitely null receiver will throw an error on call site.
+ return null;
+ }
InlineAction action = new InlineAction(candidate, invoke, reason);
- Value receiver = invoke.getReceiver();
if (receiver.getTypeLattice().isNullable()) {
- InternalOptions options = appView.options();
- if (receiver.getTypeLattice().isDefinitelyNull()) {
- if (!options.enableInliningOfInvokesWithDefinitelyNullReceivers) {
+ assert !receiver.getTypeLattice().isDefinitelyNull();
+ // When inlining an instance method call, we need to preserve the null check for the
+ // receiver. Therefore, if the receiver may be null and the candidate inlinee does not
+ // throw if the receiver is null before any other side effect, then we must synthesize a
+ // null check.
+ if (!candidate.getOptimizationInfo().checksNullReceiverBeforeAnySideEffect()) {
+ InternalOptions options = appView.options();
+ if (!options.enableInliningOfInvokesWithNullableReceivers) {
+ return null;
+ }
+ if (!options.nullableReceiverInliningFilter.isEmpty()
+ && !options.nullableReceiverInliningFilter.contains(
+ invoke.getInvokedMethod().toSourceString())) {
return null;
}
if (Log.ENABLED && Log.isLoggingEnabledFor(Inliner.class)) {
Log.debug(
Inliner.class,
- "Inlining method `%s` with definitely null receiver into `%s`",
+ "Inlining method `%s` with nullable receiver into `%s`",
invoke.getInvokedMethod().toSourceString(),
invocationContext.toSourceString());
}
- action.setShouldReturnEmptyThrowingCode();
- } else {
- // When inlining an instance method call, we need to preserve the null check for the
- // receiver. Therefore, if the receiver may be null and the candidate inlinee does not
- // throw if the receiver is null before any other side effect, then we must synthesize a
- // null check.
- if (!candidate.getOptimizationInfo().checksNullReceiverBeforeAnySideEffect()) {
- if (!options.enableInliningOfInvokesWithNullableReceivers) {
- return null;
- }
- if (!options.nullableReceiverInliningFilter.isEmpty()
- && !options.nullableReceiverInliningFilter.contains(
- invoke.getInvokedMethod().toSourceString())) {
- return null;
- }
- if (Log.ENABLED && Log.isLoggingEnabledFor(Inliner.class)) {
- Log.debug(
- Inliner.class,
- "Inlining method `%s` with nullable receiver into `%s`",
- invoke.getInvokedMethod().toSourceString(),
- invocationContext.toSourceString());
- }
- action.setShouldSynthesizeNullCheckForReceiver();
- }
+ action.setShouldSynthesizeNullCheckForReceiver();
}
}
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 c35cd11..b9c16e7 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
@@ -536,7 +536,6 @@
public final Invoke invoke;
final Reason reason;
- private boolean shouldReturnEmptyThrowingCode;
private boolean shouldSynthesizeNullCheckForReceiver;
InlineAction(DexEncodedMethod target, Invoke invoke, Reason reason) {
@@ -545,10 +544,6 @@
this.reason = reason;
}
- void setShouldReturnEmptyThrowingCode() {
- shouldReturnEmptyThrowingCode = true;
- }
-
void setShouldSynthesizeNullCheckForReceiver() {
shouldSynthesizeNullCheckForReceiver = true;
}
@@ -561,53 +556,48 @@
LensCodeRewriter lensCodeRewriter) {
Origin origin = appView.appInfo().originFor(target.method.holder);
- IRCode code;
- if (shouldReturnEmptyThrowingCode) {
- code = target.buildEmptyThrowingIRCode(appView, origin);
- } else {
- // Build the IR for a yet not processed method, and perform minimal IR processing.
- code = target.buildInliningIR(context, appView, generator, callerPosition, origin);
+ // Build the IR for a yet not processed method, and perform minimal IR processing.
+ IRCode code = target.buildInliningIR(context, appView, generator, callerPosition, origin);
- // Insert a null check if this is needed to preserve the implicit null check for the
- // receiver.
- if (shouldSynthesizeNullCheckForReceiver) {
- List<Value> arguments = code.collectArguments();
- if (!arguments.isEmpty()) {
- Value receiver = arguments.get(0);
- assert receiver.isThis();
+ // Insert a null check if this is needed to preserve the implicit null check for the
+ // receiver.
+ if (shouldSynthesizeNullCheckForReceiver) {
+ List<Value> arguments = code.collectArguments();
+ if (!arguments.isEmpty()) {
+ Value receiver = arguments.get(0);
+ assert receiver.isThis();
- BasicBlock entryBlock = code.entryBlock();
+ BasicBlock entryBlock = code.entryBlock();
- // Insert a new block between the last argument instruction and the first actual
- // instruction of the method.
- BasicBlock throwBlock =
- entryBlock.listIterator(code, arguments.size()).split(code, 0, null);
- assert !throwBlock.hasCatchHandlers();
+ // Insert a new block between the last argument instruction and the first actual
+ // instruction of the method.
+ BasicBlock throwBlock =
+ entryBlock.listIterator(code, arguments.size()).split(code, 0, null);
+ assert !throwBlock.hasCatchHandlers();
- // Link the entry block to the successor of the newly inserted block.
- BasicBlock continuationBlock = throwBlock.unlinkSingleSuccessor();
- entryBlock.link(continuationBlock);
+ // Link the entry block to the successor of the newly inserted block.
+ BasicBlock continuationBlock = throwBlock.unlinkSingleSuccessor();
+ entryBlock.link(continuationBlock);
- // Replace the last instruction of the entry block, which is now a goto instruction,
- // with an `if-eqz` instruction that jumps to the newly inserted block if the receiver
- // is null.
- If ifInstruction = new If(If.Type.EQ, receiver);
- entryBlock.replaceLastInstruction(ifInstruction, code);
- assert ifInstruction.getTrueTarget() == throwBlock;
- assert ifInstruction.fallthroughBlock() == continuationBlock;
+ // Replace the last instruction of the entry block, which is now a goto instruction,
+ // with an `if-eqz` instruction that jumps to the newly inserted block if the receiver
+ // is null.
+ If ifInstruction = new If(If.Type.EQ, receiver);
+ entryBlock.replaceLastInstruction(ifInstruction, code);
+ assert ifInstruction.getTrueTarget() == throwBlock;
+ assert ifInstruction.fallthroughBlock() == continuationBlock;
- // Replace the single goto instruction in the newly inserted block by `throw null`.
- InstructionListIterator iterator = throwBlock.listIterator(code);
- Value nullValue = iterator.insertConstNullInstruction(code, appView.options());
- iterator.next();
- iterator.replaceCurrentInstruction(new Throw(nullValue));
- } else {
- assert false : "Unable to synthesize a null check for the receiver";
- }
+ // Replace the single goto instruction in the newly inserted block by `throw null`.
+ InstructionListIterator iterator = throwBlock.listIterator(code);
+ Value nullValue = iterator.insertConstNullInstruction(code, appView.options());
+ iterator.next();
+ iterator.replaceCurrentInstruction(new Throw(nullValue));
+ } else {
+ assert false : "Unable to synthesize a null check for the receiver";
}
- if (!target.isProcessed()) {
- lensCodeRewriter.rewrite(code, target);
- }
+ }
+ if (!target.isProcessed()) {
+ lensCodeRewriter.rewrite(code, target);
}
return new InlineeWithReason(code, reason);
}
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 dbd4989..faf5a9e 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -196,9 +196,6 @@
public boolean enableNonNullTracking = true;
public boolean enableInlining =
!Version.isDev() || System.getProperty("com.android.tools.r8.disableinlining") == null;
- public boolean enableInliningOfInvokesWithDefinitelyNullReceivers =
- System.getProperty("com.android.tools.r8.disableInliningOfInvokesWithDefinitelyNullReceivers")
- == null;
public boolean enableInliningOfInvokesWithNullableReceivers = true;
public boolean disableInliningOfLibraryMethodOverrides = true;
public boolean enableClassInlining = true;
