Skip to content

Commit d869913

Browse files
kuharkuhar
committed
[Dominators] Teach LoopDeletion to use the new incremental API
Summary: This patch makes LoopDeletion use the incremental DominatorTree API. We modify LoopDeletion to perform the deletion in 5 steps: 1. Create a new dummy edge from the preheader to the exit, by adding a conditional branch. 2. Inform the DomTree about the new edge. 3. Remove the conditional branch and replace it with an unconditional edge to the exit. This removes the edge to the loop header, making it unreachable. 4. Inform the DomTree about the deleted edge. 5. Remove the unreachable block from the function. Creating the dummy conditional branch is necessary to perform incremental DomTree update. We should consider using the batch updater when it's ready. Reviewers: dberlin, davide, grosser, sanjoy Reviewed By: dberlin, grosser Subscribers: mzolotukhin, llvm-commits Differential Revision: https://reviews.llvm.org/D35391 llvm-svn: 309850
1 parent 0bd906e commit d869913

File tree

4 files changed

+136
-25
lines changed

4 files changed

+136
-25
lines changed

‎llvm/include/llvm/Support/GenericDomTree.h

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -481,11 +481,15 @@ class DominatorTreeBase {
481481

482482
/// Inform the dominator tree about a CFG edge deletion and update the tree.
483483
///
484-
/// This function has to be called just after making the update
485-
/// on the actual CFG. There cannot be any other updates that the dominator
486-
/// tree doesn't know about. The only exception is when the deletion that the
487-
/// tree is informed about makes some (dominator) subtree unreachable -- in
488-
/// this case, it is fine to perform deletions within this subtree.
484+
/// This function has to be called just after making the update on the actual
485+
/// CFG. An internal functions checks if the edge doesn't exist in the CFG in
486+
/// DEBUG mode. There cannot be any other updates that the
487+
/// dominator tree doesn't know about.
488+
///
489+
/// However, it is fine to perform multiple CFG deletions that make different
490+
/// subtrees forward-unreachable and to inform the DomTree about them all at
491+
/// the same time, as the incremental algorithm doesn't walk the tree above
492+
/// the NearestCommonDominator of a deleted edge
489493
///
490494
/// Note that for postdominators it automatically takes care of deleting
491495
/// a reverse edge internally (so there's no need to swap the parameters).

‎llvm/lib/Transforms/Scalar/LoopDeletion.cpp

Lines changed: 41 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -239,17 +239,44 @@ static void deleteDeadLoop(Loop *L, DominatorTree &DT, ScalarEvolution &SE,
239239

240240
auto *ExitBlock = L->getUniqueExitBlock();
241241
assert(ExitBlock && "Should have a unique exit block!");
242-
243242
assert(L->hasDedicatedExits() && "Loop should have dedicated exits!");
244243

245-
// Connect the preheader directly to the exit block.
244+
auto *OldBr = dyn_cast<BranchInst>(Preheader->getTerminator());
245+
assert(OldBr && "Preheader must end with a branch");
246+
assert(OldBr->isUnconditional() && "Preheader must have a single successor");
247+
// Connect the preheader to the exit block. Keep the old edge to the header
248+
// around to perform the dominator tree update in two separate steps
249+
// -- #1 insertion of the edge preheader -> exit and #2 deletion of the edge
250+
// preheader -> header.
251+
//
252+
//
253+
// 0. Preheader 1. Preheader 2. Preheader
254+
// | | | |
255+
// V | V |
256+
// Header <--\ | Header <--\ | Header <--\
257+
// | | | | | | | | | | |
258+
// | V | | | V | | | V |
259+
// | Body --/ | | Body --/ | | Body --/
260+
// V V V V V
261+
// Exit Exit Exit
262+
//
263+
// By doing this is two separate steps we can perform the dominator tree
264+
// update without using the batch update API.
265+
//
246266
// Even when the loop is never executed, we cannot remove the edge from the
247267
// source block to the exit block. Consider the case where the unexecuted loop
248268
// branches back to an outer loop. If we deleted the loop and removed the edge
249269
// coming to this inner loop, this will break the outer loop structure (by
250270
// deleting the backedge of the outer loop). If the outer loop is indeed a
251271
// non-loop, it will be deleted in a future iteration of loop deletion pass.
252-
Preheader->getTerminator()->replaceUsesOfWith(L->getHeader(), ExitBlock);
272+
IRBuilder<> Builder(OldBr);
273+
Builder.CreateCondBr(Builder.getFalse(), L->getHeader(), ExitBlock);
274+
// Remove the old branch. The conditional branch becomes a new terminator.
275+
OldBr->eraseFromParent();
276+
277+
// Update the dominator tree by informing it about the new edge from the
278+
// preheader to the exit.
279+
DT.insertEdge(Preheader, ExitBlock);
253280

254281
// Rewrite phis in the exit block to get their inputs from the Preheader
255282
// instead of the exiting block.
@@ -276,25 +303,19 @@ static void deleteDeadLoop(Loop *L, DominatorTree &DT, ScalarEvolution &SE,
276303
++BI;
277304
}
278305

279-
// Update the dominator tree and remove the instructions and blocks that will
280-
// be deleted from the reference counting scheme.
281-
SmallVector<DomTreeNode*, 8> ChildNodes;
282-
for (Loop::block_iterator LI = L->block_begin(), LE = L->block_end();
283-
LI != LE; ++LI) {
284-
// Move all of the block's children to be children of the Preheader, which
285-
// allows us to remove the domtree entry for the block.
286-
ChildNodes.insert(ChildNodes.begin(), DT[*LI]->begin(), DT[*LI]->end());
287-
for (DomTreeNode *ChildNode : ChildNodes) {
288-
DT.changeImmediateDominator(ChildNode, DT[Preheader]);
289-
}
306+
// Disconnect the loop body by branching directly to its exit.
307+
Builder.SetInsertPoint(Preheader->getTerminator());
308+
Builder.CreateBr(ExitBlock);
309+
// Remove the old branch.
310+
Preheader->getTerminator()->eraseFromParent();
290311

291-
ChildNodes.clear();
292-
DT.eraseNode(*LI);
312+
// Inform the dominator tree about the removed edge.
313+
DT.deleteEdge(Preheader, L->getHeader());
293314

294-
// Remove the block from the reference counting scheme, so that we can
295-
// delete it freely later.
296-
(*LI)->dropAllReferences();
297-
}
315+
// Remove the block from the reference counting scheme, so that we can
316+
// delete it freely later.
317+
for (auto *Block : L->blocks())
318+
Block->dropAllReferences();
298319

299320
// Erase the instructions and the blocks without having to worry
300321
// about ordering because we already dropped the references.

‎llvm/test/Transforms/LoopDeletion/2017-07-11-incremental-dt.ll

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
; RUN: opt < %s -loop-deletion -S
2+
; RUN: opt < %s -loop-deletion -analyze -domtree 2>&1 | FileCheck -check-prefix=DT %s
3+
; RUN: opt < %s -loop-deletion -analyze -verify-dom-info
4+
5+
; CHECK: for.body
6+
; CHECK-NOT: for.cond1
7+
8+
; Verify only the important parts of the DomTree.
9+
; DT: [1] %entry
10+
; DT: [2] %for.cond
11+
; DT: [3] %lbl63A679E5
12+
; DT: [3] %for.cond9
13+
; DT: [3] %lbl64774A9B
14+
; DT: [3] %for.body
15+
; DT: [4] %for.cond3.loopexit
16+
17+
define i32 @fn1() {
18+
entry:
19+
br label %for.cond
20+
21+
for.cond: ; preds = %entry
22+
br i1 undef, label %lbl63A679E5, label %for.body
23+
24+
for.body: ; preds = %for.cond
25+
br label %for.cond1
26+
27+
for.cond1: ; preds = %for.cond1, %for.body
28+
br i1 undef, label %for.cond1, label %for.cond3.loopexit
29+
30+
for.cond3.loopexit: ; preds = %for.cond1
31+
br label %for.cond3
32+
33+
for.cond3: ; preds = %for.cond9, %for.cond3.loopexit
34+
br i1 undef, label %for.body4, label %for.cond17
35+
36+
for.body4: ; preds = %for.cond3
37+
br label %for.cond5
38+
39+
for.cond5: ; preds = %lbl63A679E5, %for.body4
40+
br label %for.cond9
41+
42+
lbl63A679E5: ; preds = %for.cond
43+
br label %for.cond5
44+
45+
for.cond9: ; preds = %for.end14.split, %for.cond5
46+
br i1 undef, label %for.cond3, label %lbl64774A9B
47+
48+
lbl64774A9B: ; preds = %for.cond17, %for.cond9
49+
br label %for.end14.split
50+
51+
for.end14.split: ; preds = %lbl64774A9B
52+
br label %for.cond9
53+
54+
for.cond17: ; preds = %for.cond3
55+
br label %lbl64774A9B
56+
}

‎llvm/unittests/IR/DominatorTreeTest.cpp

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -797,6 +797,36 @@ TEST(DominatorTree, DeleteUnreachable) {
797797
}
798798
}
799799

800+
TEST(DominatorTree, DeletionsInSubtrees) {
801+
CFGHolder Holder;
802+
std::vector<CFGBuilder::Arc> Arcs = {{"0", "1"}, {"1", "2"}, {"1", "3"},
803+
{"1", "6"}, {"3", "4"}, {"2", "5"},
804+
{"5", "2"}};
805+
806+
// It is possible to perform multiple deletions and inform the
807+
// DominatorTree about them at the same time, if the all of the
808+
// deletions happen in different subtrees.
809+
std::vector<CFGBuilder::Update> Updates = {{Delete, {"1", "2"}},
810+
{Delete, {"1", "3"}}};
811+
CFGBuilder B(Holder.F, Arcs, Updates);
812+
DominatorTree DT(*Holder.F);
813+
EXPECT_TRUE(DT.verify());
814+
815+
Optional<CFGBuilder::Update> LastUpdate;
816+
while ((LastUpdate = B.applyUpdate()))
817+
;
818+
819+
DT.deleteEdge(B.getOrAddBlock("1"), B.getOrAddBlock("2"));
820+
DT.deleteEdge(B.getOrAddBlock("1"), B.getOrAddBlock("3"));
821+
822+
EXPECT_TRUE(DT.verify());
823+
EXPECT_EQ(DT.getNode(B.getOrAddBlock("2")), nullptr);
824+
EXPECT_EQ(DT.getNode(B.getOrAddBlock("3")), nullptr);
825+
EXPECT_EQ(DT.getNode(B.getOrAddBlock("4")), nullptr);
826+
EXPECT_EQ(DT.getNode(B.getOrAddBlock("5")), nullptr);
827+
EXPECT_NE(DT.getNode(B.getOrAddBlock("6")), nullptr);
828+
}
829+
800830
TEST(DominatorTree, InsertDelete) {
801831
std::vector<CFGBuilder::Arc> Arcs = {
802832
{"1", "2"}, {"2", "3"}, {"3", "4"}, {"4", "5"}, {"5", "6"}, {"5", "7"},

0 commit comments

Comments
 (0)