Always check for phis used outside loop in loop unroller
Fixes: b/341618078
Change-Id: Ic38311603904eda3e13cd2fd23c990bd0749d39b
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/passes/NaturalIntLoopRemover.java b/src/main/java/com/android/tools/r8/ir/conversion/passes/NaturalIntLoopRemover.java
index 5d078fe..a52a144 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/passes/NaturalIntLoopRemover.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/passes/NaturalIntLoopRemover.java
@@ -18,7 +18,6 @@
import com.android.tools.r8.ir.conversion.passes.result.CodeRewriterResult;
import com.android.tools.r8.ir.optimize.AffectedValues;
import com.android.tools.r8.utils.WorkList;
-import com.google.common.collect.Sets;
import java.util.Set;
/**
@@ -108,7 +107,7 @@
if (!analyzeLoopExit(loopBody, comparison, builder)) {
return false;
}
- if (!analyzePhiUses(loopBody, comparison, builder)) {
+ if (!analyzePhiUses(loopBody, comparison)) {
return false;
}
@@ -122,25 +121,10 @@
}
/**
- * The loop unroller removes phis corresponding to the loop backjump. There are three scenarios:
- * (1) The loop has a single exit point analyzed, phis used outside the loop are replaced by the
- * value at the end of the loop body.
- * (2) The phis are unused outside the loop, and they are simply removed.
- * (3) The loop has multiple exits and the phis are used outside the loop, this would require
- * dealing with complex merge point and postponing phis after the loop, we bail out.
+ * The loop unroller removes phis corresponding to the loop backjump if they are not used outside
+ * the loop.
*/
- private boolean analyzePhiUses(
- Set<BasicBlock> loopBody, If comparison, NaturalIntLoopWithKnowIterations.Builder builder) {
- // Check for single exit scenario.
- Set<BasicBlock> successors = Sets.newIdentityHashSet();
- for (BasicBlock basicBlock : loopBody) {
- successors.addAll(basicBlock.getSuccessors());
- }
- successors.removeAll(loopBody);
- if (successors.size() == 1) {
- assert successors.iterator().next() == builder.getLoopExit();
- return true;
- }
+ private boolean analyzePhiUses(Set<BasicBlock> loopBody, If comparison) {
// Check phis are unused outside the loop.
for (Phi phi : comparison.getBlock().getPhis()) {
for (Instruction use : phi.uniqueUsers()) {
diff --git a/src/test/java/com/android/tools/r8/regress/B341618078Test.java b/src/test/java/com/android/tools/r8/ir/optimize/loops/B341618078Test.java
similarity index 94%
rename from src/test/java/com/android/tools/r8/regress/B341618078Test.java
rename to src/test/java/com/android/tools/r8/ir/optimize/loops/B341618078Test.java
index 640eb4d..85d5d98 100644
--- a/src/test/java/com/android/tools/r8/regress/B341618078Test.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/loops/B341618078Test.java
@@ -1,7 +1,7 @@
// Copyright (c) 2024, the R8 project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
-package com.android.tools.r8.regress;
+package com.android.tools.r8.ir.optimize.loops;
import static org.junit.Assume.assumeTrue;
@@ -54,8 +54,7 @@
.addKeepMainRule(TestClass.class)
.setMinApi(parameters)
.run(parameters.getRuntime(), TestClass.class)
- // TODO(b/341618078): Should be EXPECTED_OUTPUT.
- .assertSuccessWithOutputLines("1", "1");
+ .assertSuccessWithOutput(EXPECTED_OUTPUT);
}
static class TestClass {
