Skip to content

Commit c96f019

Browse files
authored
[BasicBlockUtils] Remove broken support for eh pads in SplitEdge() (#137816)
d81d9e8 changed SplitEdge() to make use of ehAwareSplitEdge() for critical edges where the target is an eh pad. However, the implementation is incorrect at least for landing pads. What is currently produced for the code in the modified unit test is something like this: continue: invoke void @sink() to label %normal unwind label %new_bb new_bb: %cp = cleanuppad within %exception [] cleanupret from %cp unwind label %exception exception: %cleanup = landingpad i8 cleanup br label %trivial-eh-handler This mixes different exception handling mechanisms in a nonsensical way, and is not well-formed IR. To actually "split" the landingpad edge (for a rather loose definition of "split"), I think we'd have to generate something along these lines: exception.split: %cleanup.split = landingpad i8 cleanup br label %exception.cont exception: %cleanup.orig = landingpad i8 cleanup br label %exception.cont exception.cont: %cleanup = phi i8 [ %cleanup.split, %exception_split ], [ %cleanup.orig, %exception ] I didn't bother actually implementing that, seeing as how nobody noticed the existing codegen being broken in the last four years, so clearly nobody actually needs this function to work with EH edges. Just return nullptr instead.
1 parent af497d9 commit c96f019

File tree

2 files changed

+2
-21
lines changed

2 files changed

+2
-21
lines changed

llvm/lib/Transforms/Utils/BasicBlockUtils.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -768,11 +768,6 @@ BasicBlock *llvm::SplitEdge(BasicBlock *BB, BasicBlock *Succ, DominatorTree *DT,
768768
CriticalEdgeSplittingOptions(DT, LI, MSSAU).setPreserveLCSSA();
769769

770770
if ((isCriticalEdge(LatchTerm, SuccNum, Options.MergeIdenticalEdges))) {
771-
// If it is a critical edge, and the succesor is an exception block, handle
772-
// the split edge logic in this specific function
773-
if (Succ->isEHPad())
774-
return ehAwareSplitEdge(BB, Succ, nullptr, nullptr, Options, BBName);
775-
776771
// If this is a critical edge, let SplitKnownCriticalEdge do it.
777772
return SplitKnownCriticalEdge(LatchTerm, SuccNum, Options, BBName);
778773
}

llvm/unittests/Transforms/Utils/BasicBlockUtilsTest.cpp

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -285,22 +285,8 @@ declare void @sink_alt() cold
285285
EXPECT_TRUE(Ehpad);
286286

287287
BasicBlock *NewBB = SplitEdge(SrcBlock, DestBlock, &DT, &LI, &MSSAU, "");
288-
289-
MSSA.verifyMemorySSA();
290-
EXPECT_TRUE(DT.verify());
291-
EXPECT_NE(NewBB, nullptr);
292-
EXPECT_EQ(NewBB->getSinglePredecessor(), SrcBlock);
293-
EXPECT_EQ(NewBB, SrcBlock->getTerminator()->getSuccessor(SuccNum));
294-
EXPECT_EQ(NewBB->getParent(), F);
295-
296-
bool BBFlag = false;
297-
for (BasicBlock &BB : *F) {
298-
if (BB.getName() == NewBB->getName()) {
299-
BBFlag = true;
300-
break;
301-
}
302-
}
303-
EXPECT_TRUE(BBFlag);
288+
// SplitEdge cannot split an eh pad edge.
289+
EXPECT_EQ(NewBB, nullptr);
304290
}
305291

306292
TEST(BasicBlockUtils, splitBasicBlockBefore_ex1) {

0 commit comments

Comments
 (0)