[llvm] c5ea42b - Revert "[LoopUnroll] Directly update DT instead of DTU."
Arthur Eubanks via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 19 17:04:40 PST 2023
Author: Arthur Eubanks
Date: 2023-01-19T17:01:15-08:00
New Revision: c5ea42bcf48c8f3d3e35a6bff620b06d2a499108
URL: https://github.com/llvm/llvm-project/commit/c5ea42bcf48c8f3d3e35a6bff620b06d2a499108
DIFF: https://github.com/llvm/llvm-project/commit/c5ea42bcf48c8f3d3e35a6bff620b06d2a499108.diff
LOG: Revert "[LoopUnroll] Directly update DT instead of DTU."
This reverts commit d0907ce7ed9f159562ca3f4cfd8d87e89e93febe.
Causes `opt -passes=loop-unroll-full` to crash on
```
define void @foo() {
bb:
br label %bb1
bb1: ; preds = %bb1, %bb1, %bb
switch i1 true, label %bb1 [
i1 true, label %bb2
i1 false, label %bb1
]
bb2: ; preds = %bb1
ret void
}
```
Added:
Modified:
llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
llvm/lib/Transforms/Utils/LoopUnroll.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h b/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
index c97baaf4afc2f..7db466ecdd8c6 100644
--- a/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
+++ b/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
@@ -90,14 +90,11 @@ bool DeleteDeadPHIs(BasicBlock *BB, const TargetLibraryInfo *TLI = nullptr,
/// if BB's Pred has a branch to BB and to AnotherBB, and BB has a single
/// successor Sing. In this case the branch will be updated with Sing instead of
/// BB, and BB will still be merged into its predecessor and removed.
-/// If \p DT is not nullptr, update it directly; in that case, DTU must be
-/// nullptr.
bool MergeBlockIntoPredecessor(BasicBlock *BB, DomTreeUpdater *DTU = nullptr,
LoopInfo *LI = nullptr,
MemorySSAUpdater *MSSAU = nullptr,
MemoryDependenceResults *MemDep = nullptr,
- bool PredecessorWithTwoSuccessors = false,
- DominatorTree *DT = nullptr);
+ bool PredecessorWithTwoSuccessors = false);
/// Merge block(s) sucessors, if possible. Return true if at least two
/// of the blocks were merged together.
diff --git a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
index 8e49edb4dac16..d14e5b8224500 100644
--- a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
+++ b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
@@ -179,8 +179,7 @@ bool llvm::DeleteDeadPHIs(BasicBlock *BB, const TargetLibraryInfo *TLI,
bool llvm::MergeBlockIntoPredecessor(BasicBlock *BB, DomTreeUpdater *DTU,
LoopInfo *LI, MemorySSAUpdater *MSSAU,
MemoryDependenceResults *MemDep,
- bool PredecessorWithTwoSuccessors,
- DominatorTree *DT) {
+ bool PredecessorWithTwoSuccessors) {
if (BB->hasAddressTaken())
return false;
@@ -233,21 +232,10 @@ bool llvm::MergeBlockIntoPredecessor(BasicBlock *BB, DomTreeUpdater *DTU,
FoldSingleEntryPHINodes(BB, MemDep);
}
- if (DT) {
- assert(!DTU && "cannot use both DT and DTU for updates");
- DomTreeNode *PredNode = DT->getNode(PredBB);
- DomTreeNode *BBNode = DT->getNode(BB);
- if (PredNode) {
- assert(BBNode && "PredNode unreachable but BBNode reachable?");
- for (DomTreeNode *C : to_vector(BBNode->children()))
- C->setIDom(PredNode);
- }
- }
// DTU update: Collect all the edges that exit BB.
// These dominator edges will be redirected from Pred.
std::vector<DominatorTree::UpdateType> Updates;
if (DTU) {
- assert(!DT && "cannot use both DT and DTU for updates");
// To avoid processing the same predecessor more than once.
SmallPtrSet<BasicBlock *, 8> SeenSuccs;
SmallPtrSet<BasicBlock *, 2> SuccsOfPredBB(succ_begin(PredBB),
@@ -323,12 +311,6 @@ bool llvm::MergeBlockIntoPredecessor(BasicBlock *BB, DomTreeUpdater *DTU,
if (DTU)
DTU->applyUpdates(Updates);
- if (DT) {
- assert(succ_empty(BB) &&
- "successors should have been transferred to PredBB");
- DT->eraseNode(BB);
- }
-
// Finally, erase the old block and update dominator info.
DeleteDeadBlock(BB, DTU);
diff --git a/llvm/lib/Transforms/Utils/LoopUnroll.cpp b/llvm/lib/Transforms/Utils/LoopUnroll.cpp
index f8251ee9d6458..cc96d15d5e596 100644
--- a/llvm/lib/Transforms/Utils/LoopUnroll.cpp
+++ b/llvm/lib/Transforms/Utils/LoopUnroll.cpp
@@ -321,7 +321,6 @@ LoopUnrollResult llvm::UnrollLoop(Loop *L, UnrollLoopOptions ULO, LoopInfo *LI,
unsigned TripMultiple;
unsigned BreakoutTrip;
bool ExitOnTrue;
- BasicBlock *FirstExitingBlock = nullptr;
SmallVector<BasicBlock *> ExitingBlocks;
};
DenseMap<BasicBlock *, ExitInfo> ExitInfos;
@@ -681,7 +680,8 @@ LoopUnrollResult llvm::UnrollLoop(Loop *L, UnrollLoopOptions ULO, LoopInfo *LI,
assert(!UnrollVerifyDomtree ||
DT->verify(DominatorTree::VerificationLevel::Fast));
- SmallVector<DominatorTree::UpdateType> DTUpdates;
+ DomTreeUpdater DTU(DT, DomTreeUpdater::UpdateStrategy::Lazy);
+
auto SetDest = [&](BasicBlock *Src, bool WillExit, bool ExitOnTrue) {
auto *Term = cast<BranchInst>(Src->getTerminator());
const unsigned Idx = ExitOnTrue ^ WillExit;
@@ -695,7 +695,7 @@ LoopUnrollResult llvm::UnrollLoop(Loop *L, UnrollLoopOptions ULO, LoopInfo *LI,
BranchInst::Create(Dest, Term);
Term->eraseFromParent();
- DTUpdates.emplace_back(DominatorTree::Delete, Src, DeadSucc);
+ DTU.applyUpdates({{DominatorTree::Delete, Src, DeadSucc}});
};
auto WillExit = [&](const ExitInfo &Info, unsigned i, unsigned j,
@@ -733,56 +733,28 @@ LoopUnrollResult llvm::UnrollLoop(Loop *L, UnrollLoopOptions ULO, LoopInfo *LI,
// Fold branches for iterations where we know that they will exit or not
// exit.
- for (auto &Pair : ExitInfos) {
- ExitInfo &Info = Pair.second;
+ for (const auto &Pair : ExitInfos) {
+ const ExitInfo &Info = Pair.second;
for (unsigned i = 0, e = Info.ExitingBlocks.size(); i != e; ++i) {
// The branch destination.
unsigned j = (i + 1) % e;
bool IsLatch = Pair.first == LatchBlock;
std::optional<bool> KnownWillExit = WillExit(Info, i, j, IsLatch);
- if (!KnownWillExit) {
- if (!Info.FirstExitingBlock)
- Info.FirstExitingBlock = Info.ExitingBlocks[i];
+ if (!KnownWillExit)
continue;
- }
// We don't fold known-exiting branches for non-latch exits here,
// because this ensures that both all loop blocks and all exit blocks
// remain reachable in the CFG.
// TODO: We could fold these branches, but it would require much more
// sophisticated updates to LoopInfo.
- if (*KnownWillExit && !IsLatch) {
- if (!Info.FirstExitingBlock)
- Info.FirstExitingBlock = Info.ExitingBlocks[i];
+ if (*KnownWillExit && !IsLatch)
continue;
- }
SetDest(Info.ExitingBlocks[i], *KnownWillExit, Info.ExitOnTrue);
}
}
- DomTreeUpdater DTU(DT, DomTreeUpdater::UpdateStrategy::Lazy);
- DomTreeUpdater *DTUToUse = &DTU;
- if (ExitingBlocks.size() == 1) {
- // Manually update the DT if there's a single exiting node. In that case
- // there's a single exit node and it is sufficient to update the nodes
- // immediately dominated by the original exiting block. They will become
- // dominated by the first exiting block that leaves the loop after
- // unrolling. Note that the CFG inside the loop does not change, so there's
- // no need to update the DT inside the unrolled loop.
- DTUToUse = nullptr;
- auto &[OriginalExit, Info] = *ExitInfos.begin();
- if (!Info.FirstExitingBlock)
- Info.FirstExitingBlock = Info.ExitingBlocks.back();
- for (auto *C : to_vector(DT->getNode(OriginalExit)->children())) {
- if (L->contains(C->getBlock()))
- continue;
- C->setIDom(DT->getNode(Info.FirstExitingBlock));
- }
- } else {
- DTU.applyUpdates(DTUpdates);
- }
-
// When completely unrolling, the last latch becomes unreachable.
if (!LatchIsExiting && CompletelyUnroll) {
// There is no need to update the DT here, because there must be a unique
@@ -802,21 +774,16 @@ LoopUnrollResult llvm::UnrollLoop(Loop *L, UnrollLoopOptions ULO, LoopInfo *LI,
if (Term && Term->isUnconditional()) {
BasicBlock *Dest = Term->getSuccessor(0);
BasicBlock *Fold = Dest->getUniquePredecessor();
- if (MergeBlockIntoPredecessor(Dest, /*DTU=*/DTUToUse, LI,
- /*MSSAU=*/nullptr, /*MemDep=*/nullptr,
- /*PredecessorWithTwoSuccessors=*/false,
- DTUToUse ? nullptr : DT)) {
+ if (MergeBlockIntoPredecessor(Dest, &DTU, LI)) {
// Dest has been folded into Fold. Update our worklists accordingly.
std::replace(Latches.begin(), Latches.end(), Dest, Fold);
llvm::erase_value(UnrolledLoopBlocks, Dest);
}
}
}
+ // Apply updates to the DomTree.
+ DT = &DTU.getDomTree();
- if (DTUToUse) {
- // Apply updates to the DomTree.
- DT = &DTU.getDomTree();
- }
assert(!UnrollVerifyDomtree ||
DT->verify(DominatorTree::VerificationLevel::Fast));
More information about the llvm-commits
mailing list