Skip to content

Commit eb8519e

Browse files
ssahasrassahasra
committed
refactor and address review comments
- The traansform now uses the improved control flow hub. - Remove explicit typecast for iterators. - When setting the new cycle entry, check that it is valid. - Update loop info only if it was already available. - New test when a child loop's header is an entry to parent cycle. - New test when a cycle contains a loop with the same header. - Renamed test when nested cycles have the same entry.
1 parent babfe4c commit eb8519e

File tree

9 files changed

+457
-299
lines changed

9 files changed

+457
-299
lines changed

‎llvm/include/llvm/ADT/GenericCycleInfo.h

+8-17
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ template <typename ContextT> class GenericCycle {
109109

110110
/// \brief Replace all entries with \p Block as single entry.
111111
void setSingleEntry(BlockT *Block) {
112+
assert(contains(Block));
112113
Entries.clear();
113114
Entries.push_back(Block);
114115
}
@@ -198,26 +199,16 @@ template <typename ContextT> class GenericCycle {
198199
//@{
199200
using const_entry_iterator =
200201
typename SmallVectorImpl<BlockT *>::const_iterator;
201-
const_entry_iterator entry_begin() const {
202-
return const_entry_iterator{Entries.begin()};
203-
}
204-
const_entry_iterator entry_end() const {
205-
return const_entry_iterator{Entries.end()};
206-
}
207-
208-
using const_reverse_entry_iterator =
209-
typename SmallVectorImpl<BlockT *>::const_reverse_iterator;
210-
const_reverse_entry_iterator entry_rbegin() const {
211-
return const_reverse_entry_iterator{Entries.rbegin()};
212-
}
213-
const_reverse_entry_iterator entry_rend() const {
214-
return const_reverse_entry_iterator{Entries.rend()};
215-
}
216-
202+
const_entry_iterator entry_begin() const { return Entries.begin(); }
203+
const_entry_iterator entry_end() const { return Entries.end(); }
217204
size_t getNumEntries() const { return Entries.size(); }
218205
iterator_range<const_entry_iterator> entries() const {
219-
return llvm::make_range(Entries.begin(), Entries.end());
206+
return llvm::make_range(entry_begin(), entry_end());
220207
}
208+
using const_reverse_entry_iterator =
209+
typename SmallVectorImpl<BlockT *>::const_reverse_iterator;
210+
const_reverse_entry_iterator entry_rbegin() const { return Entries.rbegin(); }
211+
const_reverse_entry_iterator entry_rend() const { return Entries.rend(); }
221212
//@}
222213

223214
Printable printEntries(const ContextT &Ctx) const {

‎llvm/lib/Transforms/Utils/FixIrreducible.cpp

+160-97
Original file line numberDiff line numberDiff line change
@@ -6,23 +6,7 @@
66
//
77
//===----------------------------------------------------------------------===//
88
//
9-
// To convert an irreducible cycle C to a natural loop L:
10-
//
11-
// 1. Add a new node N to C.
12-
// 2. Redirect all external incoming edges through N.
13-
// 3. Redirect all edges incident on header H through N.
14-
//
15-
// This is sufficient to ensure that:
16-
//
17-
// a. Every closed path in C also exists in L, with the modification that any
18-
// path passing through H now passes through N before reaching H.
19-
// b. Every external path incident on any entry of C is now incident on N and
20-
// then redirected to the entry.
21-
//
22-
// Thus, L is a strongly connected component dominated by N, and hence L is a
23-
// natural loop with header N.
24-
//
25-
// INPUT CFG: The blocks H and B form an irreducible loop with two headers.
9+
// INPUT CFG: The blocks H and B form an irreducible cycle with two headers.
2610
//
2711
// Entry
2812
// / \
@@ -33,7 +17,7 @@
3317
// v
3418
// Exit
3519
//
36-
// OUTPUT CFG:
20+
// OUTPUT CFG: Converted to a natural loop with a new header N.
3721
//
3822
// Entry
3923
// |
@@ -47,16 +31,50 @@
4731
// v
4832
// Exit
4933
//
34+
// To convert an irreducible cycle C to a natural loop L:
35+
//
36+
// 1. Add a new node N to C.
37+
// 2. Redirect all external incoming edges through N.
38+
// 3. Redirect all edges incident on header H through N.
39+
//
40+
// This is sufficient to ensure that:
41+
//
42+
// a. Every closed path in C also exists in L, with the modification that any
43+
// path passing through H now passes through N before reaching H.
44+
// b. Every external path incident on any entry of C is now incident on N and
45+
// then redirected to the entry.
46+
//
47+
// Thus, L is a strongly connected component dominated by N, and hence L is a
48+
// natural loop with header N.
49+
//
50+
// When an irreducible cycle C with header H is transformed into a loop, the
51+
// following invariants hold:
52+
//
53+
// 1. No new subcycles are "discovered" in the set (C-H). The only internal
54+
// edges that are redirected by the transform are incident on H. Any subcycle
55+
// S in (C-H), already existed prior to this transform, and is already in the
56+
// list of children for this cycle C.
5057
//
51-
// The actual transformation is handled by function CreateControlFlowHub, which
52-
// takes a set of incoming blocks (the predecessors) and outgoing blocks (the
53-
// entries). The function also moves every PHINode in an outgoing block to the
54-
// hub. Since the hub dominates all the outgoing blocks, each such PHINode
55-
// continues to dominate its uses. Since every entry the cycle has at least two
56-
// predecessors, every value used in the entry (or later) but defined in a
57-
// predecessor (or earlier) is represented by a PHINode in a entry. Hence the
58-
// above handling of PHINodes is sufficient and no further processing is
59-
// required to restore SSA.
58+
// 2. Subcycles of C are not modified by the transform. For some subcycle S of
59+
// C, edges incident on the entries of S are either internal to C, or they
60+
// are now redirected through N, which is outside of S. So the list of
61+
// entries to S does not change. Since the transform only adds a block
62+
// outside S, and redirects edges that are not internal to S, the list of
63+
// blocks in S does not change.
64+
//
65+
// 3. Similarly, any natural loop L included in C is not affected, with one
66+
// exception: L is "destroyed" by the transform iff its header is H. The
67+
// backedges of such a loop are now redirected to N instead, and hence the
68+
// body of this loop gets merged into the new loop with header N.
69+
//
70+
// The actual transformation is handled by the ControlFlowHub, which redirects
71+
// specified control flow edges through a set of guard blocks. This also moves
72+
// every PHINode in an outgoing block to the hub. Since the hub dominates all
73+
// the outgoing blocks, each such PHINode continues to dominate its uses. Since
74+
// every header in an SCC has at least two predecessors, every value used in the
75+
// header (or later) but defined in a predecessor (or earlier) is represented by
76+
// a PHINode in a header. Hence the above handling of PHINodes is sufficient and
77+
// no further processing is required to restore SSA.
6078
//
6179
// Limitation: The pass cannot handle switch statements and indirect
6280
// branches. Both must be lowered to plain branches first.
@@ -119,8 +137,9 @@ static void reconnectChildLoops(LoopInfo &LI, Loop *ParentLoop, Loop *NewLoop,
119137
// Any candidate is a child iff its header is owned by the new loop. Move all
120138
// the children to a new vector.
121139
auto FirstChild = std::partition(
122-
CandidateLoops.begin(), CandidateLoops.end(),
123-
[&](Loop *L) { return !NewLoop->contains(L->getHeader()); });
140+
CandidateLoops.begin(), CandidateLoops.end(), [&](Loop *L) {
141+
return NewLoop == L || !NewLoop->contains(L->getHeader());
142+
});
124143
SmallVector<Loop *, 8> ChildLoops(FirstChild, CandidateLoops.end());
125144
CandidateLoops.erase(FirstChild, CandidateLoops.end());
126145

@@ -154,57 +173,127 @@ static void reconnectChildLoops(LoopInfo &LI, Loop *ParentLoop, Loop *NewLoop,
154173
}
155174
}
156175

176+
static void updateLoopInfo(LoopInfo &LI, Cycle &C,
177+
ArrayRef<BasicBlock *> GuardBlocks) {
178+
// The parent loop is a natural loop L mapped to the cycle header H as long as
179+
// H is not also the header of L. In the latter case, L is destroyed and we
180+
// seek its parent instead.
181+
BasicBlock *CycleHeader = C.getHeader();
182+
Loop *ParentLoop = LI.getLoopFor(CycleHeader);
183+
if (ParentLoop && ParentLoop->getHeader() == CycleHeader)
184+
ParentLoop = ParentLoop->getParentLoop();
185+
186+
// Create a new loop from the now-transformed cycle
187+
auto *NewLoop = LI.AllocateLoop();
188+
if (ParentLoop) {
189+
ParentLoop->addChildLoop(NewLoop);
190+
} else {
191+
LI.addTopLevelLoop(NewLoop);
192+
}
193+
194+
// Add the guard blocks to the new loop. The first guard block is
195+
// the head of all the backedges, and it is the first to be inserted
196+
// in the loop. This ensures that it is recognized as the
197+
// header. Since the new loop is already in LoopInfo, the new blocks
198+
// are also propagated up the chain of parent loops.
199+
for (auto *G : GuardBlocks) {
200+
LLVM_DEBUG(dbgs() << "added guard block to loop: " << G->getName() << "\n");
201+
NewLoop->addBasicBlockToLoop(G, LI);
202+
}
203+
204+
for (auto *BB : C.blocks()) {
205+
NewLoop->addBlockEntry(BB);
206+
if (LI.getLoopFor(BB) == ParentLoop) {
207+
LLVM_DEBUG(dbgs() << "moved block from parent: " << BB->getName()
208+
<< "\n");
209+
LI.changeLoopFor(BB, NewLoop);
210+
} else {
211+
LLVM_DEBUG(dbgs() << "added block from child: " << BB->getName() << "\n");
212+
}
213+
}
214+
LLVM_DEBUG(dbgs() << "header for new loop: "
215+
<< NewLoop->getHeader()->getName() << "\n");
216+
217+
reconnectChildLoops(LI, ParentLoop, NewLoop, C.getHeader());
218+
219+
LLVM_DEBUG(dbgs() << "Verify new loop.\n"; NewLoop->print(dbgs()));
220+
NewLoop->verifyLoop();
221+
if (ParentLoop) {
222+
LLVM_DEBUG(dbgs() << "Verify parent loop.\n"; ParentLoop->print(dbgs()));
223+
ParentLoop->verifyLoop();
224+
}
225+
}
226+
157227
// Given a set of blocks and headers in an irreducible SCC, convert it into a
158228
// natural loop. Also insert this new loop at its appropriate place in the
159229
// hierarchy of loops.
160230
static bool fixIrreducible(Cycle &C, CycleInfo &CI, DominatorTree &DT,
161231
LoopInfo *LI) {
162232
if (C.isReducible())
163233
return false;
234+
LLVM_DEBUG(dbgs() << "Processing cycle:\n" << CI.print(&C) << "\n";);
164235

236+
ControlFlowHub CHub;
165237
SetVector<BasicBlock *> Predecessors;
166238

167239
// Redirect internal edges incident on the header.
168-
BasicBlock *OldHeader = C.getHeader();
169-
for (BasicBlock *P : predecessors(OldHeader)) {
240+
BasicBlock *Header = C.getHeader();
241+
for (BasicBlock *P : predecessors(Header)) {
170242
if (C.contains(P))
171243
Predecessors.insert(P);
172244
}
173245

246+
for (BasicBlock *P : Predecessors) {
247+
auto *Branch = cast<BranchInst>(P->getTerminator());
248+
// Exactly one of the two successors is the header.
249+
BasicBlock *Succ0 = Branch->getSuccessor(0) == Header ? Header : nullptr;
250+
BasicBlock *Succ1 = Succ0 ? nullptr : Header;
251+
if (!Succ0)
252+
assert(Branch->getSuccessor(1) == Header);
253+
assert(Succ0 || Succ1);
254+
CHub.addBranch(P, Succ0, Succ1);
255+
256+
LLVM_DEBUG(dbgs() << "Added internal branch: " << P->getName() << " -> "
257+
<< (Succ0 ? Succ0->getName() : "") << " "
258+
<< (Succ1 ? Succ1->getName() : "") << "\n");
259+
}
260+
174261
// Redirect external incoming edges. This includes the edges on the header.
262+
Predecessors.clear();
175263
for (BasicBlock *E : C.entries()) {
176264
for (BasicBlock *P : predecessors(E)) {
177265
if (!C.contains(P))
178266
Predecessors.insert(P);
179267
}
180268
}
181269

182-
LLVM_DEBUG(
183-
dbgs() << "Found predecessors:";
184-
for (auto P : Predecessors) {
185-
dbgs() << " " << P->getName();
186-
}
187-
dbgs() << "\n");
188-
189-
// Redirect all the backedges through a "hub" consisting of a series
190-
// of guard blocks that manage the flow of control from the
191-
// predecessors to the headers.
192-
ControlFlowHub CHub;
193270
for (BasicBlock *P : Predecessors) {
194271
auto *Branch = cast<BranchInst>(P->getTerminator());
195272
BasicBlock *Succ0 = Branch->getSuccessor(0);
196-
Succ0 = Headers.count(Succ0) ? Succ0 : nullptr;
273+
Succ0 = C.contains(Succ0) ? Succ0 : nullptr;
197274
BasicBlock *Succ1 =
198275
Branch->isUnconditional() ? nullptr : Branch->getSuccessor(1);
199-
Succ1 = Succ1 && Headers.count(Succ1) ? Succ1 : nullptr;
276+
Succ1 = Succ1 && C.contains(Succ1) ? Succ1 : nullptr;
200277
CHub.addBranch(P, Succ0, Succ1);
201278

202-
LLVM_DEBUG(dbgs() << "Added branch: " << P->getName() << " -> "
279+
LLVM_DEBUG(dbgs() << "Added external branch: " << P->getName() << " -> "
203280
<< (Succ0 ? Succ0->getName() : "") << " "
204281
<< (Succ1 ? Succ1->getName() : "") << "\n");
205282
}
206283

207-
SmallVector<BasicBlock *, 8> GuardBlocks;
284+
// Redirect all the backedges through a "hub" consisting of a series
285+
// of guard blocks that manage the flow of control from the
286+
// predecessors to the headers.
287+
SmallVector<BasicBlock *> GuardBlocks;
288+
289+
// Minor optimization: The cycle entries are discovered in an order that is
290+
// the opposite of the order in which these blocks appear as branch targets.
291+
// This results in a lot of condition inversions in the control flow out of
292+
// the new ControlFlowHub, which can be mitigated if the orders match. So we
293+
// reverse the entries when adding them to the hub.
294+
SetVector<BasicBlock *> Entries;
295+
Entries.insert(C.entry_rbegin(), C.entry_rend());
296+
208297
DomTreeUpdater DTU(DT, DomTreeUpdater::UpdateStrategy::Eager);
209298
CHub.finalize(&DTU, GuardBlocks, "irr");
210299
#if defined(EXPENSIVE_CHECKS)
@@ -213,57 +302,23 @@ static bool fixIrreducible(Cycle &C, CycleInfo &CI, DominatorTree &DT,
213302
assert(DT.verify(DominatorTree::VerificationLevel::Fast));
214303
#endif
215304

305+
// If we are updating LoopInfo, do that now before modifying the cycle. This
306+
// ensures that the first guard block is the header of a new natural loop.
307+
if (LI)
308+
updateLoopInfo(*LI, C, GuardBlocks);
309+
216310
for (auto *G : GuardBlocks) {
217-
LLVM_DEBUG(dbgs() << "added guard block: " << G->getName() << "\n");
311+
LLVM_DEBUG(dbgs() << "added guard block to cycle: " << G->getName()
312+
<< "\n");
218313
CI.addBlockToCycle(G, &C);
219314
}
220315
C.setSingleEntry(GuardBlocks[0]);
221316

222-
if (!LI)
223-
return true;
224-
225-
Loop *ParentLoop = LI->getLoopFor(OldHeader);
226-
// Create a new loop from the now-transformed cycle
227-
auto *NewLoop = LI->AllocateLoop();
228-
if (ParentLoop) {
229-
ParentLoop->addChildLoop(NewLoop);
230-
} else {
231-
LI->addTopLevelLoop(NewLoop);
232-
}
233-
234-
// Add the guard blocks to the new loop. The first guard block is
235-
// the head of all the backedges, and it is the first to be inserted
236-
// in the loop. This ensures that it is recognized as the
237-
// header. Since the new loop is already in LoopInfo, the new blocks
238-
// are also propagated up the chain of parent loops.
239-
for (auto *G : GuardBlocks) {
240-
LLVM_DEBUG(dbgs() << "added guard block: " << G->getName() << "\n");
241-
NewLoop->addBasicBlockToLoop(G, *LI);
242-
}
243-
244-
for (auto *BB : C.blocks()) {
245-
NewLoop->addBlockEntry(BB);
246-
if (LI->getLoopFor(BB) == ParentLoop) {
247-
LLVM_DEBUG(dbgs() << "moved block from parent: " << BB->getName()
248-
<< "\n");
249-
LI->changeLoopFor(BB, NewLoop);
250-
} else {
251-
LLVM_DEBUG(dbgs() << "added block from child: " << BB->getName() << "\n");
252-
}
253-
}
254-
LLVM_DEBUG(dbgs() << "header for new loop: "
255-
<< NewLoop->getHeader()->getName() << "\n");
256-
257-
reconnectChildLoops(*LI, ParentLoop, NewLoop, OldHeader);
258-
259-
NewLoop->verifyLoop();
260-
if (ParentLoop) {
261-
ParentLoop->verifyLoop();
262-
}
263-
#if defined(EXPENSIVE_CHECKS)
264-
LI->verify(DT);
265-
#endif // EXPENSIVE_CHECKS
317+
C.verifyCycle();
318+
if (Cycle *Parent = C.getParentCycle())
319+
Parent->verifyCycle();
266320

321+
LLVM_DEBUG(dbgs() << "Finished one cycle:\n"; CI.print(dbgs()););
267322
return true;
268323
}
269324

@@ -275,15 +330,23 @@ static bool FixIrreducibleImpl(Function &F, CycleInfo &CI, DominatorTree &DT,
275330
assert(hasOnlySimpleTerminator(F) && "Unsupported block terminator.");
276331

277332
bool Changed = false;
278-
SmallVector<Cycle *> Worklist{CI.toplevel_cycles()};
333+
for (Cycle *TopCycle : CI.toplevel_cycles()) {
334+
for (Cycle *C : depth_first(TopCycle)) {
335+
Changed |= fixIrreducible(*C, CI, DT, LI);
336+
}
337+
}
338+
339+
if (!Changed)
340+
return false;
279341

280-
while (!Worklist.empty()) {
281-
Cycle *C = Worklist.pop_back_val();
282-
Changed |= fixIrreducible(*C, CI, DT, LI);
283-
append_range(Worklist, C->children());
342+
#if defined(EXPENSIVE_CHECKS)
343+
CI.verify();
344+
if (LI) {
345+
LI.verify(DT);
284346
}
347+
#endif // EXPENSIVE_CHECKS
285348

286-
return Changed;
349+
return true;
287350
}
288351

289352
bool FixIrreducible::runOnFunction(Function &F) {

0 commit comments

Comments
 (0)