Fix insertion of null check in inliner
Bug: 216407481
Change-Id: Ibda8b9bdca9fea0d1786aebf2885130e118f0a9f
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 834e78d..063a54b 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
@@ -577,8 +577,7 @@
&& !isSynthesizingNullCheckForReceiverUsingMonitorEnter) {
SimpleEffectAnalysisResult checksReceiverBeingNull =
canInlineWithoutSynthesizingNullCheckForReceiver(appView, code);
- if (!checksReceiverBeingNull.hasResult()
- || !checksReceiverBeingNull.isPartial()
+ if (checksReceiverBeingNull.isNotSatisfied()
|| (checksReceiverBeingNull.isPartial()
&& checksReceiverBeingNull.topMostNotSatisfiedBlockSize() > 1)) {
synthesizeNullCheckForReceiver(appView, code, invoke, code.entryBlock());
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/SimpleDominatingEffectAnalysis.java b/src/main/java/com/android/tools/r8/ir/optimize/SimpleDominatingEffectAnalysis.java
index 8942395..ae95f9e 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/SimpleDominatingEffectAnalysis.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/SimpleDominatingEffectAnalysis.java
@@ -166,18 +166,19 @@
public static class SimpleEffectAnalysisResult {
+ private final ResultState result;
private final List<Instruction> satisfyingInstructions;
private final List<BasicBlock> topmostNotSatisfiedBlocks;
private SimpleEffectAnalysisResult(
- List<Instruction> satisfyingInstructions, List<BasicBlock> topmostNotSatisfiedBlocks) {
-
+ ResultState result,
+ List<Instruction> satisfyingInstructions,
+ List<BasicBlock> topmostNotSatisfiedBlocks) {
+ this.result = result;
this.satisfyingInstructions = satisfyingInstructions;
this.topmostNotSatisfiedBlocks = topmostNotSatisfiedBlocks;
- }
-
- public boolean hasResult() {
- return true;
+ assert !result.isPartial()
+ || (!satisfyingInstructions.isEmpty() && !topmostNotSatisfiedBlocks.isEmpty());
}
public void forEachSatisfyingInstruction(Consumer<Instruction> instructionConsumer) {
@@ -196,8 +197,12 @@
return new SimpleEffectAnalysisResultBuilder();
}
+ public boolean isNotSatisfied() {
+ return result.isNotSatisfied();
+ }
+
public boolean isPartial() {
- return !satisfyingInstructions.isEmpty() && !topmostNotSatisfiedBlocks.isEmpty();
+ return result.isPartial();
}
}
@@ -205,10 +210,10 @@
List<Instruction> satisfyingInstructions = new ArrayList<>();
List<BasicBlock> failingBlocksForPartialResults = ImmutableList.of();
- private boolean isFailed;
+ private ResultState result;
public void fail() {
- isFailed = true;
+ result = ResultState.NOT_SATISFIED;
}
public void addSatisfyingInstruction(Instruction instruction) {
@@ -219,20 +224,21 @@
this.failingBlocksForPartialResults = basicBlocks;
}
+ public void setResult(ResultState result) {
+ this.result = result;
+ }
+
public SimpleEffectAnalysisResult build() {
- return isFailed
+ return result.isNotComputed()
? NO_RESULT
- : new SimpleEffectAnalysisResult(satisfyingInstructions, failingBlocksForPartialResults);
+ : new SimpleEffectAnalysisResult(
+ result, satisfyingInstructions, failingBlocksForPartialResults);
}
}
private static final SimpleEffectAnalysisResult NO_RESULT =
- new SimpleEffectAnalysisResult(ImmutableList.of(), ImmutableList.of()) {
- @Override
- public boolean hasResult() {
- return false;
- }
- };
+ new SimpleEffectAnalysisResult(
+ ResultState.NOT_SATISFIED, ImmutableList.of(), ImmutableList.of());
public static SimpleEffectAnalysisResult run(IRCode code, InstructionAnalysis analysis) {
SimpleEffectAnalysisResultBuilder builder = SimpleEffectAnalysisResult.builder();
@@ -288,6 +294,7 @@
}
node.setState(resultState);
if (node.getNode().isEntry()) {
+ builder.setResult(resultState.state);
builder.setFailingBlocksForPartialResults(resultState.failingBlocks);
}
return CONTINUE;
diff --git a/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheckInlinedTest.java b/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheckInlinedTest.java
index 6b7b2d0..bee2e58 100644
--- a/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheckInlinedTest.java
+++ b/src/test/java/com/android/tools/r8/naming/retrace/RetraceInlineeWithNullCheckInlinedTest.java
@@ -4,21 +4,19 @@
package com.android.tools.r8.naming.retrace;
import static com.android.tools.r8.naming.retrace.StackTrace.isSame;
-import static com.android.tools.r8.naming.retrace.StackTrace.isSameExceptForLineNumbers;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresentAndRenamed;
+import static com.android.tools.r8.utils.codeinspector.Matchers.notIf;
import static org.hamcrest.MatcherAssert.assertThat;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.NoMethodStaticizing;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
-import com.android.tools.r8.naming.retrace.StackTrace.StackTraceLine;
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 java.util.Objects;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -72,29 +70,8 @@
assertThat(callerClass, isPresent());
MethodSubject staticized = callerClass.uniqueMethodWithName("outerCaller");
assertThat(staticized, isPresentAndRenamed());
- // TODO(b/214377135): The stack traces should be the same (when 206427323) is
- // resolved.
- if (throwReceiverNpe && canUseJavaUtilObjectsRequireNonNull(parameters)) {
- StackTrace requireNonNullFrame =
- StackTrace.builder().add(stackTrace.get(0)).build();
- assertThat(
- requireNonNullFrame,
- isSameExceptForLineNumbers(
- StackTrace.builder()
- .add(
- StackTraceLine.builder()
- .setClassName(Objects.class.getTypeName())
- .setMethodName("requireNonNull")
- .setFileName("Objects.java")
- .build())
- .build()));
-
- StackTrace stackTraceWithoutRequireNonNull =
- StackTrace.builder().add(stackTrace).remove(0).build();
- assertThat(stackTraceWithoutRequireNonNull, isSame(expectedStackTrace));
- } else {
- assertThat(stackTrace, isSame(expectedStackTrace));
- }
+ // TODO(b/216464180): This should be fixed by maintaining the bit on positions.
+ assertThat(stackTrace, notIf(isSame(expectedStackTrace), throwReceiverNpe));
});
}
