[llvm] 12dd3a7 - Recommit "[LoopUnroll] Directly update DT instead of DTU."
Florian Hahn via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 20 08:33:47 PST 2023
Author: Florian Hahn
Date: 2023-01-20T16:33:40Z
New Revision: 12dd3a7b54cd19524fc29edba374d26972246744
URL: https://github.com/llvm/llvm-project/commit/12dd3a7b54cd19524fc29edba374d26972246744
DIFF: https://github.com/llvm/llvm-project/commit/12dd3a7b54cd19524fc29edba374d26972246744.diff
LOG: Recommit "[LoopUnroll] Directly update DT instead of DTU."
This reverts commit c5ea42bcf48c8f3d3e35a6bff620b06d2a499108.
Recommit the patch with a fix for loops where the exiting terminator is
not a branch instruction. In that case, ExitInfos may be empty. In
addition to checking if there's a single exiting block also check if
there's a single ExitInfo.
A test case has been added in f92b35392ed8e4631.
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 7db466ecdd8c6..c97baaf4afc2f 100644
--- a/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
+++ b/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
@@ -90,11 +90,14 @@ 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);
+ bool PredecessorWithTwoSuccessors = false,
+ DominatorTree *DT = nullptr);
/// 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 9b35faf1a8188..58a226fc601c1 100644
--- a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
+++ b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
@@ -179,7 +179,8 @@ bool llvm::DeleteDeadPHIs(BasicBlock *BB, const TargetLibraryInfo *TLI,
bool llvm::MergeBlockIntoPredecessor(BasicBlock *BB, DomTreeUpdater *DTU,
LoopInfo *LI, MemorySSAUpdater *MSSAU,
MemoryDependenceResults *MemDep,
- bool PredecessorWithTwoSuccessors) {
+ bool PredecessorWithTwoSuccessors,
+ DominatorTree *DT) {
if (BB->hasAddressTaken())
return false;
@@ -232,10 +233,21 @@ 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),
@@ -311,6 +323,12 @@ 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 cc96d15d5e596..e8f585b4a94dd 100644
--- a/llvm/lib/Transforms/Utils/LoopUnroll.cpp
+++ b/llvm/lib/Transforms/Utils/LoopUnroll.cpp
@@ -321,6 +321,7 @@ 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;
@@ -680,8 +681,7 @@ LoopUnrollResult llvm::UnrollLoop(Loop *L, UnrollLoopOptions ULO, LoopInfo *LI,
assert(!UnrollVerifyDomtree ||
DT->verify(DominatorTree::VerificationLevel::Fast));
- DomTreeUpdater DTU(DT, DomTreeUpdater::UpdateStrategy::Lazy);
-
+ SmallVector<DominatorTree::UpdateType> DTUpdates;
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();
- DTU.applyUpdates({{DominatorTree::Delete, Src, DeadSucc}});
+ DTUpdates.emplace_back(DominatorTree::Delete, Src, DeadSucc);
};
auto WillExit = [&](const ExitInfo &Info, unsigned i, unsigned j,
@@ -733,28 +733,56 @@ LoopUnrollResult llvm::UnrollLoop(Loop *L, UnrollLoopOptions ULO, LoopInfo *LI,
// Fold branches for iterations where we know that they will exit or not
// exit.
- for (const auto &Pair : ExitInfos) {
- const ExitInfo &Info = Pair.second;
+ for (auto &Pair : ExitInfos) {
+ 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 (!KnownWillExit) {
+ if (!Info.FirstExitingBlock)
+ Info.FirstExitingBlock = Info.ExitingBlocks[i];
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 (*KnownWillExit && !IsLatch) {
+ if (!Info.FirstExitingBlock)
+ Info.FirstExitingBlock = Info.ExitingBlocks[i];
continue;
+ }
SetDest(Info.ExitingBlocks[i], *KnownWillExit, Info.ExitOnTrue);
}
}
+ DomTreeUpdater DTU(DT, DomTreeUpdater::UpdateStrategy::Lazy);
+ DomTreeUpdater *DTUToUse = &DTU;
+ if (ExitingBlocks.size() == 1 && ExitInfos.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
@@ -774,16 +802,21 @@ 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, LI)) {
+ if (MergeBlockIntoPredecessor(Dest, /*DTU=*/DTUToUse, LI,
+ /*MSSAU=*/nullptr, /*MemDep=*/nullptr,
+ /*PredecessorWithTwoSuccessors=*/false,
+ DTUToUse ? nullptr : DT)) {
// 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