Reland "[BasicBlockUtils] Fix dominator tree update for entry block in splitBlockBefore() (#178895) by Enna1 · Pull Request #179392 · llvm/llvm-project

Enna1 · 2026-02-03T05:01:40Z

#178895 caused a clang crash(see https://lab.llvm.org/buildbot/#/builders/210/builds/8229), reverted in 6d52d26.

The crash is assertion `DT && "DT should be available to update LoopInfo!"' failed.

llvm-project/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp

Line 1106 in ad8d534

assert(DT && "DT should be available to update LoopInfo!");

 #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

llvm-project/llvm/lib/CodeGen/CodeGenPrepare.cpp

Line 660 in ad8d534

DT.reset();

.
To fix this assertion failure, use getDT() for splitLargeGEPOffsets() to build the DominatorTree if it is set to nullptr by DT.reset().

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/8248

…n splitBlockBefore() (#178895)"

github-actions · 2026-02-03T05:03:23Z

✅ With the latest revision this PR passed the C/C++ code formatter.

github-actions · 2026-02-03T06:44:30Z

🪟 Windows x64 Test Results

130403 tests passed
2904 tests skipped

✅ The build succeeded and all tests passed.

llvmbot · 2026-02-03T07:52:06Z

@llvm/pr-subscribers-llvm-transforms

Author: Mingjie Xu (Enna1)

Changes

#178895 caused a clang crash(see https://lab.llvm.org/buildbot/#/builders/210/builds/8229), reverted in 6d52d26.

The crash is assertion `DT && "DT should be available to update LoopInfo!"' failed.

llvm-project/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp

Line 1106 in ad8d534

assert(DT && "DT should be available to update LoopInfo!");

 #<!-- -->7 0x00007f5a380254e3 __assert_perror_fail (/usr/lib/libc.so.6+0x254e3)
 #<!-- -->8 0x0000563df5d8fde1 UpdateAnalysisInformation(llvm::BasicBlock*, llvm::BasicBlock*, llvm::ArrayRef&lt;llvm::BasicBlock*&gt;, llvm::DomTreeUpdater*, llvm::DominatorTree*, llvm::LoopInfo*, llvm::MemorySSAUpdater*, bool, bool&amp;) BasicBlockUtils.cpp:0:0
 #<!-- -->9 0x0000563df5d8f3bb llvm::splitBlockBefore(llvm::BasicBlock*, llvm::ilist_iterator_w_bits&lt;llvm::ilist_detail::node_options&lt;llvm::Instruction, true, false, void, true, llvm::BasicBlock&gt;, false, false&gt;, llvm::DomTreeUpdater*, llvm::LoopInfo*, llvm::MemorySSAUpdater*, llvm::Twine const&amp;)
#<!-- -->10 0x0000563df5d8cb08 llvm::SplitEdge(llvm::BasicBlock*, llvm::BasicBlock*, llvm::DominatorTree*, llvm::LoopInfo*, llvm::MemorySSAUpdater*, llvm::Twine const&amp;)
#<!-- -->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&amp;) CodeGenPrepare.cpp:0:0
#<!-- -->13 0x0000563df4fbb36c (anonymous namespace)::CodeGenPrepareLegacyPass::runOnFunction(llvm::Function&amp;) 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

llvm-project/llvm/lib/CodeGen/CodeGenPrepare.cpp

Line 660 in ad8d534

DT.reset();

.

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

Full diff: https://github.com/llvm/llvm-project/pull/179392.diff

3 Files Affected:

(modified) llvm/lib/CodeGen/CodeGenPrepare.cpp (+3-2)
(modified) llvm/lib/Transforms/Utils/BasicBlockUtils.cpp (+19-44)
(modified) llvm/unittests/Transforms/Utils/BasicBlockUtilsTest.cpp (+25)

diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp
index 288ce3b52d0e9..4eb771a5573ee 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,8 @@ 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(

nikic · 2026-02-03T10:17:09Z

llvm/lib/CodeGen/CodeGenPrepare.cpp


    if (MadeChange)
-      eliminateFallThrough(F, DT.get());
+      eliminateFallThrough(F, &getDT(F));


I think the idea here was that we don't need to recompute the DT if we're only going to update it. We only need getDT() if we're going to use it.

So I think this call can be left alone. (The same applies to SplitEdge, but that one is an edge-case, so I don't mind forcing the DT calculation there.)

Thanks, this makes sense.
keep DT.get() here.

… update it in eliminateFallThrough()

nikic

LGTM

…n splitBlockBefore() (llvm#178895) (llvm#179392) llvm#178895 caused a clang crash(see https://lab.llvm.org/buildbot/#/builders/210/builds/8229), reverted in 6d52d26. 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 ``` llvm#7 0x00007f5a380254e3 __assert_perror_fail (/usr/lib/libc.so.6+0x254e3) llvm#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 llvm#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&) llvm#10 0x0000563df5d8cb08 llvm::SplitEdge(llvm::BasicBlock*, llvm::BasicBlock*, llvm::DominatorTree*, llvm::LoopInfo*, llvm::MemorySSAUpdater*, llvm::Twine const&) llvm#11 0x0000563df4ff5b59 (anonymous namespace)::CodeGenPrepare::splitLargeGEPOffsets()::$_1::operator()(long, llvm::Value*, llvm::GetElementPtrInst*) const CodeGenPrepare.cpp:0:0 llvm#12 0x0000563df4fc0ec8 (anonymous namespace)::CodeGenPrepare::_run(llvm::Function&) CodeGenPrepare.cpp:0:0 llvm#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. To fix this assertion failure, use `getDT()` for `splitLargeGEPOffsets()` to build the DominatorTree if it is set to nullptr by `DT.reset()`. 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/8248

Splitting a basic block BB into a pair of blocks NewBB->BB used to make LoopInfo invalid. Commit 28a0cfa (PR #179392) fixed this issue. So this commit just adds the test case the issue was found with.

Reland "[BasicBlockUtils] Fix dominator tree update for entry block i…

7c48059

…n splitBlockBefore() (#178895)"

clang-format

d0a4f8d

Enna1 requested a review from nikic February 3, 2026 07:51

Enna1 marked this pull request as ready for review February 3, 2026 07:51

llvmbot added llvm:codegen llvm:transforms labels Feb 3, 2026

nikic mentioned this pull request Feb 3, 2026

[BasicBlockUtilsTests] Added test case splitBlockBefore2. NFC #179408

Merged

yrouban self-requested a review February 3, 2026 09:54

nikic reviewed Feb 3, 2026

View reviewed changes

use DT.get() for eliminateFallThrough() as we are going to use it not…

da092b4

… update it in eliminateFallThrough()

nikic approved these changes Feb 3, 2026

View reviewed changes

Enna1 merged commit 28a0cfa into main Feb 3, 2026
11 checks passed

Enna1 deleted the users/Enna1/Reland-splitBlockBefore-DomTreeUpdater branch February 3, 2026 14:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reland "[BasicBlockUtils] Fix dominator tree update for entry block in splitBlockBefore() (#178895)#179392

Reland "[BasicBlockUtils] Fix dominator tree update for entry block in splitBlockBefore() (#178895)#179392
Enna1 merged 3 commits intomainfrom
users/Enna1/Reland-splitBlockBefore-DomTreeUpdater

Enna1 commented Feb 3, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Feb 3, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Feb 3, 2026 •

edited

Loading

Uh oh!

llvmbot commented Feb 3, 2026

Uh oh!

nikic Feb 3, 2026

Uh oh!

Enna1 Feb 3, 2026

Uh oh!

nikic left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Enna1 commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🪟 Windows x64 Test Results

Uh oh!

llvmbot commented Feb 3, 2026

Uh oh!

nikic Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Enna1 Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Enna1 commented Feb 3, 2026 •

edited

Loading

github-actions bot commented Feb 3, 2026 •

edited

Loading

github-actions bot commented Feb 3, 2026 •

edited

Loading