[llvm] [SimplifyCFG] Do not convert branch to select for loop carried phi nodes (PR #227242)
Shreeyash Pandey via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 29 04:20:19 PDT 2026
https://github.com/bojle updated https://github.com/llvm/llvm-project/pull/227242
>From c5a0d85c1ef998c151b1b30aa69e824092ef458d Mon Sep 17 00:00:00 2001
From: Shreeyash Pandey <shrpand at qti.qualcomm.com>
Date: Mon, 28 Sep 2026 10:47:58 +0000
Subject: [PATCH 1/4] [SimplifyCFG] Do not convert branch to select for loop
carried phi nodes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Fixes https://github.com/llvm/llvm-project/issues/222923
The patch adds a guard to SimplifyCFG’s speculativelyExecuteBB()
transformation. It recognizes a narrow diamond in which an effectively empty
branch arm controls a PHI update, and that PHI is carried into a loop-header
PHI or directly into the next loop-bound comparison.
In this form, converting the branch to a select results into a data dependency
which can potentially limit ILP.
SimplifyCFG’s existing speculation model does not perform a full
branch-versus-select analysis that accounts for branch probabilities, loop
recurrence latency, target-specific select lowering, and downstream CFG
optimizations. A later target-aware pass such as SelectOptimize can make a more
informed decision, but by then the original CFG structure may already have been
lost. Additionally, SelectOptimize only runs at O3, which causes poor-codegen
at O2 due to SimplifyCFG's short-sighted conversion.
The issue was discovered in mcf (-O2) SPEC 2017.
With this patch, these are the perf numbers across the benchmark (-O2):
+------------------+--------------+
| Benchmark | Percent Gain |
+------------------+--------------+
| 500.perlbench_r | -1.48% |
| 502.gcc_r | -0.33% |
| 505.mcf_r | +8.78% |
| 520.omnetpp_r | -1.08% |
| 523.xalancbmk_r | +0.33% |
| 525.x264_r | +1.27% |
| 531.deepsjeng_r | -0.18% |
| 541.leela_r | +2.33% |
| 557.xz_r | -0.45% |
+------------------+--------------+
Signed-off-by: Shreeyash Pandey <shrpand at qti.qualcomm.com>
---
llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 83 +++++++++++++++++++
.../SimplifyCFG/loop-carried-select.ll | 58 +++++++++++++
2 files changed, 141 insertions(+)
create mode 100644 llvm/test/Transforms/SimplifyCFG/loop-carried-select.ll
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 2f9a4819f9984..49982945ac439 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -3119,6 +3119,82 @@ static Value *isSafeToSpeculateStore(Instruction *I, BasicBlock *BrBB,
return nullptr;
}
+/// Return true for the narrow loop-carried PHI shape where converting the
+/// branch to a select would put the selected value directly on the next
+/// loop-header branch condition.
+static bool shouldVetoLoopCarriedSelect(
+ BasicBlock *BB, BasicBlock *ThenBB, BasicBlock *EndBB,
+ ArrayRef<WeakVH> LoopHeaders) {
+ auto *BI = dyn_cast<CondBrInst>(BB->getTerminator());
+ if (!BI || !isa<ICmpInst>(BI->getCondition()))
+ return false;
+
+ // Only handle the empty-arm diamond that speculativelyExecuteBB would
+ // otherwise flatten.
+ if (ThenBB->getSinglePredecessor() != BB ||
+ ThenBB->getSingleSuccessor() != EndBB)
+ return false;
+ for (const Instruction &I : *ThenBB)
+ if (!I.isDebugOrPseudoInst() && !I.isTerminator())
+ return false;
+
+ // Usually the merge block feeds a separate loop header. A previous CFG
+ // simplification can instead make the merge block itself the loop header;
+ // in that form its PHI is used directly by its own terminator. It can also
+ // remain a latch-like merge block whose conditional terminator feeds a loop
+ // header. Handle all of these forms so this protection remains effective
+ // across repeated SimplifyCFG invocations, even when LoopHeaders was built
+ // before the CFG was reshaped.
+ BasicBlock *Header = nullptr;
+ auto IsLoopHeader = [&LoopHeaders](BasicBlock *Block) {
+ return is_contained(LoopHeaders, WeakVH(Block));
+ };
+ auto *EndBI = dyn_cast<CondBrInst>(EndBB->getTerminator());
+ bool UseEndBBCondition = IsLoopHeader(EndBB);
+ if (!UseEndBBCondition && EndBI)
+ UseEndBBCondition = any_of(EndBI->successors(), IsLoopHeader);
+ if (UseEndBBCondition) {
+ Header = EndBB;
+ } else {
+ Header = EndBB->getSingleSuccessor();
+ if (!Header || !IsLoopHeader(Header))
+ return false;
+ }
+
+ auto *HeaderBI = dyn_cast<CondBrInst>(Header->getTerminator());
+ if (!HeaderBI || !isa<ICmpInst>(HeaderBI->getCondition()))
+ return false;
+ auto *HeaderCond = cast<Instruction>(HeaderBI->getCondition());
+
+ // Look for a nontrivial merge PHI feeding a loop-header PHI whose value
+ // is directly used by the loop-header comparison. Inspect the current
+ // merge block here instead of relying on a PHI list collected by
+ // validateAndCostRequiredSelects
+ for (PHINode &PN : EndBB->phis()) {
+ Value *BBValue = PN.getIncomingValueForBlock(BB);
+ Value *ThenValue = PN.getIncomingValueForBlock(ThenBB);
+ if (!BBValue || !ThenValue || BBValue == ThenValue)
+ continue;
+
+ if (UseEndBBCondition) {
+ if (any_of(HeaderCond->operands(),
+ [&PN](Value *Op) { return Op == &PN; }))
+ return true;
+ continue;
+ }
+
+ for (PHINode &HeaderPN : Header->phis()) {
+ if (HeaderPN.getIncomingValueForBlock(EndBB) != &PN)
+ continue;
+
+ if (any_of(HeaderCond->operands(),
+ [&HeaderPN](Value *Op) { return Op == &HeaderPN; }))
+ return true;
+ }
+ }
+
+ return false;
+}
/// Estimate the cost of the insertion(s) and check that the PHI nodes can be
/// converted to selects.
static bool validateAndCostRequiredSelects(BasicBlock *BB, BasicBlock *ThenBB,
@@ -3246,6 +3322,13 @@ bool SimplifyCFGOpt::speculativelyExecuteBB(CondBrInst *BI,
BasicBlock *BB = BI->getParent();
BasicBlock *EndBB = ThenBB->getTerminator()->getSuccessor(0);
+
+ if (shouldVetoLoopCarriedSelect(BB, ThenBB, EndBB, LoopHeaders)) {
+ LLVM_DEBUG(dbgs() << "vetoing speculative execution of loop-carried "
+ << "branch in " << BB->getName() << "\n");
+ return false;
+ }
+
InstructionCost Budget =
PHINodeFoldingThreshold * TargetTransformInfo::TCC_Basic;
diff --git a/llvm/test/Transforms/SimplifyCFG/loop-carried-select.ll b/llvm/test/Transforms/SimplifyCFG/loop-carried-select.ll
new file mode 100644
index 0000000000000..e10dd6f06dec1
--- /dev/null
+++ b/llvm/test/Transforms/SimplifyCFG/loop-carried-select.ll
@@ -0,0 +1,58 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6
+; RUN: opt -S -passes='simplifycfg' < %s | FileCheck %s
+define void @loop_carried_select(ptr %p, ptr %q, i64 %max) {
+; CHECK-LABEL: define void @loop_carried_select(
+; CHECK-SAME: ptr [[P:%.*]], ptr [[Q:%.*]], i64 [[MAX:%.*]]) {
+; CHECK-NEXT: [[ENTRY:.*]]:
+; CHECK-NEXT: br label %[[LOOP:.*]]
+; CHECK: [[LOOP]]:
+; CHECK-NEXT: [[IV:%.*]] = phi i64 [ 1, %[[ENTRY]] ], [ [[NEXT:%.*]], %[[MERGE:.*]] ]
+; CHECK-NEXT: [[DONE:%.*]] = icmp sgt i64 [[IV]], [[MAX]]
+; CHECK-NEXT: br i1 [[DONE]], label %[[EXIT:.*]], label %[[BODY:.*]]
+; CHECK: [[BODY]]:
+; CHECK-NEXT: [[DOUBLED:%.*]] = shl i64 [[IV]], 1
+; CHECK-NEXT: [[UPDATED:%.*]] = add i64 [[DOUBLED]], 1
+; CHECK-NEXT: [[IN_BOUNDS:%.*]] = icmp slt i64 [[DOUBLED]], [[MAX]]
+; CHECK-NEXT: br i1 [[IN_BOUNDS]], label %[[CHECK:.*]], label %[[MERGE]]
+; CHECK: [[CHECK]]:
+; CHECK-NEXT: [[A:%.*]] = load i64, ptr [[P]], align 4
+; CHECK-NEXT: [[B:%.*]] = load i64, ptr [[Q]], align 4
+; CHECK-NEXT: [[LESS:%.*]] = icmp slt i64 [[A]], [[B]]
+; CHECK-NEXT: [[SPEC_SELECT:%.*]] = select i1 [[LESS]], i64 [[UPDATED]], i64 [[DOUBLED]]
+; CHECK-NEXT: br label %[[MERGE]]
+; CHECK: [[MERGE]]:
+; CHECK-NEXT: [[NEXT]] = phi i64 [ [[DOUBLED]], %[[BODY]] ], [ [[SPEC_SELECT]], %[[CHECK]] ]
+; CHECK-NEXT: br label %[[LOOP]]
+; CHECK: [[EXIT]]:
+; CHECK-NEXT: ret void
+;
+entry:
+ br label %loop
+
+loop:
+ %iv = phi i64 [ 1, %entry ], [ %next, %merge ]
+ %done = icmp sgt i64 %iv, %max
+ br i1 %done, label %exit, label %body
+
+body:
+ %doubled = shl i64 %iv, 1
+ %updated = add i64 %doubled, 1
+ %in_bounds = icmp slt i64 %doubled, %max
+ br i1 %in_bounds, label %check, label %merge
+
+check:
+ %a = load i64, ptr %p
+ %b = load i64, ptr %q
+ %less = icmp slt i64 %a, %b
+ br i1 %less, label %empty, label %merge
+
+empty:
+ br label %merge
+
+merge:
+ %next = phi i64 [ %updated, %empty ], [ %doubled, %check ], [ %doubled, %body ]
+ br label %loop
+
+exit:
+ ret void
+}
>From 895c7ddf1068bc62eccdd5b51160bb6444811f89 Mon Sep 17 00:00:00 2001
From: Shreeyash Pandey <shrpand at qti.qualcomm.com>
Date: Tue, 29 Sep 2026 09:27:10 +0000
Subject: [PATCH 2/4] modify test with patch
Signed-off-by: Shreeyash Pandey <shrpand at qti.qualcomm.com>
---
llvm/test/Transforms/SimplifyCFG/loop-carried-select.ll | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/llvm/test/Transforms/SimplifyCFG/loop-carried-select.ll b/llvm/test/Transforms/SimplifyCFG/loop-carried-select.ll
index e10dd6f06dec1..b76dc4aa18a38 100644
--- a/llvm/test/Transforms/SimplifyCFG/loop-carried-select.ll
+++ b/llvm/test/Transforms/SimplifyCFG/loop-carried-select.ll
@@ -18,10 +18,11 @@ define void @loop_carried_select(ptr %p, ptr %q, i64 %max) {
; CHECK-NEXT: [[A:%.*]] = load i64, ptr [[P]], align 4
; CHECK-NEXT: [[B:%.*]] = load i64, ptr [[Q]], align 4
; CHECK-NEXT: [[LESS:%.*]] = icmp slt i64 [[A]], [[B]]
-; CHECK-NEXT: [[SPEC_SELECT:%.*]] = select i1 [[LESS]], i64 [[UPDATED]], i64 [[DOUBLED]]
+; CHECK-NEXT: br i1 [[LESS]], label %[[EMPTY:.*]], label %[[MERGE]]
+; CHECK: [[EMPTY]]:
; CHECK-NEXT: br label %[[MERGE]]
; CHECK: [[MERGE]]:
-; CHECK-NEXT: [[NEXT]] = phi i64 [ [[DOUBLED]], %[[BODY]] ], [ [[SPEC_SELECT]], %[[CHECK]] ]
+; CHECK-NEXT: [[NEXT]] = phi i64 [ [[UPDATED]], %[[EMPTY]] ], [ [[DOUBLED]], %[[CHECK]] ], [ [[DOUBLED]], %[[BODY]] ]
; CHECK-NEXT: br label %[[LOOP]]
; CHECK: [[EXIT]]:
; CHECK-NEXT: ret void
>From d8340a8d00f04b9edd5b940143c0ede5e4b570e3 Mon Sep 17 00:00:00 2001
From: Shreeyash Pandey <shrpand at qti.qualcomm.com>
Date: Tue, 29 Sep 2026 09:48:25 +0000
Subject: [PATCH 3/4] clang format
Signed-off-by: Shreeyash Pandey <shrpand at qti.qualcomm.com>
---
llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 49982945ac439..4f782bda7e424 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -3121,10 +3121,10 @@ static Value *isSafeToSpeculateStore(Instruction *I, BasicBlock *BrBB,
/// Return true for the narrow loop-carried PHI shape where converting the
/// branch to a select would put the selected value directly on the next
-/// loop-header branch condition.
-static bool shouldVetoLoopCarriedSelect(
- BasicBlock *BB, BasicBlock *ThenBB, BasicBlock *EndBB,
- ArrayRef<WeakVH> LoopHeaders) {
+/// loop-header branch condition.
+static bool shouldVetoLoopCarriedSelect(BasicBlock *BB, BasicBlock *ThenBB,
+ BasicBlock *EndBB,
+ ArrayRef<WeakVH> LoopHeaders) {
auto *BI = dyn_cast<CondBrInst>(BB->getTerminator());
if (!BI || !isa<ICmpInst>(BI->getCondition()))
return false;
>From 0b84dd51f84f113a67af81ddf1840ad090fdd1c6 Mon Sep 17 00:00:00 2001
From: Shreeyash Pandey <shrpand at qti.qualcomm.com>
Date: Tue, 29 Sep 2026 11:19:58 +0000
Subject: [PATCH 4/4] comments
Signed-off-by: Shreeyash Pandey <shrpand at qti.qualcomm.com>
---
llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 4f782bda7e424..40a8bcb1bb89c 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -3140,11 +3140,7 @@ static bool shouldVetoLoopCarriedSelect(BasicBlock *BB, BasicBlock *ThenBB,
// Usually the merge block feeds a separate loop header. A previous CFG
// simplification can instead make the merge block itself the loop header;
- // in that form its PHI is used directly by its own terminator. It can also
- // remain a latch-like merge block whose conditional terminator feeds a loop
- // header. Handle all of these forms so this protection remains effective
- // across repeated SimplifyCFG invocations, even when LoopHeaders was built
- // before the CFG was reshaped.
+ // in that form its PHI is used directly by its own terminator.
BasicBlock *Header = nullptr;
auto IsLoopHeader = [&LoopHeaders](BasicBlock *Block) {
return is_contained(LoopHeaders, WeakVH(Block));
More information about the llvm-commits
mailing list