[llvm] [JumpThreading] Materialize PHIs in duplicateCondBranchOnPHIIntoPred (PR #204859)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 23 09:16:04 PDT 2026
https://github.com/lijinpei-amd updated https://github.com/llvm/llvm-project/pull/204859
>From 30e336c356b9c3211d32151135ee552575ac44be Mon Sep 17 00:00:00 2001
From: Li Jinpei <jinpli at amd.com>
Date: Sat, 20 Jun 2026 00:42:18 +0800
Subject: [PATCH 1/2] [JumpThreading] Pre-commit tests for branch-on-phi
duplication miscompile
Add tests reduced from #197725 and #203868. The following commit fixes the
miscompile and updates the checks.
Assisted-by: Opus-4.8 (Claude Code)
---
.../dup-cond-br-recursive-phi.ll | 29 +++++++++++++++++++
.../JumpThreading/phi-copy-to-pred.ll | 22 ++++++++++++++
2 files changed, 51 insertions(+)
create mode 100644 llvm/test/Transforms/JumpThreading/dup-cond-br-recursive-phi.ll
diff --git a/llvm/test/Transforms/JumpThreading/dup-cond-br-recursive-phi.ll b/llvm/test/Transforms/JumpThreading/dup-cond-br-recursive-phi.ll
new file mode 100644
index 0000000000000..2eb1e3a5e3595
--- /dev/null
+++ b/llvm/test/Transforms/JumpThreading/dup-cond-br-recursive-phi.ll
@@ -0,0 +1,29 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -passes=jump-threading -jump-threading-across-loop-headers=true -S < %s | FileCheck %s
+
+; origin: https://github.com/llvm/llvm-project/issues/203868
+define i64 @PR203868(i64 %g28, i1 %tobool, i1 %c.next, i64 %arr.init, i64 %ov.init) {
+; CHECK-LABEL: @PR203868(
+; CHECK-NEXT: br6:
+; CHECK-NEXT: br i1 [[TOBOOL:%.*]], label [[BF7:%.*]], label [[EXIT:%.*]]
+; CHECK: bf7:
+; CHECK-NEXT: [[OV_02:%.*]] = phi i64 [ [[OV_NEXT:%.*]], [[BF7]] ], [ [[G28:%.*]], [[BR6:%.*]] ]
+; CHECK-NEXT: [[OV_NEXT]] = add i64 [[OV_02]], [[OV_INIT:%.*]]
+; CHECK-NEXT: br i1 [[C_NEXT:%.*]], label [[BF7]], label [[EXIT]]
+; CHECK: exit:
+; CHECK-NEXT: [[ARR1:%.*]] = phi i64 [ [[OV_NEXT]], [[BF7]] ], [ [[ARR_INIT:%.*]], [[BR6]] ]
+; CHECK-NEXT: ret i64 [[ARR1]]
+;
+entry:
+ br label %br6
+br6:
+ %arr = phi i64 [ %ov.0, %bf7 ], [ %arr.init, %entry ]
+ %c.0 = phi i1 [ %c.next, %bf7 ], [ %tobool, %entry ]
+ %ov.0 = phi i64 [ %ov.next, %bf7 ], [ %g28, %entry ]
+ br i1 %c.0, label %bf7, label %exit
+bf7:
+ %ov.next = add i64 %ov.0, %ov.init
+ br label %br6
+exit:
+ ret i64 %arr
+}
diff --git a/llvm/test/Transforms/JumpThreading/phi-copy-to-pred.ll b/llvm/test/Transforms/JumpThreading/phi-copy-to-pred.ll
index 17f3b1a3c4c2d..da9d86eac8073 100644
--- a/llvm/test/Transforms/JumpThreading/phi-copy-to-pred.ll
+++ b/llvm/test/Transforms/JumpThreading/phi-copy-to-pred.ll
@@ -67,3 +67,25 @@ EXIT1:
EXIT2:
ret i32 1
}
+
+; origin: https://github.com/llvm/llvm-project/issues/197725
+define i32 @PR197725() {
+; CHECK-LABEL: @PR197725(
+; CHECK-NEXT: exit:
+; CHECK-NEXT: [[NOT:%.*]] = xor i32 0, 1
+; CHECK-NEXT: ret i32 0
+;
+entry:
+ br i1 false, label %loop2, label %loop1
+loop1:
+ %cond = phi i1 [ %tobool, %loop2 ], [ false, %entry ]
+ %val = phi i32 [ %not_phi, %loop2 ], [ 0, %entry ]
+ %not = xor i32 %val, 1
+ br i1 %cond, label %exit, label %loop2
+exit:
+ ret i32 %val
+loop2:
+ %not_phi = phi i32 [ %not, %loop1 ], [ 0, %entry ]
+ %tobool = icmp ne i8 1, 0
+ br label %loop1
+}
>From 69c7ce95f4f4c1357482d305e8dc3b489a40c713 Mon Sep 17 00:00:00 2001
From: Li Jinpei <jinpli at amd.com>
Date: Sat, 20 Jun 2026 00:42:34 +0800
Subject: [PATCH 2/2] [JumpThreading] Materialize PHIs in
duplicateCondBranchOnPHIIntoPred
In duplicateCondBranchOnPHIIntoPred, updateSSA iteratively updates the uses of
the instructions of BB (the duplicated block) according to ValueMapping. For
PHIs, however, the mapping is inconsistent: the keys refer to the values before
the parallel assignment of the PHIs, while the mapped-to values refer to the
values after it. E.g.
BB:
%arr = phi [ %ov.0, %PredBB ], ...
%ov.0 = phi [ %ov.sel, %PredBB ], ...
--->
%arr => %ov.0
%ov.0 => %ov.sel
So an iterative replacement miscompiles: a use of the duplicated %arr is
replaced by %ov.sel, while the correct replacement is %ov.0.
Fix this by splitting PredBB -> BB (SplitEdge) into a PredEdgeBB, cloning the
PHIs into PredEdgeBB and mapping BB's PHIs to the clones:
BB:
%arr = phi [ %ov.0, %PredEdgeBB ], ...
%ov.0 = phi [ %ov.sel, %PredEdgeBB ], ...
PredEdgeBB:
%arr.dup = phi [ %ov.0, %PredBB ]
%ov.0.dup = phi [ %ov.sel, %PredBB ]
--->
%arr => %arr.dup
%ov.0 => %ov.0.dup
Since a distinct set of PHIs is used as the mapped-to values, the iterative
replacement is correct.
Fixes #197725.
Fixes #203868.
Assisted-by: Opus-4.8 (Claude Code)
---
llvm/lib/Transforms/Scalar/JumpThreading.cpp | 29 ++++++++++++-------
.../dup-cond-br-recursive-phi.ll | 2 +-
.../JumpThreading/phi-copy-to-pred.ll | 3 +-
3 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/JumpThreading.cpp b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
index ce01f99982be7..7a0542aac83fc 100644
--- a/llvm/lib/Transforms/Scalar/JumpThreading.cpp
+++ b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
@@ -2690,16 +2690,16 @@ bool JumpThreadingPass::duplicateCondBranchOnPHIIntoPred(
<< "' to eliminate branch on phi. Cost: "
<< DuplicationCost << " block is:" << *BB << "\n");
- // Unless PredBB ends with an unconditional branch, split the edge so that we
- // can just clone the bits from BB into the end of the new PredBB.
+ // When BB contains PHIs, we need a dedicated PredBB to clone these PHIs into,
+ // so split the PredBB -> BB edge to create one. Otherwise fall back to
+ // cloning into PredBB directly, splitting only when it lacks an unconditional
+ // branch.
+ BasicBlock *OldPredBB = PredBB;
UncondBrInst *OldPredBranch = dyn_cast<UncondBrInst>(PredBB->getTerminator());
-
- if (!OldPredBranch) {
- BasicBlock *OldPredBB = PredBB;
+ if (isa<PHINode>(BB->front()) || !OldPredBranch) {
PredBB = SplitEdge(OldPredBB, BB);
Updates.push_back({DominatorTree::Insert, OldPredBB, PredBB});
Updates.push_back({DominatorTree::Insert, PredBB, BB});
- Updates.push_back({DominatorTree::Delete, OldPredBB, BB});
OldPredBranch = cast<UncondBrInst>(PredBB->getTerminator());
}
@@ -2711,8 +2711,12 @@ bool JumpThreadingPass::duplicateCondBranchOnPHIIntoPred(
auto RItBeforeInsertPt = std::next(OldPredBranch->getReverseIterator());
BasicBlock::iterator BI = BB->begin();
- for (; PHINode *PN = dyn_cast<PHINode>(BI); ++BI)
- ValueMapping[PN] = PN->getIncomingValueForBlock(PredBB);
+ for (; PHINode *PN = dyn_cast<PHINode>(BI); ++BI) {
+ PHINode *NewPN = PHINode::Create(PN->getType(), 1, PN->getName() + ".dup");
+ NewPN->insertBefore(OldPredBranch->getIterator());
+ NewPN->addIncoming(PN->getIncomingValueForBlock(PredBB), OldPredBB);
+ ValueMapping[PN] = NewPN;
+ }
// Clone noalias scope declarations in the duplicated instructions. Otherwise
// the duplicate would share the original block's scopes, and alias analysis
@@ -2792,10 +2796,15 @@ bool JumpThreadingPass::duplicateCondBranchOnPHIIntoPred(
// Remove the unconditional branch at the end of the PredBB block.
OldPredBranch->eraseFromParent();
- if (auto *BPI = getBPI())
- BPI->copyEdgeProbabilities(BB, PredBB);
DTU->applyUpdatesPermissive(Updates);
+ BasicBlock *ThreadBB = PredBB;
+ if (PredBB != OldPredBB && MergeBlockIntoPredecessor(PredBB, DTU.get()))
+ ThreadBB = OldPredBB;
+
+ if (auto *BPI = getBPI())
+ BPI->copyEdgeProbabilities(BB, ThreadBB);
+
++NumDupes;
return true;
}
diff --git a/llvm/test/Transforms/JumpThreading/dup-cond-br-recursive-phi.ll b/llvm/test/Transforms/JumpThreading/dup-cond-br-recursive-phi.ll
index 2eb1e3a5e3595..230c0353a7b06 100644
--- a/llvm/test/Transforms/JumpThreading/dup-cond-br-recursive-phi.ll
+++ b/llvm/test/Transforms/JumpThreading/dup-cond-br-recursive-phi.ll
@@ -11,7 +11,7 @@ define i64 @PR203868(i64 %g28, i1 %tobool, i1 %c.next, i64 %arr.init, i64 %ov.in
; CHECK-NEXT: [[OV_NEXT]] = add i64 [[OV_02]], [[OV_INIT:%.*]]
; CHECK-NEXT: br i1 [[C_NEXT:%.*]], label [[BF7]], label [[EXIT]]
; CHECK: exit:
-; CHECK-NEXT: [[ARR1:%.*]] = phi i64 [ [[OV_NEXT]], [[BF7]] ], [ [[ARR_INIT:%.*]], [[BR6]] ]
+; CHECK-NEXT: [[ARR1:%.*]] = phi i64 [ [[OV_02]], [[BF7]] ], [ [[ARR_INIT:%.*]], [[BR6]] ]
; CHECK-NEXT: ret i64 [[ARR1]]
;
entry:
diff --git a/llvm/test/Transforms/JumpThreading/phi-copy-to-pred.ll b/llvm/test/Transforms/JumpThreading/phi-copy-to-pred.ll
index da9d86eac8073..b8a3199a31e0a 100644
--- a/llvm/test/Transforms/JumpThreading/phi-copy-to-pred.ll
+++ b/llvm/test/Transforms/JumpThreading/phi-copy-to-pred.ll
@@ -73,7 +73,8 @@ define i32 @PR197725() {
; CHECK-LABEL: @PR197725(
; CHECK-NEXT: exit:
; CHECK-NEXT: [[NOT:%.*]] = xor i32 0, 1
-; CHECK-NEXT: ret i32 0
+; CHECK-NEXT: [[NOT1:%.*]] = xor i32 [[NOT]], 1
+; CHECK-NEXT: ret i32 [[NOT]]
;
entry:
br i1 false, label %loop2, label %loop1
More information about the llvm-commits
mailing list