Skip to content

Commit 691bb41

Browse files
committed
implement feedback
1 parent 142a016 commit 691bb41

File tree

5 files changed

+59
-58
lines changed

5 files changed

+59
-58
lines changed

llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -608,17 +608,10 @@ LLVM_ABI bool SplitIndirectBrCriticalEdges(Function &F,
608608
// successors
609609
LLVM_ABI void InvertBranch(BranchInst *PBI, IRBuilderBase &Builder);
610610

611-
template <typename... TermInst>
612-
LLVM_ABI bool hasOnlyGivenTerminators(const Function &F);
613-
614-
// Check whether the function only has blocks with simple terminators:
611+
// Check whether the function only has simple terminator:
615612
// br/brcond/unreachable/ret
616613
LLVM_ABI bool hasOnlySimpleTerminator(const Function &F);
617614

618-
// Check whether the function only has blocks with simple terminators
619-
// (br/brcond/unreachable/ret) or callbr.
620-
LLVM_ABI bool hasOnlySimpleTerminatorOrCallBr(const Function &F);
621-
622615
LLVM_ABI Printable printBBPtr(const BasicBlock *BB);
623616

624617
} // end namespace llvm

llvm/lib/Transforms/Utils/BasicBlockUtils.cpp

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1766,25 +1766,16 @@ void llvm::InvertBranch(BranchInst *PBI, IRBuilderBase &Builder) {
17661766
PBI->swapSuccessors();
17671767
}
17681768

1769-
template <typename... TermInst>
1770-
bool llvm::hasOnlyGivenTerminators(const Function &F) {
1769+
bool llvm::hasOnlySimpleTerminator(const Function &F) {
17711770
for (auto &BB : F) {
17721771
auto *Term = BB.getTerminator();
1773-
if (!(isa<TermInst>(Term) || ...))
1772+
if (!(isa<ReturnInst>(Term) || isa<UnreachableInst>(Term) ||
1773+
isa<BranchInst>(Term)))
17741774
return false;
17751775
}
17761776
return true;
17771777
}
17781778

1779-
bool llvm::hasOnlySimpleTerminator(const Function &F) {
1780-
return hasOnlyGivenTerminators<ReturnInst, UnreachableInst, BranchInst>(F);
1781-
}
1782-
1783-
bool llvm::hasOnlySimpleTerminatorOrCallBr(const Function &F) {
1784-
return hasOnlyGivenTerminators<ReturnInst, UnreachableInst, BranchInst,
1785-
CallBrInst>(F);
1786-
}
1787-
17881779
Printable llvm::printBBPtr(const BasicBlock *BB) {
17891780
return Printable([BB](raw_ostream &OS) {
17901781
if (BB)

llvm/lib/Transforms/Utils/ControlFlowUtils.cpp

Lines changed: 51 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,53 @@ std::pair<BasicBlock *, bool> ControlFlowHub::finalize(
346346
return {FirstGuardBlock, true};
347347
}
348348

349+
// Check if the given cycle is disjoint with the cycle of the given basic block.
350+
// If the basic block does not belong to any cycle, this is regarded as
351+
// disjoint, too.
352+
static bool disjointWithBlock(CycleInfo *CI, Cycle *C, BasicBlock *BB) {
353+
Cycle *CC = CI->getCycle(BB);
354+
if (!CC)
355+
return true;
356+
Cycle *CommonC = CI->getSmallestCommonCycle(C, CC);
357+
return CommonC != C && CommonC != CC;
358+
}
359+
360+
// Check if the given loop is disjoint with the loop of the given basic block.
361+
// If the basic block does not belong to any loop, this is regarded as
362+
// disjoint, too.
363+
static bool disjointWithBlock(LoopInfo *LI, Loop *L, BasicBlock *BB) {
364+
Loop *LL = LI->getLoopFor(BB);
365+
return LL && !L->contains(LL) && !LL->contains(L);
366+
}
367+
368+
template <typename TI, typename T>
369+
static void updateCycleLoopInfo(TI *LCI, bool AttachToCallBr,
370+
BasicBlock *CallBrBlock,
371+
BasicBlock *CallBrTarget, BasicBlock *Succ) {
372+
static_assert(std::is_same_v<TI, CycleInfo> || std::is_same_v<TI, LoopInfo>,
373+
"type must be CycleInfo or LoopInfo");
374+
if (!LCI)
375+
return;
376+
377+
T *LC;
378+
if constexpr (std::is_same_v<TI, CycleInfo>)
379+
LC = LCI->getCycle(AttachToCallBr ? CallBrBlock : Succ);
380+
else
381+
LC = LCI->getLoopFor(AttachToCallBr ? CallBrBlock : Succ);
382+
if (!LC)
383+
return;
384+
385+
// Check if the cycles/loops are disjoint. In that case, we do not add the
386+
// intermediate target to any cycle/loop.
387+
if (AttachToCallBr && disjointWithBlock(LCI, LC, Succ))
388+
return;
389+
390+
if constexpr (std::is_same_v<TI, CycleInfo>)
391+
LCI->addBlockToCycle(CallBrTarget, LC);
392+
else
393+
LC->addBasicBlockToLoop(CallBrTarget, *LCI);
394+
}
395+
349396
BasicBlock *ControlFlowHub::createCallBrTarget(
350397
CallBrInst *CallBr, BasicBlock *Succ, unsigned SuccIdx, bool AttachToCallBr,
351398
CycleInfo *CI, DomTreeUpdater *DTU, LoopInfo *LI) {
@@ -359,36 +406,10 @@ BasicBlock *ControlFlowHub::createCallBrTarget(
359406
CallBr->setSuccessor(SuccIdx, CallBrTarget);
360407
// Jump from the new target block to the original successor.
361408
BranchInst::Create(Succ, CallBrTarget);
362-
if (LI) {
363-
if (Loop *L = LI->getLoopFor(AttachToCallBr ? CallBrBlock : Succ)) {
364-
bool AddToLoop = true;
365-
if (AttachToCallBr) {
366-
// Check if the loops are disjoint. In that case, we do not add the
367-
// intermediate target to any loop.
368-
if (auto *LL = LI->getLoopFor(Succ);
369-
LL && !L->contains(LL) && !LL->contains(L))
370-
AddToLoop = false;
371-
}
372-
if (AddToLoop)
373-
L->addBasicBlockToLoop(CallBrTarget, *LI);
374-
}
375-
}
376-
if (CI) {
377-
if (auto *C = CI->getCycle(AttachToCallBr ? CallBrBlock : Succ); C) {
378-
bool AddToCycle = true;
379-
if (AttachToCallBr) {
380-
// Check if the cycles are disjoint. In that case, we do not add the
381-
// intermediate target to any cycle.
382-
if (auto *CC = CI->getCycle(Succ); CC) {
383-
auto *CommonC = CI->getSmallestCommonCycle(C, CC);
384-
if (CommonC != C && CommonC != CC)
385-
AddToCycle = false;
386-
}
387-
}
388-
if (AddToCycle)
389-
CI->addBlockToCycle(CallBrTarget, C);
390-
}
391-
}
409+
updateCycleLoopInfo<LoopInfo, Loop>(LI, AttachToCallBr, CallBrBlock,
410+
CallBrTarget, Succ);
411+
updateCycleLoopInfo<CycleInfo, Cycle>(CI, AttachToCallBr, CallBrBlock,
412+
CallBrTarget, Succ);
392413
if (DTU) {
393414
DTU->applyUpdates({{DominatorTree::Insert, CallBrBlock, CallBrTarget}});
394415
if (DTU->getDomTree().dominates(CallBrBlock, Succ))

llvm/lib/Transforms/Utils/FixIrreducible.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ static bool fixIrreducible(Cycle &C, CycleInfo &CI, DominatorTree &DT,
308308
if (Succ != Header)
309309
continue;
310310
BasicBlock *NewSucc = ControlFlowHub::createCallBrTarget(
311-
CallBr, Succ, I, false, &CI, &DTU, LI);
311+
CallBr, Succ, I, /* AttachToCallBr = */ false, &CI, &DTU, LI);
312312
CHub.addBranch(NewSucc, Succ);
313313
LLVM_DEBUG(dbgs() << "Added internal branch: " << printBBPtr(NewSucc)
314314
<< " -> " << printBBPtr(Succ) << "\n");
@@ -345,7 +345,7 @@ static bool fixIrreducible(Cycle &C, CycleInfo &CI, DominatorTree &DT,
345345
if (!C.contains(Succ))
346346
continue;
347347
BasicBlock *NewSucc = ControlFlowHub::createCallBrTarget(
348-
CallBr, Succ, I, true, &CI, &DTU, LI);
348+
CallBr, Succ, I, /* AttachToCallBr = */ true, &CI, &DTU, LI);
349349
CHub.addBranch(NewSucc, Succ);
350350
LLVM_DEBUG(dbgs() << "Added external branch: " << printBBPtr(NewSucc)
351351
<< " -> " << printBBPtr(Succ) << "\n");
@@ -400,8 +400,6 @@ static bool FixIrreducibleImpl(Function &F, CycleInfo &CI, DominatorTree &DT,
400400
LLVM_DEBUG(dbgs() << "===== Fix irreducible control-flow in function: "
401401
<< F.getName() << "\n");
402402

403-
assert(hasOnlySimpleTerminatorOrCallBr(F) && "Unsupported block terminator.");
404-
405403
bool Changed = false;
406404
for (Cycle *TopCycle : CI.toplevel_cycles()) {
407405
for (Cycle *C : depth_first(TopCycle)) {

llvm/lib/Transforms/Utils/UnifyLoopExits.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ static bool unifyLoopExits(DominatorTree &DT, LoopInfo &LI, Loop *L) {
179179
if (L->contains(Succ))
180180
continue;
181181
BasicBlock *NewSucc = ControlFlowHub::createCallBrTarget(
182-
CallBr, Succ, J, false, nullptr, &DTU, &LI);
182+
CallBr, Succ, J, /* AttachToCallBr = */ false, nullptr, &DTU, &LI);
183183
// ExitingBlocks is later used to restore SSA, so we need to make sure
184184
// that the blocks used for phi nodes in the guard blocks match the
185185
// predecessors of the guard blocks, which, in the case of callbr, are
@@ -219,7 +219,7 @@ static bool unifyLoopExits(DominatorTree &DT, LoopInfo &LI, Loop *L) {
219219
// members of the parent loop. Same goes for the callbr target blocks if they
220220
// have not already been added to the respective parent loop by adding them to
221221
// the original callbr target's loop.
222-
if (auto ParentLoop = L->getParentLoop()) {
222+
if (auto *ParentLoop = L->getParentLoop()) {
223223
for (auto *G : GuardBlocks) {
224224
ParentLoop->addBasicBlockToLoop(G, LI);
225225
}
@@ -254,8 +254,6 @@ bool UnifyLoopExitsLegacyPass::runOnFunction(Function &F) {
254254
auto &LI = getAnalysis<LoopInfoWrapperPass>().getLoopInfo();
255255
auto &DT = getAnalysis<DominatorTreeWrapperPass>().getDomTree();
256256

257-
assert(hasOnlySimpleTerminatorOrCallBr(F) && "Unsupported block terminator.");
258-
259257
return runImpl(LI, DT);
260258
}
261259

0 commit comments

Comments
 (0)