[llvm] [LoopInterchange] Bail out if a PHI would be cloned into the new latch (PR #212742)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 29 04:59:06 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-transforms
Author: Ryotaro Kasuga (kasuga-fj)
<details>
<summary>Changes</summary>
The transformation stage of LoopInterchange splits the inner loop latch and clones the necessary instructions from the original latch into the new one. PHI nodes cannot be cloned this way, so the legality check is supposed to reject cases where such a clone would happen. However, it missed some cases, allowing a PHI node to be cloned and invalid IR to be produced.
This patch fixes the issue by making the legality check follow all the instruction trees the transformation would clone, i.e., the use-def chains of both the exit condition and the induction variable updates, and reject the interchange if a PHI node other than the induction PHIs appears in them. Previously, the check only ran when the inner loop contains subloops, only followed the use-def chains of the exit condition, and only rejected PHI nodes placed in the latch block.
Note that this is a stop-gap solution, especially because the legality check strongly depends on the details of the transformation stage. The ideal solution would be to enhance the transformation so that it can handle any PHI node correctly.
Fixes #<!-- -->210071
---
Full diff: https://github.com/llvm/llvm-project/pull/212742.diff
2 Files Affected:
- (modified) llvm/lib/Transforms/Scalar/LoopInterchange.cpp (+26-30)
- (added) llvm/test/Transforms/LoopInterchange/inner-latch-defs-iv.ll (+58)
``````````diff
diff --git a/llvm/lib/Transforms/Scalar/LoopInterchange.cpp b/llvm/lib/Transforms/Scalar/LoopInterchange.cpp
index c79bf04df656a..d4b8859669051 100644
--- a/llvm/lib/Transforms/Scalar/LoopInterchange.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopInterchange.cpp
@@ -1481,41 +1481,37 @@ static bool areOuterLoopExitPHIsSupported(Loop *OuterLoop, Loop *InnerLoop) {
return true;
}
-/// The transform clones the inner latch's exit condition into the new latch
-/// (see MoveInstructions in LoopInterchangeTransform::transform), but it does
-/// not relocate PHI nodes. So if a PHI in the inner latch feeds that condition,
-/// a later interchange can leave the cloned PHI with a stale incoming block,
-/// producing invalid IR. Reject that case here.
+/// The transform partially clones the inner loop's latch block, but PHI nodes
+/// cannot be cloned this way. This function follows the instruction trees that
+/// would be cloned and checks whether any PHI node other than the induction
+/// PHIs feeds them. If such a PHI is found, the interchange is rejected.
///
-/// For example, %p is a PHI in the inner latch and the inner loop's exit test
-/// reads %p, so %p feeds the condition that would be cloned:
-///
-/// inner.latch:
-/// %p = phi i64 [ %v, %subloop.latch ]
-/// %ec = icmp eq i64 %iv, %p ; inner exit test reads %p
-/// br i1 %ec, label %exit, label %inner.header
-///
-/// TODO: Handle transformation of lcssa phis in the InnerLoop latch in case of
-/// multi-level loop nests.
-static bool areInnerLoopLatchPHIsSupported(Loop *InnerLoop) {
- if (InnerLoop->getSubLoops().empty())
- return true;
-
+/// TODO: This check strongly depends on the current implementation of the
+/// transform. Ideally, the transform should be able to handle such PHI nodes in
+/// the inner loop latch.
+static bool areInnerLoopLatchPHIsSupported(Loop *InnerLoop,
+ ArrayRef<PHINode *> InductionPHIs) {
BasicBlock *InnerLoopLatch = InnerLoop->getLoopLatch();
- auto *LatchBI = dyn_cast<CondBrInst>(InnerLoopLatch->getTerminator());
- if (!LatchBI)
- return true;
- auto *CondI = dyn_cast<Instruction>(LatchBI->getCondition());
- if (!CondI)
- return true;
- // Bail if a phi in the inner latch feeds the exit condition, walking operands
- // within the inner loop.
+ // Seed the worklist with the roots of the use-def chains the transform
+ // clones: the latch's exit condition and the incoming values of the induction
+ // PHIs from the latch.
SmallSetVector<Instruction *, 8> Worklist;
- Worklist.insert(CondI);
+ if (auto *LatchBI = dyn_cast<CondBrInst>(InnerLoopLatch->getTerminator()))
+ if (auto *CondI = dyn_cast<Instruction>(LatchBI->getCondition()))
+ Worklist.insert(CondI);
+ for (PHINode *InductionPHI : InductionPHIs) {
+ if (auto *IncomingI = dyn_cast<Instruction>(
+ InductionPHI->getIncomingValueForBlock(InnerLoopLatch)))
+ if (!is_contained(InductionPHIs, IncomingI))
+ Worklist.insert(IncomingI);
+ }
+
+ // Bail if a PHI node other than the induction PHIs feeds the cloned
+ // instructions, walking the operand trees within the inner loop.
for (unsigned I = 0; I < Worklist.size(); ++I) {
Instruction *Cur = Worklist[I];
- if (isa<PHINode>(Cur) && Cur->getParent() == InnerLoopLatch)
+ if (isa<PHINode>(Cur) && !is_contained(InductionPHIs, Cur))
return false;
for (Value *Op : Cur->operands())
if (auto *OpI = dyn_cast<Instruction>(Op))
@@ -1571,7 +1567,7 @@ bool LoopInterchangeLegality::canInterchangeLoops(unsigned InnerLoopId,
return false;
}
- if (!areInnerLoopLatchPHIsSupported(InnerLoop)) {
+ if (!areInnerLoopLatchPHIsSupported(InnerLoop, InnerLoopInductions)) {
LLVM_DEBUG(dbgs() << "Found unsupported PHI nodes in inner loop latch.\n");
ORE->emit([&]() {
return OptimizationRemarkMissed(DEBUG_TYPE, "UnsupportedInnerLatchPHI",
diff --git a/llvm/test/Transforms/LoopInterchange/inner-latch-defs-iv.ll b/llvm/test/Transforms/LoopInterchange/inner-latch-defs-iv.ll
new file mode 100644
index 0000000000000..80222abfd9379
--- /dev/null
+++ b/llvm/test/Transforms/LoopInterchange/inner-latch-defs-iv.ll
@@ -0,0 +1,58 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6
+; RUN: opt -passes=loop-interchange -loop-interchange-profitabilities=ignore -S %s | FileCheck %s
+
+; Ensure that induction variables defined in the inner latch don't cause
+; problems for loop interchange.
+
+define void @f(ptr %A) {
+; CHECK-LABEL: define void @f(
+; CHECK-SAME: ptr [[A:%.*]]) {
+; CHECK-NEXT: [[ENTRY:.*]]:
+; CHECK-NEXT: br label %[[OUTER_HEADER:.*]]
+; CHECK: [[OUTER_HEADER]]:
+; CHECK-NEXT: [[I:%.*]] = phi i64 [ 0, %[[ENTRY]] ], [ [[I_NEXT:%.*]], %[[OUTER_LATCH:.*]] ]
+; CHECK-NEXT: br label %[[INNER_HEADER:.*]]
+; CHECK: [[INNER_HEADER]]:
+; CHECK-NEXT: [[J:%.*]] = phi i64 [ 0, %[[OUTER_HEADER]] ], [ [[J_NEXT_PHI:%.*]], %[[INNER_LATCH:.*]] ]
+; CHECK-NEXT: [[J_NEXT:%.*]] = add i64 [[J]], 1
+; CHECK-NEXT: [[GEP:%.*]] = getelementptr [256 x i8], ptr [[A]], i64 [[J]], i64 [[I]]
+; CHECK-NEXT: store i8 0, ptr [[GEP]], align 1
+; CHECK-NEXT: br label %[[INNER_LATCH]]
+; CHECK: [[INNER_LATCH]]:
+; CHECK-NEXT: [[J_NEXT_PHI]] = phi i64 [ [[J_NEXT]], %[[INNER_HEADER]] ]
+; CHECK-NEXT: [[EC_J:%.*]] = icmp eq i64 [[J_NEXT]], 256
+; CHECK-NEXT: br i1 [[EC_J]], label %[[OUTER_LATCH]], label %[[INNER_HEADER]]
+; CHECK: [[OUTER_LATCH]]:
+; CHECK-NEXT: [[I_NEXT]] = add i64 [[I]], 1
+; CHECK-NEXT: [[EC_I:%.*]] = icmp eq i64 [[I_NEXT]], 256
+; CHECK-NEXT: br i1 [[EC_I]], label %[[EXIT:.*]], label %[[OUTER_HEADER]]
+; CHECK: [[EXIT]]:
+; CHECK-NEXT: ret void
+;
+entry:
+ br label %outer.header
+
+outer.header:
+ %i = phi i64 [ 0, %entry ], [ %i.next, %outer.latch ]
+ br label %inner.header
+
+inner.header:
+ %j = phi i64 [ 0, %outer.header ], [ %j.next.phi, %inner.latch ]
+ %j.next = add i64 %j, 1
+ %gep = getelementptr [256 x i8], ptr %A, i64 %j, i64 %i
+ store i8 0, ptr %gep
+ br label %inner.latch
+
+inner.latch:
+ %j.next.phi = phi i64 [ %j.next, %inner.header ]
+ %ec.j = icmp eq i64 %j.next, 256
+ br i1 %ec.j, label %outer.latch, label %inner.header
+
+outer.latch:
+ %i.next = add i64 %i, 1
+ %ec.i = icmp eq i64 %i.next, 256
+ br i1 %ec.i, label %exit, label %outer.header
+
+exit:
+ ret void
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/212742
More information about the llvm-commits
mailing list