Skip to content

Commit 1563edc

Browse files
authored
[Utils] Fix incorrect LCSSA PHI nodes when splitting critical edges with MergeIdenticalEdges
When splitting a critical edge from a block with multiple identical edges to an exit block while both `PreserveLCSSA` and `MergeIdenticalEdges` are enabled, the generated LCSSA PHI nodes in the split block would miss incoming values from merged edges. This occurs because: 1. `MergeIdenticalEdges` merges multiple identical edges into a single edge to the new split block. 2. `PreserveLCSSA` (in `createPHIsForSplitLoopExit`) previously assumed only one incoming edge from the original block, creating PHI nodes with incomplete predecessors. The fix modifies `createPHIsForSplitLoopExit` to account for merged edges by passing all original predecessor entries (matching the number of merged edges) when creating PHI nodes. This ensures all incoming values are properly reflected in the split block's PHI nodes. Add unittest case in `BasicBlockUtilsTest.cpp` to verify the correction of PHI node generation in this scenario.
1 parent 2f808dd commit 1563edc

File tree

2 files changed

+67
-1
lines changed

2 files changed

+67
-1
lines changed

llvm/lib/Transforms/Utils/BreakCriticalEdges.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,8 @@ llvm::SplitKnownCriticalEdge(Instruction *TI, unsigned SuccNum,
205205
}
206206
}
207207

208+
unsigned NumSplittedIdenticalEdges = 1;
209+
208210
// If there are any other edges from TIBB to DestBB, update those to go
209211
// through the split block, making those edges non-critical as well (and
210212
// reducing the number of phi entries in the DestBB if relevant).
@@ -217,6 +219,9 @@ llvm::SplitKnownCriticalEdge(Instruction *TI, unsigned SuccNum,
217219

218220
// We found another edge to DestBB, go to NewBB instead.
219221
TI->setSuccessor(i, NewBB);
222+
223+
// Record the number of splitted identical edges to DestBB.
224+
NumSplittedIdenticalEdges++;
220225
}
221226
}
222227

@@ -288,7 +293,11 @@ llvm::SplitKnownCriticalEdge(Instruction *TI, unsigned SuccNum,
288293

289294
// Update LCSSA form in the newly created exit block.
290295
if (Options.PreserveLCSSA) {
291-
createPHIsForSplitLoopExit(TIBB, NewBB, DestBB);
296+
// If > 1 identical edges to be splitted, we need to introduce
297+
// the incoming blocks of the same number for the new PHINode.
298+
createPHIsForSplitLoopExit(
299+
SmallVector<BasicBlock *, 4>(NumSplittedIdenticalEdges, TIBB),
300+
NewBB, DestBB);
292301
}
293302

294303
if (!LoopPreds.empty()) {

llvm/unittests/Transforms/Utils/BasicBlockUtilsTest.cpp

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,63 @@ define void @crit_edge(i1 %cond0, i1 %cond1) {
438438
EXPECT_TRUE(PDT.verify());
439439
}
440440

441+
TEST(BasicBlockUtils, SplitLoopCriticalEdge) {
442+
LLVMContext C;
443+
std::unique_ptr<Module> M = parseIR(C, R"IR(
444+
declare dso_local i1 @predicate(ptr noundef %p)
445+
446+
define dso_local ptr @Parse(ptr noundef %gp) {
447+
entry:
448+
br label %for.inc
449+
450+
for.inc:
451+
%phi = phi ptr [ %gp, %entry ], [ %cp, %while.cond ], [ %cp, %while.cond ]
452+
%cond = call i1 @predicate(ptr noundef %phi)
453+
%inc= getelementptr inbounds i8, ptr %phi, i64 1
454+
br i1 %cond, label %while.cond, label %exit
455+
456+
while.cond:
457+
%cp = phi ptr [ %inc, %for.inc ], [ %incdec, %while.body ]
458+
%val = load i8, ptr %cp, align 1
459+
switch i8 %val, label %while.body [
460+
i8 10, label %for.inc
461+
i8 0, label %for.inc
462+
]
463+
464+
while.body:
465+
%incdec= getelementptr inbounds i8, ptr %cp, i64 1
466+
br label %while.cond
467+
468+
exit:
469+
ret ptr %phi
470+
}
471+
)IR");
472+
Function *F = M->getFunction("Parse");
473+
DominatorTree DT(*F);
474+
LoopInfo LI(DT);
475+
476+
CriticalEdgeSplittingOptions CESO =
477+
CriticalEdgeSplittingOptions(nullptr, &LI, nullptr)
478+
.setMergeIdenticalEdges()
479+
.setPreserveLCSSA();
480+
EXPECT_EQ(2u, SplitAllCriticalEdges(*F, CESO));
481+
482+
BasicBlock *WhileBB = getBasicBlockByName(*F, "while.cond");
483+
BasicBlock *SplitBB = WhileBB->getTerminator()->getSuccessor(1);
484+
// The only 1 successor of SplitBB is %for.inc
485+
ASSERT_EQ(1u, SplitBB->getTerminator()->getNumSuccessors());
486+
// MergeIdenticalEdges: SplitBB has two identical predecessors, %while.cond.
487+
ASSERT_EQ(WhileBB, SplitBB->getUniquePredecessor());
488+
ASSERT_EQ(true, SplitBB->hasNPredecessors(2));
489+
490+
PHINode *PN = dyn_cast<PHINode>(&(SplitBB->front()));
491+
// PreserveLCSSA: should insert a PHI node in front of SplitBB
492+
ASSERT_NE(nullptr, PN);
493+
// The PHI node should have 2 identical incoming blocks.
494+
ASSERT_EQ(2u, PN->getNumIncomingValues());
495+
ASSERT_EQ(PN->getIncomingBlock(0), PN->getIncomingBlock(1));
496+
}
497+
441498
TEST(BasicBlockUtils, SplitIndirectBrCriticalEdgesIgnorePHIs) {
442499
LLVMContext C;
443500
std::unique_ptr<Module> M = parseIR(C, R"IR(

0 commit comments

Comments
 (0)