Also remove dead phis during trivial phi removal
This fixes a bug in the class inliner that manifests when the inliner creates a phi for the return value, which ends up not having any users.
Change-Id: Id9b0a7ec910b017c42eccae16fe7e18078e81690
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/constant/SparseConditionalConstantPropagation.java b/src/main/java/com/android/tools/r8/ir/analysis/constant/SparseConditionalConstantPropagation.java
index 72ac893..0bb8f21 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/constant/SparseConditionalConstantPropagation.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/constant/SparseConditionalConstantPropagation.java
@@ -117,7 +117,7 @@
block.deduplicatePhis();
}
- code.removeAllTrivialPhis();
+ code.removeAllDeadAndTrivialPhis();
}
private LatticeElement getLatticeElement(Value value) {
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 952bd5c..883d49f 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
@@ -1126,27 +1126,32 @@
return true;
}
- public boolean removeAllTrivialPhis() {
- return removeAllTrivialPhis(null, null);
+ public boolean removeAllDeadAndTrivialPhis() {
+ return removeAllDeadAndTrivialPhis(null, null);
}
- public boolean removeAllTrivialPhis(IRBuilder builder) {
- return removeAllTrivialPhis(builder, null);
+ public boolean removeAllDeadAndTrivialPhis(IRBuilder builder) {
+ return removeAllDeadAndTrivialPhis(builder, null);
}
- public boolean removeAllTrivialPhis(Set<Value> affectedValues) {
- return removeAllTrivialPhis(null, affectedValues);
+ public boolean removeAllDeadAndTrivialPhis(Set<Value> affectedValues) {
+ return removeAllDeadAndTrivialPhis(null, affectedValues);
}
- public boolean removeAllTrivialPhis(IRBuilder builder, Set<Value> affectedValues) {
- boolean anyTrivialPhisRemoved = false;
+ public boolean removeAllDeadAndTrivialPhis(IRBuilder builder, Set<Value> affectedValues) {
+ boolean anyPhisRemoved = false;
for (BasicBlock block : blocks) {
List<Phi> phis = new ArrayList<>(block.getPhis());
for (Phi phi : phis) {
- anyTrivialPhisRemoved |= phi.removeTrivialPhi(builder, affectedValues);
+ if (phi.hasAnyUsers()) {
+ anyPhisRemoved |= phi.removeTrivialPhi(builder, affectedValues);
+ } else {
+ phi.removeDeadPhi();
+ anyPhisRemoved = true;
+ }
}
}
- return anyTrivialPhisRemoved;
+ return anyPhisRemoved;
}
public int reserveMarkingColor() {
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
index 9598e95..e87f619 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
@@ -688,7 +688,7 @@
block.deduplicatePhis();
}
- ir.removeAllTrivialPhis(this);
+ ir.removeAllDeadAndTrivialPhis(this);
ir.removeUnreachableBlocks();
// Compute precise types for all values.
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/StringSwitchConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/StringSwitchConverter.java
index 41d1363..20ab05c 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/StringSwitchConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/StringSwitchConverter.java
@@ -113,7 +113,7 @@
}
}
if (changed) {
- code.removeAllTrivialPhis();
+ code.removeAllDeadAndTrivialPhis();
code.removeUnreachableBlocks();
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
index 60813ab..45befcf 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
@@ -208,7 +208,7 @@
// Therefore, Assume elimination may result in a trivial phi:
// z <- phi(x, x)
if (needToCheckTrivialPhis) {
- code.removeAllTrivialPhis(valuesThatRequireWidening);
+ code.removeAllDeadAndTrivialPhis(valuesThatRequireWidening);
}
if (!valuesThatRequireWidening.isEmpty()) {
@@ -1221,10 +1221,10 @@
assumeDynamicTypeRemover.removeMarkedInstructions(blocksToBeRemoved).finish();
if (!blocksToBeRemoved.isEmpty()) {
code.removeBlocks(blocksToBeRemoved);
- code.removeAllTrivialPhis(affectedValues);
+ code.removeAllDeadAndTrivialPhis(affectedValues);
assert code.getUnreachableBlocks().isEmpty();
} else if (mayHaveRemovedTrivialPhi || assumeDynamicTypeRemover.mayHaveIntroducedTrivialPhi()) {
- code.removeAllTrivialPhis(affectedValues);
+ code.removeAllDeadAndTrivialPhis(affectedValues);
}
if (!affectedValues.isEmpty()) {
new TypeAnalysis(appView).narrowing(affectedValues);
@@ -1299,7 +1299,7 @@
// Removing check-cast may result in a trivial phi:
// v3 <- phi(v1, v1)
if (needToRemoveTrivialPhis) {
- code.removeAllTrivialPhis(affectedValues);
+ code.removeAllDeadAndTrivialPhis(affectedValues);
if (!affectedValues.isEmpty()) {
typeAnalysis.narrowing(affectedValues);
}
@@ -2593,7 +2593,7 @@
}
if (changed) {
- code.removeAllTrivialPhis();
+ code.removeAllDeadAndTrivialPhis();
}
assert code.isConsistentSSA();
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java b/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
index 5160e6c..2dff525 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
@@ -257,7 +257,7 @@
shouldSimplifyIfs |= newConst.outValue().hasUserThatMatches(Instruction::isIf);
} while (iterator.hasNext());
- shouldSimplifyIfs |= code.removeAllTrivialPhis();
+ shouldSimplifyIfs |= code.removeAllDeadAndTrivialPhis();
if (shouldSimplifyIfs) {
codeRewriter.simplifyIf(code);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/IdempotentFunctionCallCanonicalizer.java b/src/main/java/com/android/tools/r8/ir/optimize/IdempotentFunctionCallCanonicalizer.java
index 9cbc694..ebf6e57 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/IdempotentFunctionCallCanonicalizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/IdempotentFunctionCallCanonicalizer.java
@@ -248,7 +248,7 @@
}
}
- code.removeAllTrivialPhis();
+ code.removeAllDeadAndTrivialPhis();
assert code.isConsistentSSA();
}
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 c9bf058..c9bc764 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
@@ -1058,7 +1058,7 @@
assumeDynamicTypeRemover.finish();
classInitializationAnalysis.finish();
code.removeBlocks(blocksToRemove);
- code.removeAllTrivialPhis();
+ code.removeAllDeadAndTrivialPhis();
assert code.isConsistentSSA();
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java b/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java
index 96f6bcb..8e0858a 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java
@@ -393,7 +393,7 @@
}
assumeDynamicTypeRemover.removeMarkedInstructions(blocksToBeRemoved).finish();
code.removeBlocks(blocksToBeRemoved);
- code.removeAllTrivialPhis(valuesToNarrow);
+ code.removeAllDeadAndTrivialPhis(valuesToNarrow);
code.removeUnreachableBlocks();
if (!valuesToNarrow.isEmpty()) {
new TypeAnalysis(appView).narrowing(valuesToNarrow);
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
index 6707fe1..9b443bc 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
@@ -270,7 +270,7 @@
// Restore normality.
Set<Value> affectedValues = Sets.newIdentityHashSet();
- code.removeAllTrivialPhis(affectedValues);
+ code.removeAllDeadAndTrivialPhis(affectedValues);
if (!affectedValues.isEmpty()) {
new TypeAnalysis(appView).narrowing(affectedValues);
}
diff --git a/src/test/java/com/android/tools/r8/internal/YouTubeV1419TreeShakeJarVerificationTest.java b/src/test/java/com/android/tools/r8/internal/YouTubeV1419TreeShakeJarVerificationTest.java
index 48acfdb..dd47f7f 100644
--- a/src/test/java/com/android/tools/r8/internal/YouTubeV1419TreeShakeJarVerificationTest.java
+++ b/src/test/java/com/android/tools/r8/internal/YouTubeV1419TreeShakeJarVerificationTest.java
@@ -5,6 +5,7 @@
import static com.android.tools.r8.ToolHelper.isLocalDevelopment;
import static com.android.tools.r8.ToolHelper.shouldRunSlowTests;
+import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeTrue;
@@ -46,8 +47,12 @@
R8TestCompileResult compileResult =
testForR8(parameters.getBackend())
.addKeepRuleFiles(getKeepRuleFiles())
+ .allowDiagnosticMessages()
.allowUnusedProguardConfigurationRules()
- .compile();
+ .compile()
+ .assertAllInfoMessagesMatch(
+ containsString("Proguard configuration rule does not match anything"))
+ .assertAllWarningMessagesMatch(containsString("Ignoring option:"));
if (ToolHelper.isLocalDevelopment()) {
DexItemFactory dexItemFactory = new DexItemFactory();
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/UnusedReturnValueTest.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/UnusedReturnValueTest.java
index c46ae1b..2a2bb9e 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/UnusedReturnValueTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/UnusedReturnValueTest.java
@@ -31,8 +31,6 @@
testForR8(parameters.getBackend())
.addInnerClasses(UnusedReturnValueTest.class)
.addKeepMainRule(TestClass.class)
- // TODO(christofferqa): Fix class inliner.
- .allowClassInlinerGracefulExit()
.enableInliningAnnotations()
.setMinApi(parameters.getApiLevel())
.compile()
