Merge "Rerun partial type analysis after NonNull removal"
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/type/TypeAnalysis.java b/src/main/java/com/android/tools/r8/ir/analysis/type/TypeAnalysis.java
index 6a0b9bc..ffbd3fd 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/type/TypeAnalysis.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/type/TypeAnalysis.java
@@ -51,6 +51,13 @@
analyze();
}
+ public void widening(Iterable<Value> values) {
+ mode = Mode.WIDENING;
+ assert worklist.isEmpty();
+ values.forEach(this::enqueue);
+ analyze();
+ }
+
public void narrowing(Iterable<Value> values) {
mode = Mode.NARROWING;
assert worklist.isEmpty();
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index a883959..a662e1e 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -161,7 +161,7 @@
this.enableWholeProgramOptimizations = appView != null;
if (enableWholeProgramOptimizations) {
assert appInfo.hasLiveness();
- this.nonNullTracker = new NonNullTracker();
+ this.nonNullTracker = new NonNullTracker(appInfo);
this.inliner = new Inliner(appView.withLiveness(), this, options);
this.outliner = new Outliner(appInfo.withLiveness(), options, this);
this.memberValuePropagation =
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 fc11b78..479c3e4 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
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.ir.optimize;
+import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.Code;
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexEncodedMethod;
@@ -19,6 +20,7 @@
import com.android.tools.r8.ir.optimize.Inliner.InlineAction;
import com.android.tools.r8.ir.optimize.Inliner.Reason;
import com.android.tools.r8.logging.Log;
+import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.IteratorUtils;
import java.util.BitSet;
@@ -29,6 +31,7 @@
final class DefaultInliningOracle implements InliningOracle, InliningStrategy {
+ private final AppView<? extends AppInfoWithLiveness> appView;
private final Inliner inliner;
private final DexEncodedMethod method;
private final IRCode code;
@@ -40,6 +43,7 @@
private int instructionAllowance;
DefaultInliningOracle(
+ AppView<? extends AppInfoWithLiveness> appView,
Inliner inliner,
DexEncodedMethod method,
IRCode code,
@@ -48,6 +52,7 @@
InternalOptions options,
int inliningInstructionLimit,
int inliningInstructionAllowance) {
+ this.appView = appView;
this.inliner = inliner;
this.method = method;
this.code = code;
@@ -379,8 +384,9 @@
assert IteratorUtils.peekNext(blockIterator) == block;
// Kick off the tracker to add non-null IRs only to the inlinee blocks.
- Set<Value> nonNullValues = new NonNullTracker()
- .addNonNullInPart(code, blockIterator, inlinee.blocks::contains);
+ Set<Value> nonNullValues =
+ new NonNullTracker(appView.appInfo())
+ .addNonNullInPart(code, blockIterator, inlinee.blocks::contains);
assert !blockIterator.hasNext();
// Restore the old state of the iterator.
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 000eac2..2a74cad 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
@@ -559,6 +559,7 @@
int inliningInstructionAllowance) {
return new DefaultInliningOracle(
+ appView,
this,
method,
code,
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/NonNullTracker.java b/src/main/java/com/android/tools/r8/ir/optimize/NonNullTracker.java
index e6997d4..536eac8 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/NonNullTracker.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/NonNullTracker.java
@@ -4,6 +4,8 @@
package com.android.tools.r8.ir.optimize;
import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.graph.AppInfo;
+import com.android.tools.r8.ir.analysis.type.TypeAnalysis;
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.DominatorTree;
import com.android.tools.r8.ir.code.IRCode;
@@ -30,7 +32,10 @@
public class NonNullTracker {
- public NonNullTracker() {
+ private final AppInfo appInfo;
+
+ public NonNullTracker(AppInfo appInfo) {
+ this.appInfo = appInfo;
}
@VisibleForTesting
@@ -260,6 +265,8 @@
}
public void cleanupNonNull(IRCode code) {
+ Set<Value> affectedValues = Sets.newIdentityHashSet();
+
InstructionIterator it = code.instructionIterator();
boolean needToCheckTrivialPhis = false;
while (it.hasNext()) {
@@ -275,6 +282,16 @@
NonNull nonNull = instruction.asNonNull();
Value src = nonNull.src();
Value dest = nonNull.dest();
+
+ // Add all values whose definition may depend on `dest` to `affectedValues`.
+ for (Instruction user : dest.uniqueUsers()) {
+ if (user.outValue() != null) {
+ affectedValues.add(user.outValue());
+ }
+ }
+ affectedValues.addAll(dest.uniquePhiUsers());
+
+ // Replace `dest` by `src`.
needToCheckTrivialPhis = needToCheckTrivialPhis || dest.uniquePhiUsers().size() != 0;
dest.replaceUsers(src);
it.remove();
@@ -290,6 +307,17 @@
if (needToCheckTrivialPhis) {
code.removeAllTrivialPhis();
}
+
+ // We need to update the types of all values whose definitions depend on a non-null value.
+ // This is needed to preserve soundness of the types after the NonNull instructions have been
+ // removed.
+ //
+ // As an example, consider a check-cast instruction on the form "z = (T) y". If y used to be
+ // defined by a NonNull instruction, then the type analysis could have used this information
+ // to mark z as non-null. However, cleanupNonNull() have now replaced y by a nullable value x.
+ // Since z is defined as "z = (T) x", and x is nullable, it is no longer sound to have that z
+ // is not nullable. This is fixed by rerunning the type analysis for the affected values.
+ new TypeAnalysis(appInfo, code.method).widening(affectedValues);
}
}
diff --git a/src/test/java/com/android/tools/r8/ir/analysis/type/NullabilityTest.java b/src/test/java/com/android/tools/r8/ir/analysis/type/NullabilityTest.java
index 0ea8731..c618dad 100644
--- a/src/test/java/com/android/tools/r8/ir/analysis/type/NullabilityTest.java
+++ b/src/test/java/com/android/tools/r8/ir/analysis/type/NullabilityTest.java
@@ -50,7 +50,7 @@
DexEncodedMethod foo = codeInspector.clazz(mainClass.getName()).method(signature).getMethod();
IRCode irCode =
foo.buildIR(appInfo, GraphLense.getIdentityLense(), TEST_OPTIONS, Origin.unknown());
- NonNullTracker nonNullTracker = new NonNullTracker();
+ NonNullTracker nonNullTracker = new NonNullTracker(appInfo);
nonNullTracker.addNonNull(irCode);
TypeAnalysis analysis = new TypeAnalysis(appInfo, foo);
analysis.widening(foo, irCode);
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/NonNullTrackerTest.java b/src/test/java/com/android/tools/r8/ir/optimize/NonNullTrackerTest.java
index 058f017..564a81c 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/NonNullTrackerTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/NonNullTrackerTest.java
@@ -41,7 +41,7 @@
foo.buildIR(appInfo, GraphLense.getIdentityLense(), TEST_OPTIONS, Origin.unknown());
checkCountOfNonNull(irCode, 0);
- NonNullTracker nonNullTracker = new NonNullTracker();
+ NonNullTracker nonNullTracker = new NonNullTracker(appInfo);
nonNullTracker.addNonNull(irCode);
assertTrue(irCode.isConsistentSSA());
