Skip to content

Commit

Permalink
[LoopPeel] Use reference instead of pointer for DT argument
Browse files Browse the repository at this point in the history
Cleanup code in peelLoop API. We already have usage of DT without guarding
against a null DT, so this change constant folds the remaining null DT
checks.
Also make the argument a reference so that it is clear the argument is
a nonnull DT.
Extracted from D118472.
  • Loading branch information
annamthomas committed Feb 1, 2022
1 parent 453620f commit bc48a26
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 35 deletions.
2 changes: 1 addition & 1 deletion llvm/include/llvm/Transforms/Utils/LoopPeel.h
Expand Up @@ -21,7 +21,7 @@ namespace llvm {
bool canPeel(Loop *L);

bool peelLoop(Loop *L, unsigned PeelCount, LoopInfo *LI, ScalarEvolution *SE,
DominatorTree *DT, AssumptionCache *AC, bool PreserveLCSSA);
DominatorTree &DT, AssumptionCache *AC, bool PreserveLCSSA);

TargetTransformInfo::PeelingPreferences
gatherPeelingPreferences(Loop *L, ScalarEvolution &SE,
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Transforms/Scalar/LoopFuse.cpp
Expand Up @@ -767,7 +767,7 @@ struct LoopFuser {
LLVM_DEBUG(dbgs() << "Attempting to peel first " << PeelCount
<< " iterations of the first loop. \n");

FC0.Peeled = peelLoop(FC0.L, PeelCount, &LI, &SE, &DT, &AC, true);
FC0.Peeled = peelLoop(FC0.L, PeelCount, &LI, &SE, DT, &AC, true);
if (FC0.Peeled) {
LLVM_DEBUG(dbgs() << "Done Peeling\n");

Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp
Expand Up @@ -1281,7 +1281,7 @@ static LoopUnrollResult tryToUnrollLoop(
<< " iterations";
});

if (peelLoop(L, PP.PeelCount, LI, &SE, &DT, &AC, PreserveLCSSA)) {
if (peelLoop(L, PP.PeelCount, LI, &SE, DT, &AC, PreserveLCSSA)) {
simplifyLoopAfterUnroll(L, true, LI, &SE, &DT, &AC, &TTI);
// If the loop was peeled, we already "used up" the profile information
// we had, so we don't want to unroll or peel again.
Expand Down
60 changes: 28 additions & 32 deletions llvm/lib/Transforms/Utils/LoopPeel.cpp
Expand Up @@ -737,7 +737,7 @@ TargetTransformInfo::PeelingPreferences llvm::gatherPeelingPreferences(
/// for the bulk of dynamic execution, can be further simplified by scalar
/// optimizations.
bool llvm::peelLoop(Loop *L, unsigned PeelCount, LoopInfo *LI,
ScalarEvolution *SE, DominatorTree *DT, AssumptionCache *AC,
ScalarEvolution *SE, DominatorTree &DT, AssumptionCache *AC,
bool PreserveLCSSA) {
assert(PeelCount > 0 && "Attempt to peel out zero iterations?");
assert(canPeel(L) && "Attempt to peel a loop which is not peelable?");
Expand All @@ -756,23 +756,21 @@ bool llvm::peelLoop(Loop *L, unsigned PeelCount, LoopInfo *LI,
// routes which can lead to the exit: we can reach it from the peeled
// iterations too.
DenseMap<BasicBlock *, BasicBlock *> NonLoopBlocksIDom;
if (DT) {
for (auto *BB : L->blocks()) {
auto *BBDomNode = DT->getNode(BB);
SmallVector<BasicBlock *, 16> ChildrenToUpdate;
for (auto *ChildDomNode : BBDomNode->children()) {
auto *ChildBB = ChildDomNode->getBlock();
if (!L->contains(ChildBB))
ChildrenToUpdate.push_back(ChildBB);
}
// The new idom of the block will be the nearest common dominator
// of all copies of the previous idom. This is equivalent to the
// nearest common dominator of the previous idom and the first latch,
// which dominates all copies of the previous idom.
BasicBlock *NewIDom = DT->findNearestCommonDominator(BB, Latch);
for (auto *ChildBB : ChildrenToUpdate)
NonLoopBlocksIDom[ChildBB] = NewIDom;
for (auto *BB : L->blocks()) {
auto *BBDomNode = DT.getNode(BB);
SmallVector<BasicBlock *, 16> ChildrenToUpdate;
for (auto *ChildDomNode : BBDomNode->children()) {
auto *ChildBB = ChildDomNode->getBlock();
if (!L->contains(ChildBB))
ChildrenToUpdate.push_back(ChildBB);
}
// The new idom of the block will be the nearest common dominator
// of all copies of the previous idom. This is equivalent to the
// nearest common dominator of the previous idom and the first latch,
// which dominates all copies of the previous idom.
BasicBlock *NewIDom = DT.findNearestCommonDominator(BB, Latch);
for (auto *ChildBB : ChildrenToUpdate)
NonLoopBlocksIDom[ChildBB] = NewIDom;
}

Function *F = Header->getParent();
Expand Down Expand Up @@ -822,11 +820,11 @@ bool llvm::peelLoop(Loop *L, unsigned PeelCount, LoopInfo *LI,
// If (cond) goto Header
// Exit:

BasicBlock *InsertTop = SplitEdge(PreHeader, Header, DT, LI);
BasicBlock *InsertTop = SplitEdge(PreHeader, Header, &DT, LI);
BasicBlock *InsertBot =
SplitBlock(InsertTop, InsertTop->getTerminator(), DT, LI);
SplitBlock(InsertTop, InsertTop->getTerminator(), &DT, LI);
BasicBlock *NewPreHeader =
SplitBlock(InsertBot, InsertBot->getTerminator(), DT, LI);
SplitBlock(InsertBot, InsertBot->getTerminator(), &DT, LI);

InsertTop->setName(Header->getName() + ".peel.begin");
InsertBot->setName(Header->getName() + ".peel.next");
Expand All @@ -852,23 +850,21 @@ bool llvm::peelLoop(Loop *L, unsigned PeelCount, LoopInfo *LI,
ValueToValueMapTy VMap;

cloneLoopBlocks(L, Iter, InsertTop, InsertBot, ExitEdges, NewBlocks,
LoopBlocks, VMap, LVMap, DT, LI,
LoopBlocks, VMap, LVMap, &DT, LI,
LoopLocalNoAliasDeclScopes);

// Remap to use values from the current iteration instead of the
// previous one.
remapInstructionsInBlocks(NewBlocks, VMap);

if (DT) {
// Update IDoms of the blocks reachable through exits.
if (Iter == 0)
for (auto BBIDom : NonLoopBlocksIDom)
DT->changeImmediateDominator(BBIDom.first,
cast<BasicBlock>(LVMap[BBIDom.second]));
// Update IDoms of the blocks reachable through exits.
if (Iter == 0)
for (auto BBIDom : NonLoopBlocksIDom)
DT.changeImmediateDominator(BBIDom.first,
cast<BasicBlock>(LVMap[BBIDom.second]));
#ifdef EXPENSIVE_CHECKS
assert(DT->verify(DominatorTree::VerificationLevel::Fast));
assert(DT.verify(DominatorTree::VerificationLevel::Fast));
#endif
}

auto *LatchBRCopy = cast<BranchInst>(VMap[LatchBR]);
updateBranchWeights(InsertBot, LatchBRCopy, ExitWeight, FallThroughWeight);
Expand All @@ -877,7 +873,7 @@ bool llvm::peelLoop(Loop *L, unsigned PeelCount, LoopInfo *LI,
LatchBRCopy->setMetadata(LLVMContext::MD_loop, nullptr);

InsertTop = InsertBot;
InsertBot = SplitBlock(InsertBot, InsertBot->getTerminator(), DT, LI);
InsertBot = SplitBlock(InsertBot, InsertBot->getTerminator(), &DT, LI);
InsertBot->setName(Header->getName() + ".peel.next");

F->getBasicBlockList().splice(InsertTop->getIterator(),
Expand Down Expand Up @@ -912,10 +908,10 @@ bool llvm::peelLoop(Loop *L, unsigned PeelCount, LoopInfo *LI,
SE->forgetTopmostLoop(L);

// Finally DomtTree must be correct.
assert(DT->verify(DominatorTree::VerificationLevel::Fast));
assert(DT.verify(DominatorTree::VerificationLevel::Fast));

// FIXME: Incrementally update loop-simplify
simplifyLoop(L, DT, LI, SE, AC, nullptr, PreserveLCSSA);
simplifyLoop(L, &DT, LI, SE, AC, nullptr, PreserveLCSSA);

NumPeeled++;

Expand Down

0 comments on commit bc48a26

Please sign in to comment.