[llvm] Reland "[BasicBlockUtils] Fix dominator tree update for entry block in splitBlockBefore() (#178895) (PR #179392)
Mingjie Xu via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 2 21:01:47 PST 2026
https://github.com/Enna1 created https://github.com/llvm/llvm-project/pull/179392
https://github.com/llvm/llvm-project/pull/178895 caused a clang crash(see https://lab.llvm.org/buildbot/#/builders/210/builds/8229), reverted in 6d52d2683c2ceb9ab75810730c3ced2509c32bc5.
The crash is assertion `DT && "DT should be available to update LoopInfo!"' failed.
https://github.com/llvm/llvm-project/blob/ad8d5349d46734826aaeae4a2ebdc6f427a5bad8/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp#L1106
```
#7 0x00007f5a380254e3 __assert_perror_fail (/usr/lib/libc.so.6+0x254e3)
#8 0x0000563df5d8fde1 UpdateAnalysisInformation(llvm::BasicBlock*, llvm::BasicBlock*, llvm::ArrayRef<llvm::BasicBlock*>, llvm::DomTreeUpdater*, llvm::DominatorTree*, llvm::LoopInfo*, llvm::MemorySSAUpdater*, bool, bool&) BasicBlockUtils.cpp:0:0
#9 0x0000563df5d8f3bb llvm::splitBlockBefore(llvm::BasicBlock*, llvm::ilist_iterator_w_bits<llvm::ilist_detail::node_options<llvm::Instruction, true, false, void, true, llvm::BasicBlock>, false, false>, llvm::DomTreeUpdater*, llvm::LoopInfo*, llvm::MemorySSAUpdater*, llvm::Twine const&)
#10 0x0000563df5d8cb08 llvm::SplitEdge(llvm::BasicBlock*, llvm::BasicBlock*, llvm::DominatorTree*, llvm::LoopInfo*, llvm::MemorySSAUpdater*, llvm::Twine const&)
#11 0x0000563df4ff5b59 (anonymous namespace)::CodeGenPrepare::splitLargeGEPOffsets()::$_1::operator()(long, llvm::Value*, llvm::GetElementPtrInst*) const CodeGenPrepare.cpp:0:0
#12 0x0000563df4fc0ec8 (anonymous namespace)::CodeGenPrepare::_run(llvm::Function&) CodeGenPrepare.cpp:0:0
#13 0x0000563df4fbb36c (anonymous namespace)::CodeGenPrepareLegacyPass::runOnFunction(llvm::Function&) CodeGenPrepare.cpp:0:0
```
I think this happened when we get DominatorTree with `DT.get()` in `splitLargeGEPOffsets()` but `DT.reset()` already setting it to nullptr in https://github.com/llvm/llvm-project/blob/ad8d5349d46734826aaeae4a2ebdc6f427a5bad8/llvm/lib/CodeGen/CodeGenPrepare.cpp#L660.
In CodeGenPrepare, we should not use `DT.get()` to get DominatorTree directly , should use `getDT()` instead.
I don't have a RSIC-V environment, so no reproducer. checked that the crash is fixed by rerunning buildbot with this patch https://lab.llvm.org/buildbot/#/builders/210/builds/8235
>From 7c480591d605c91e0cd0013fb1fa7b4618bdf59c Mon Sep 17 00:00:00 2001
From: Enna1 <xumingjie.enna1 at bytedance.com>
Date: Tue, 3 Feb 2026 11:07:07 +0800
Subject: [PATCH] Reland "[BasicBlockUtils] Fix dominator tree update for entry
block in splitBlockBefore() (#178895)"
---
llvm/lib/CodeGen/CodeGenPrepare.cpp | 4 +-
llvm/lib/Transforms/Utils/BasicBlockUtils.cpp | 63 ++++++-------------
.../Transforms/Utils/BasicBlockUtilsTest.cpp | 25 ++++++++
3 files changed, 46 insertions(+), 46 deletions(-)
diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp
index 288ce3b52d0e9..43714b7ae2e95 100644
--- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -691,7 +691,7 @@ bool CodeGenPrepare::_run(Function &F) {
MadeChange |= optimizePhiTypes(F);
if (MadeChange)
- eliminateFallThrough(F, DT.get());
+ eliminateFallThrough(F, &getDT(F));
#ifndef NDEBUG
if (MadeChange && VerifyLoopInfo)
@@ -6868,7 +6868,7 @@ bool CodeGenPrepare::splitLargeGEPOffsets() {
NewBaseInsertPt = NewBaseInsertBB->getFirstInsertionPt();
else if (InvokeInst *Invoke = dyn_cast<InvokeInst>(BaseI)) {
NewBaseInsertBB =
- SplitEdge(NewBaseInsertBB, Invoke->getNormalDest(), DT.get(), LI);
+ SplitEdge(NewBaseInsertBB, Invoke->getNormalDest(), &getDT(*NewBaseInsertBB->getParent()), LI);
NewBaseInsertPt = NewBaseInsertBB->getFirstInsertionPt();
} else
NewBaseInsertPt = std::next(BaseI->getIterator());
diff --git a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
index 68d1fd892402c..6472e1771ec73 100644
--- a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
+++ b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
@@ -1054,50 +1054,6 @@ BasicBlock *llvm::SplitBlock(BasicBlock *Old, BasicBlock::iterator SplitPt,
return SplitBlockImpl(Old, SplitPt, DTU, /*DT=*/nullptr, LI, MSSAU, BBName);
}
-BasicBlock *llvm::splitBlockBefore(BasicBlock *Old, BasicBlock::iterator SplitPt,
- DomTreeUpdater *DTU, LoopInfo *LI,
- MemorySSAUpdater *MSSAU,
- const Twine &BBName) {
-
- BasicBlock::iterator SplitIt = SplitPt;
- while (isa<PHINode>(SplitIt) || SplitIt->isEHPad())
- ++SplitIt;
- std::string Name = BBName.str();
- BasicBlock *New = Old->splitBasicBlockBefore(
- SplitIt, Name.empty() ? Old->getName() + ".split" : Name);
-
- // The new block lives in whichever loop the old one did. This preserves
- // LCSSA as well, because we force the split point to be after any PHI nodes.
- if (LI)
- if (Loop *L = LI->getLoopFor(Old))
- L->addBasicBlockToLoop(New, *LI);
-
- if (DTU) {
- SmallVector<DominatorTree::UpdateType, 8> DTUpdates;
- // New dominates Old. The predecessor nodes of the Old node dominate
- // New node.
- SmallPtrSet<BasicBlock *, 8> UniquePredecessorsOfOld;
- DTUpdates.push_back({DominatorTree::Insert, New, Old});
- DTUpdates.reserve(DTUpdates.size() + 2 * pred_size(New));
- for (BasicBlock *PredecessorOfOld : predecessors(New))
- if (UniquePredecessorsOfOld.insert(PredecessorOfOld).second) {
- DTUpdates.push_back({DominatorTree::Insert, PredecessorOfOld, New});
- DTUpdates.push_back({DominatorTree::Delete, PredecessorOfOld, Old});
- }
-
- DTU->applyUpdates(DTUpdates);
-
- // Move MemoryAccesses still tracked in Old, but part of New now.
- // Update accesses in successor blocks accordingly.
- if (MSSAU) {
- MSSAU->applyUpdates(DTUpdates, DTU->getDomTree());
- if (VerifyMemorySSA)
- MSSAU->getMemorySSA()->verifyMemorySSA();
- }
- }
- return New;
-}
-
/// Update DominatorTree, LoopInfo, and LCCSA analysis information.
/// Invalidates DFS Numbering when DTU or DT is provided.
static void UpdateAnalysisInformation(BasicBlock *OldBB, BasicBlock *NewBB,
@@ -1212,6 +1168,25 @@ static void UpdateAnalysisInformation(BasicBlock *OldBB, BasicBlock *NewBB,
}
}
+BasicBlock *llvm::splitBlockBefore(BasicBlock *Old,
+ BasicBlock::iterator SplitPt,
+ DomTreeUpdater *DTU, LoopInfo *LI,
+ MemorySSAUpdater *MSSAU,
+ const Twine &BBName) {
+ BasicBlock::iterator SplitIt = SplitPt;
+ while (isa<PHINode>(SplitIt) || SplitIt->isEHPad())
+ ++SplitIt;
+ SmallVector<BasicBlock *, 4> Preds(predecessors(Old));
+ BasicBlock *New = Old->splitBasicBlockBefore(
+ SplitIt, BBName.isTriviallyEmpty() ? Old->getName() + ".split" : BBName);
+
+ bool HasLoopExit = false;
+ UpdateAnalysisInformation(Old, New, Preds, DTU, nullptr, LI, MSSAU, false,
+ HasLoopExit);
+
+ return New;
+}
+
/// Update the PHI nodes in OrigBB to include the values coming from NewBB.
/// This also updates AliasAnalysis, if available.
static void UpdatePHINodes(BasicBlock *OrigBB, BasicBlock *NewBB,
diff --git a/llvm/unittests/Transforms/Utils/BasicBlockUtilsTest.cpp b/llvm/unittests/Transforms/Utils/BasicBlockUtilsTest.cpp
index 00d9e9ff81e05..d5b4f6b6d6593 100644
--- a/llvm/unittests/Transforms/Utils/BasicBlockUtilsTest.cpp
+++ b/llvm/unittests/Transforms/Utils/BasicBlockUtilsTest.cpp
@@ -378,6 +378,31 @@ define i32 @no_unreachable(i1 %cond) {
EXPECT_TRUE(DT.verify());
}
+TEST(BasicBlockUtils, splitBlockBefore) {
+ LLVMContext C;
+ std::unique_ptr<Module> M = parseIR(C, R"IR(
+define i32 @basic_func(i1 %cond) {
+entry:
+ br i1 %cond, label %bb0, label %bb1
+bb0:
+ br label %bb1
+bb1:
+ %phi = phi i32 [ 0, %entry ], [ 1, %bb0 ]
+ ret i32 %phi
+}
+)IR");
+ Function *F = M->getFunction("basic_func");
+ DominatorTree DT(*F);
+ DomTreeUpdater DTU(DT, DomTreeUpdater::UpdateStrategy::Eager);
+ BasicBlock *EntryBB = &F->getEntryBlock();
+ Instruction *TI = EntryBB->getTerminator();
+
+ // Make sure the dominator tree is properly updated if calling this on the
+ // entry block.
+ splitBlockBefore(EntryBB, TI, &DTU, nullptr, nullptr);
+ EXPECT_TRUE(DTU.getDomTree().verify());
+}
+
TEST(BasicBlockUtils, SplitBlockPredecessors) {
LLVMContext C;
std::unique_ptr<Module> M = parseIR(C, R"IR(
More information about the llvm-commits
mailing list