[llvm] [SimplifyCFG] Do not redirect potential poison values from predecessors to PHI (PR #189966)
Gábor Spaits via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 6 14:03:49 PDT 2026
https://github.com/spaits updated https://github.com/llvm/llvm-project/pull/189966
>From 44745aeb7d9c77eca82abf17a14fd7e08b98bb5f Mon Sep 17 00:00:00 2001
From: Gabor Spaits <gaborspaits1 at gmail.com>
Date: Wed, 1 Apr 2026 16:21:51 +0200
Subject: [PATCH 1/2] [SimplifyCFG] Do not redirect potential poison values
from predecessors to PHI
My attempt to fix #189526 .
I have did some debugging in SimplifyCFG and found that the things may
go wrong when redirecting values from predecessors to the PHI node: https://alive2.llvm.org/ce/z/MRGLNx .
The issue is caused by replacing a `undef` with a potential poison
value.
Here is the code generated after the patch for the code snippet in the
issue: https://alive2.llvm.org/ce/z/p5Hv_y .
This PR may cause some regressions while fixing the issue!
---
llvm/lib/Transforms/Utils/Local.cpp | 26 ++++++++++-
.../ForwardSwitchConditionToPHI.ll | 43 +++++++++++++++++--
2 files changed, 63 insertions(+), 6 deletions(-)
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index 84c0989a7fe07..854e5d179f309 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -1084,7 +1084,7 @@ static bool introduceTooManyPhiEntries(BasicBlock *BB, BasicBlock *Succ) {
/// \param BBPreds The predecessors of BB.
/// \param PN The phi that we are updating.
/// \param CommonPred The common predecessor of BB and PN's BasicBlock
-static void redirectValuesFromPredecessorsToPhi(BasicBlock *BB,
+static bool redirectValuesFromPredecessorsToPhi(BasicBlock *BB,
const PredBlockVector &BBPreds,
PHINode *PN,
BasicBlock *CommonPred) {
@@ -1124,6 +1124,16 @@ static void redirectValuesFromPredecessorsToPhi(BasicBlock *BB,
Value *Selected =
selectIncomingValueForBlock(PredVal, PredBB, IncomingValues);
+ // It can cause an issue if there was a 'undef' assigned for the block
+ // before and we replace that block with a value that might be poison.
+ int BlockIdx = PN->getBasicBlockIndex(PredBB);
+ if (BlockIdx > 0 && isa<UndefValue>(PN->getIncomingValue(BlockIdx)) &&
+ (!isa<Constant>(Selected) ||
+ !isGuaranteedNotToBeUndefOrPoison(Selected))) {
+ PN->addIncoming(OldVal, BB);
+ return false;
+ }
+
// And add a new incoming value for this predecessor for the
// newly retargeted branch.
PN->addIncoming(Selected, PredBB);
@@ -1141,6 +1151,16 @@ static void redirectValuesFromPredecessorsToPhi(BasicBlock *BB,
Value *Selected =
selectIncomingValueForBlock(OldVal, PredBB, IncomingValues);
+ // It can cause an issue if there was a 'undef' assigned for the block
+ // before and we replace that block with a value that might be poison.
+ int BlockIdx = PN->getBasicBlockIndex(PredBB);
+ if (BlockIdx > 0 && isa<UndefValue>(PN->getIncomingValue(BlockIdx)) &&
+ (!isa<Constant>(Selected) ||
+ !isGuaranteedNotToBeUndefOrPoison(Selected))) {
+ PN->addIncoming(OldVal, BB);
+ return false;
+ }
+
// And add a new incoming value for this predecessor for the
// newly retargeted branch.
PN->addIncoming(Selected, PredBB);
@@ -1150,6 +1170,7 @@ static void redirectValuesFromPredecessorsToPhi(BasicBlock *BB,
}
replaceUndefValuesInPhi(PN, IncomingValues);
+ return true;
}
bool llvm::TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB,
@@ -1327,7 +1348,8 @@ bool llvm::TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB,
// Loop over all of the PHI nodes in the successor of BB.
for (BasicBlock::iterator I = Succ->begin(); isa<PHINode>(I); ++I) {
PHINode *PN = cast<PHINode>(I);
- redirectValuesFromPredecessorsToPhi(BB, BBPreds, PN, CommonPred);
+ if (!redirectValuesFromPredecessorsToPhi(BB, BBPreds, PN, CommonPred))
+ return false;
}
}
diff --git a/llvm/test/Transforms/SimplifyCFG/ForwardSwitchConditionToPHI.ll b/llvm/test/Transforms/SimplifyCFG/ForwardSwitchConditionToPHI.ll
index ea81e0e4fa48c..df4b44aa941a6 100644
--- a/llvm/test/Transforms/SimplifyCFG/ForwardSwitchConditionToPHI.ll
+++ b/llvm/test/Transforms/SimplifyCFG/ForwardSwitchConditionToPHI.ll
@@ -206,15 +206,19 @@ define { i64, i64 } @PR95919(i64 noundef %arg, i64 noundef %arg1) {
;
; FWD-LABEL: @PR95919(
; FWD-NEXT: bb:
-; FWD-NEXT: [[SWITCH:%.*]] = icmp ult i64 [[ARG1:%.*]], 2
-; FWD-NEXT: br i1 [[SWITCH]], label [[BB5:%.*]], label [[BB3:%.*]]
+; FWD-NEXT: switch i64 [[ARG1:%.*]], label [[BB3:%.*]] [
+; FWD-NEXT: i64 0, label [[BB5:%.*]]
+; FWD-NEXT: i64 1, label [[BB2:%.*]]
+; FWD-NEXT: ]
+; FWD: bb2:
+; FWD-NEXT: br label [[BB5]]
; FWD: bb3:
; FWD-NEXT: [[I:%.*]] = udiv i64 [[ARG:%.*]], [[ARG1]]
; FWD-NEXT: [[I4:%.*]] = shl nuw i64 [[I]], 1
; FWD-NEXT: br label [[BB5]]
; FWD: bb5:
-; FWD-NEXT: [[I6:%.*]] = phi i64 [ [[I4]], [[BB3]] ], [ [[ARG]], [[BB:%.*]] ]
-; FWD-NEXT: [[I7:%.*]] = phi i64 [ 1, [[BB3]] ], [ [[ARG1]], [[BB]] ]
+; FWD-NEXT: [[I6:%.*]] = phi i64 [ [[I4]], [[BB3]] ], [ undef, [[BB:%.*]] ], [ [[ARG]], [[BB2]] ]
+; FWD-NEXT: [[I7:%.*]] = phi i64 [ 1, [[BB3]] ], [ [[ARG1]], [[BB2]] ], [ [[ARG1]], [[BB]] ]
; FWD-NEXT: [[I8:%.*]] = insertvalue { i64, i64 } poison, i64 [[I7]], 0
; FWD-NEXT: [[I9:%.*]] = insertvalue { i64, i64 } [[I8]], i64 [[I6]], 1
; FWD-NEXT: ret { i64, i64 } [[I9]]
@@ -240,3 +244,34 @@ bb5: ; preds = %bb3, %bb2, %bb
%i9 = insertvalue { i64, i64 } %i8, i64 %i6, 1
ret { i64, i64 } %i9
}
+
+define i32 @NotPropagatingPoisonToUndef(i8 %cond, i32 %x) {
+; NO_FWD-LABEL: @NotPropagatingPoisonToUndef(
+; NO_FWD-NEXT: D:
+; NO_FWD-NEXT: [[COND1:%.*]] = icmp eq i8 [[COND:%.*]], 0
+; NO_FWD-NEXT: [[SPEC_SELECT:%.*]] = select i1 [[COND1]], i32 [[X:%.*]], i32 0
+; NO_FWD-NEXT: ret i32 [[SPEC_SELECT]]
+;
+; FWD-LABEL: @NotPropagatingPoisonToUndef(
+; FWD-NEXT: D:
+; FWD-NEXT: [[COND1:%.*]] = icmp eq i8 [[COND:%.*]], 0
+; FWD-NEXT: [[SPEC_SELECT:%.*]] = select i1 [[COND1]], i32 [[X:%.*]], i32 0
+; FWD-NEXT: ret i32 [[SPEC_SELECT]]
+;
+ switch i8 %cond, label %A [
+ i8 0, label %B
+ i8 1, label %C
+ ]
+A:
+ br label %D
+B:
+ br label %D
+C:
+ br label %D
+D:
+ %y = phi i32 [ undef, %A ], [ %x, %B ], [ 0, %C ]
+ ret i32 %y
+}
+
+
+
>From 36a1f61927081a27dec0d0cb9b2100ecd2b13661 Mon Sep 17 00:00:00 2001
From: Gabor Spaits <gaborspaits1 at gmail.com>
Date: Mon, 6 Apr 2026 22:46:00 +0200
Subject: [PATCH 2/2] Create own function that checks value redirection
feasibility
---
llvm/lib/Transforms/Utils/Local.cpp | 74 ++++++++++++++++++++---------
1 file changed, 51 insertions(+), 23 deletions(-)
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index 854e5d179f309..85996e9896a17 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -982,7 +982,8 @@ static void replaceUndefValuesInPhi(PHINode *PN,
for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i) {
Value *V = PN->getIncomingValue(i);
- if (!isa<UndefValue>(V)) continue;
+ if (!isa<UndefValue>(V))
+ continue;
BasicBlock *BB = PN->getIncomingBlock(i);
IncomingValueMap::const_iterator It = IncomingValues.find(BB);
@@ -1015,6 +1016,49 @@ static void replaceUndefValuesInPhi(PHINode *PN,
}
}
+static bool canRedirectValuesFromPredecessorsToPhi(
+ BasicBlock *BB, PHINode *PN, Value *OldVal, const PredBlockVector &BBPreds,
+ IncomingValueMap &IncomingValues, BasicBlock *CommonPred) {
+
+ auto IsUnsafeUndefReplacement = [&](BasicBlock *PredBB,
+ Value *Selected) -> bool {
+ int BlockIdx = PN->getBasicBlockIndex(PredBB);
+ if (BlockIdx < 0 || !isa<UndefValue>(PN->getIncomingValue(BlockIdx)))
+ return false;
+
+ return !isa<Constant>(Selected) ||
+ !isGuaranteedNotToBeUndefOrPoison(Selected);
+ };
+
+ if (auto *OldValPN = dyn_cast<PHINode>(OldVal);
+ OldValPN && OldValPN->getParent() == BB) {
+ for (unsigned i = 0, e = OldValPN->getNumIncomingValues(); i != e; ++i) {
+ BasicBlock *PredBB = OldValPN->getIncomingBlock(i);
+ if (PredBB == CommonPred)
+ continue;
+
+ Value *PredVal = OldValPN->getIncomingValue(i);
+ Value *Selected =
+ selectIncomingValueForBlock(PredVal, PredBB, IncomingValues);
+ if (IsUnsafeUndefReplacement(PredBB, Selected))
+ return false;
+ }
+ return true;
+ }
+
+ for (BasicBlock *PredBB : BBPreds) {
+ if (PredBB == CommonPred)
+ continue;
+
+ Value *Selected =
+ selectIncomingValueForBlock(OldVal, PredBB, IncomingValues);
+ if (IsUnsafeUndefReplacement(PredBB, Selected))
+ return false;
+ }
+
+ return true;
+}
+
// Only when they shares a single common predecessor, return true.
// Only handles cases when BB can't be merged while its predecessors can be
// redirected.
@@ -1105,6 +1149,12 @@ static bool redirectValuesFromPredecessorsToPhi(BasicBlock *BB,
gatherIncomingValuesToPhi(PN, BBPreds, IncomingValues);
+ if (!canRedirectValuesFromPredecessorsToPhi(BB, PN, OldVal, BBPreds,
+ IncomingValues, CommonPred)) {
+ PN->addIncoming(OldVal, BB);
+ return false;
+ }
+
// If this incoming value is one of the PHI nodes in BB, the new entries
// in the PHI node are the entries from the old PHI.
if (isa<PHINode>(OldVal) && cast<PHINode>(OldVal)->getParent() == BB) {
@@ -1123,17 +1173,6 @@ static bool redirectValuesFromPredecessorsToPhi(BasicBlock *BB,
Value *PredVal = OldValPN->getIncomingValue(i);
Value *Selected =
selectIncomingValueForBlock(PredVal, PredBB, IncomingValues);
-
- // It can cause an issue if there was a 'undef' assigned for the block
- // before and we replace that block with a value that might be poison.
- int BlockIdx = PN->getBasicBlockIndex(PredBB);
- if (BlockIdx > 0 && isa<UndefValue>(PN->getIncomingValue(BlockIdx)) &&
- (!isa<Constant>(Selected) ||
- !isGuaranteedNotToBeUndefOrPoison(Selected))) {
- PN->addIncoming(OldVal, BB);
- return false;
- }
-
// And add a new incoming value for this predecessor for the
// newly retargeted branch.
PN->addIncoming(Selected, PredBB);
@@ -1150,17 +1189,6 @@ static bool redirectValuesFromPredecessorsToPhi(BasicBlock *BB,
Value *Selected =
selectIncomingValueForBlock(OldVal, PredBB, IncomingValues);
-
- // It can cause an issue if there was a 'undef' assigned for the block
- // before and we replace that block with a value that might be poison.
- int BlockIdx = PN->getBasicBlockIndex(PredBB);
- if (BlockIdx > 0 && isa<UndefValue>(PN->getIncomingValue(BlockIdx)) &&
- (!isa<Constant>(Selected) ||
- !isGuaranteedNotToBeUndefOrPoison(Selected))) {
- PN->addIncoming(OldVal, BB);
- return false;
- }
-
// And add a new incoming value for this predecessor for the
// newly retargeted branch.
PN->addIncoming(Selected, PredBB);
More information about the llvm-commits
mailing list