[BasicBlockUtils] Fix dominator tree update for entry block in splitBlockBefore()#178895
Merged
[BasicBlockUtils] Fix dominator tree update for entry block in splitBlockBefore()#178895
Conversation
…lockBefore() Domoninator update for entry block in `SplitBlockPredecessors()` was fixed by 06dfbb5 , this patch fixes dominator tree update for entry block in `splitBlockBefore()` with `UpdateAnalysisInformation()`.
Member
|
@llvm/pr-subscribers-llvm-transforms Author: Mingjie Xu (Enna1) Changes06dfbb5 fixed domoninator update for entry block in Full diff: https://github.com/llvm/llvm-project/pull/178895.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
index b0c04086f5e84..12206e14ca8fd 100644
--- a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
+++ b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
@@ -1064,51 +1064,6 @@ BasicBlock *llvm::SplitBlock(BasicBlock *Old, BasicBlock::iterator SplitPt,
Before);
}
-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->splitBasicBlock(
- SplitIt, Name.empty() ? Old->getName() + ".split" : Name,
- /* Before=*/true);
-
- // 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,
@@ -1223,6 +1178,27 @@ 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));
+ std::string Name = BBName.str();
+ BasicBlock *New = Old->splitBasicBlock(
+ SplitIt, Name.empty() ? Old->getName() + ".split" : Name,
+ /* Before=*/true);
+
+ 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(
|
arsenm
reviewed
Jan 30, 2026
| while (isa<PHINode>(SplitIt) || SplitIt->isEHPad()) | ||
| ++SplitIt; | ||
| SmallVector<BasicBlock *, 4> Preds(predecessors(Old)); | ||
| std::string Name = BBName.str(); |
Contributor
There was a problem hiding this comment.
No intermediate std::string
nikic
approved these changes
Feb 2, 2026
Comment on lines
1190
to
1192
| BasicBlock *New = Old->splitBasicBlock( | ||
| SplitIt, BBName.isTriviallyEmpty() ? Old->getName() + ".split" : BBName, | ||
| /* Before=*/true); |
Contributor
There was a problem hiding this comment.
Suggested change
| BasicBlock *New = Old->splitBasicBlock( | |
| SplitIt, BBName.isTriviallyEmpty() ? Old->getName() + ".split" : BBName, | |
| /* Before=*/true); | |
| BasicBlock *New = Old->splitBasicBlockBefore( | |
| SplitIt, BBName.isTriviallyEmpty() ? Old->getName() + ".split" : BBName); |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/166/builds/6376 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/210/builds/8229 Here is the relevant piece of the build log for the reference |
Enna1
added a commit
to Enna1/llvm-project
that referenced
this pull request
Feb 3, 2026
…n splitBlockBefore() (llvm#178895)" This reverts commit ad8d534.
Enna1
added a commit
that referenced
this pull request
Feb 3, 2026
…n splitBlockBefore() (#178895)" (#179373) This reverts commit ad8d534. LLVM Buildbot detected a failure, https://lab.llvm.org/buildbot/#/builders/210/builds/8229
nikic
pushed a commit
to nikic/llvm-project
that referenced
this pull request
Feb 3, 2026
…n splitBlockBefore() (llvm#178895)" clang-format
Enna1
added a commit
that referenced
this pull request
Feb 3, 2026
…n splitBlockBefore() (#178895) (#179392) #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 ``` #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. 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
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Feb 3, 2026
…try block in splitBlockBefore() (#178895) (#179392) llvm/llvm-project#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 ``` #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. 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
moar55
pushed a commit
to moar55/llvm-project
that referenced
this pull request
Feb 3, 2026
…lockBefore() (llvm#178895) 06dfbb5 fixed dominator update for entry block in `SplitBlockPredecessors()`, this patch fixes dominator tree update for entry block in `splitBlockBefore()` with `UpdateAnalysisInformation()`.
moar55
pushed a commit
to moar55/llvm-project
that referenced
this pull request
Feb 3, 2026
…n splitBlockBefore() (llvm#178895)" (llvm#179373) This reverts commit ad8d534. LLVM Buildbot detected a failure, https://lab.llvm.org/buildbot/#/builders/210/builds/8229
moar55
pushed a commit
to moar55/llvm-project
that referenced
this pull request
Feb 3, 2026
…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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
06dfbb5 fixed dominator update for entry block in
SplitBlockPredecessors(), this patch fixes dominator tree update for entry block insplitBlockBefore()withUpdateAnalysisInformation().