Skip to content

Commit 206b7c2

Browse files
authored
Remove unstructured loop exits: Don't introduce loops (microsoft#6676)
Previously, if the latch exit was reachable from a different exit block for the loop, then the pass would introduce a loop involving that exit block and the latch exit. This is unwanted and unaccounted for. - Add a test for shared exits. - Add a test for non-dedicated latch exit
1 parent 56f3c40 commit 206b7c2

File tree

3 files changed

+232
-0
lines changed

3 files changed

+232
-0
lines changed

lib/Transforms/Scalar/DxilRemoveUnstructuredLoopExits.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@
134134
#include "llvm/Support/Debug.h"
135135
#include "llvm/Support/raw_ostream.h"
136136
#include "llvm/Transforms/Scalar.h"
137+
#include "llvm/Transforms/Utils/BasicBlockUtils.h"
137138
#include "llvm/Transforms/Utils/Local.h"
138139
#include "llvm/Transforms/Utils/LoopUtils.h"
139140

@@ -466,6 +467,39 @@ static bool RemoveUnstructuredLoopExitsIteration(BasicBlock *exiting_block,
466467
DominatorTree *DT) {
467468
BasicBlock *latch = L->getLoopLatch();
468469
BasicBlock *latch_exit = GetExitBlockForExitingBlock(L, latch);
470+
471+
// Ensure the latch-exit is "dedicated": no block outside the loop
472+
// branches to it.
473+
//
474+
// Suppose this iteration successfully moves an exit block X until
475+
// after the latch block. It will do so by rewiring the CFG so
476+
// the latch *exit* block will branch to X. If the latch exit
477+
// block is already reachable from X, then the rewiring will
478+
// create an unwanted loop.
479+
// So prevent this from happening by ensuring the latch exit is
480+
// "dedicated": the only branches to it come from inside the
481+
// loop, and hence not from X.
482+
//
483+
// The latch_exit block could have *multiple* branches to it from
484+
// outside the loop.
485+
//
486+
// When the edge from latch to latch_exit is split, the local picture is:
487+
//
488+
// latch --> middle --> tail
489+
//
490+
// where:
491+
// - Branches that used to go to latch_exit, from outside the loop, now
492+
// point to 'tail'.
493+
// - 'middle' is now an exit block for the loop, and its only incoming
494+
// edge is from latch.
495+
for (auto *pred : predecessors(latch_exit)) {
496+
if (!L->contains(pred)) {
497+
SplitEdge(latch, latch_exit, DT, LI);
498+
// Quit early and recalculate exit blocks.
499+
return true;
500+
}
501+
}
502+
469503
BasicBlock *exit_block = GetExitBlockForExitingBlock(L, exiting_block);
470504

471505
// If exiting block already dominates latch, then no need to do anything.
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
; RUN: opt %s -analyze -loops | FileCheck -check-prefix=LOOPBEFORE %s
2+
; RUN: opt %s -dxil-remove-unstructured-loop-exits -o %t.bc
3+
; RUN: opt %t.bc -S | FileCheck %s
4+
; RUN: opt %t.bc -analyze -loops | FileCheck -check-prefix=LOOPAFTER %s
5+
6+
; Ensure the pass works when the latch exit is reachable from the exit block,
7+
; and does not introduce an extra loop.
8+
; If an exit block could reach the latch exit block, then an old version
9+
; of the pass would introduce a loop because after modification, the
10+
; latch-exit block would branch to the exit block (which has since been
11+
; repositioned in the CFG).
12+
13+
;
14+
; entry
15+
; |
16+
; v
17+
; +-> header ---> then --+
18+
; | | | |
19+
; | | +---------+ |
20+
; | | | v
21+
; | v v exit0
22+
; +--- latch |
23+
; | v
24+
; | exit1
25+
; | |
26+
; | v
27+
; | exit2
28+
; | |
29+
; | +--------------+
30+
; | |
31+
; v v
32+
; end # 'end' is the latch-exit block
33+
34+
; Before performing the loop exit restructuring, split the latch -> end edge, like this:
35+
;
36+
; entry
37+
; |
38+
; v
39+
; +-> header ---> then --+
40+
; | | | |
41+
; | | +---------+ |
42+
; | | | v
43+
; | v v exit0
44+
; +--- latch |
45+
; | v
46+
; | exit1
47+
; v |
48+
; latch.end_crit_edge v
49+
; | exit2
50+
; | |
51+
; | +--------------+
52+
; | |
53+
; v v
54+
; end
55+
56+
; Then it will be safe to rewire 'then' block as follows.
57+
; This achieves the goal of making all exiting blocks dominate
58+
; the latch. And crucially, a new loop is not created.
59+
;
60+
; entry
61+
; |
62+
; v
63+
; +-> header ---> then
64+
; | | |
65+
; | | +---------+
66+
; | | |
67+
; | v v
68+
; | dx.struct.new_exiting
69+
; | | |
70+
; | v |
71+
; +--- latch |
72+
; | +-----+
73+
; v v
74+
; latch.end_crit_edge --> exit0
75+
; | |
76+
; | v
77+
; | exit1
78+
; | |
79+
; | v
80+
; | exit2
81+
; | |
82+
; | +----------------+
83+
; | |
84+
; v v
85+
; end
86+
87+
; LOOPBEFORE: Loop at depth 1 containing: %header<header>,%then<exiting>,%latch<latch><exiting>
88+
; LOOPBEFORE-NOT: Loop at depth
89+
90+
; Don't create an extra loop
91+
; LOOPAFTER-NOT: Loop at depth {{.*}} containing: {{.*}}%end
92+
; LOOPAFTER-NOT: Loop at depth {{.*}} containing: {{.*}}%exit0
93+
; LOOPAFTER-NOT: Loop at depth {{.*}} containing: {{.*}}%exit1
94+
; LOOPAFTER-NOT: Loop at depth {{.*}} containing: {{.*}}%exit2
95+
; LOOPAFTER: Loop at depth 1 containing: %header<header>,%then,%dx.struct_exit.new_exiting<exiting>,%latch<latch><exiting>
96+
; LOOPAFTER-NOT: Loop at depth
97+
98+
target datalayout = "e-m:e-p:32:32-i1:32-i8:32-i16:32-i32:32-i64:64-f16:32-f32:32-f64:64-n8:16:32:64"
99+
target triple = "dxil-ms-dx"
100+
101+
define void @main(i1 %cond) {
102+
entry:
103+
br label %header
104+
105+
header:
106+
br i1 %cond, label %then, label %latch
107+
108+
then:
109+
br i1 %cond, label %exit0, label %latch
110+
111+
latch:
112+
br i1 %cond, label %end, label %header
113+
114+
; a long chain that eventually gets to %end
115+
exit0:
116+
br label %exit1
117+
exit1:
118+
br label %exit2
119+
exit2:
120+
br label %end
121+
122+
end:
123+
ret void
124+
}
125+
126+
; CHECK: define void @main
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
; RUN: opt %s -analyze -loops | FileCheck -check-prefix=LOOPBEFORE %s
2+
; RUN: opt %s -dxil-remove-unstructured-loop-exits -o %t.bc
3+
; RUN: opt %t.bc -S | FileCheck %s
4+
; RUN: opt %t.bc -analyze -loops | FileCheck -check-prefix=LOOPAFTER %s
5+
6+
; Two exiting blocks target the same exit block. This should work.
7+
; Also, the pass should not introduce a new loop, particularly not among
8+
; the latch-exiting blocks.
9+
10+
; LOOPBEFORE: Loop at depth 1 containing: %header<header>,%then.a<exiting>,%midloop,%then.b<exiting>,%latch<latch><exiting>
11+
; LOOPBEFORE-NOT: Loop at depth
12+
13+
; Don't create a loop containing: %end<header>,%0<exiting>,%shared_exit<latch>
14+
; LOOPAFTER-NOT: Loop at depth {{.*}} containing: {{.*}}%end
15+
; LOOPAFTER-NOT: Loop at depth {{.*}} containing: {{.*}}%0
16+
; LOOPAFTER-NOT: Loop at depth {{.*}} containing: {{.*}}%shared_exit
17+
; LOOPAFTER: Loop at depth 1 containing: %header<header>,%then.a,%dx.struct_exit.new_exiting<exiting>,%midloop,%then.b,%dx.struct_exit.new_exiting2<exiting>,%latch<latch><exiting>
18+
; LOOPAFTER-NOT: Loop at depth
19+
20+
target datalayout = "e-m:e-p:32:32-i1:32-i8:32-i16:32-i32:32-i64:64-f16:32-f32:32-f64:64-n8:16:32:64"
21+
target triple = "dxil-ms-dx"
22+
23+
; The input has two if-then-else structures in a row:
24+
; header/then.a/midloop
25+
; midloop/then.b/latch
26+
;
27+
; entry
28+
; |
29+
; v
30+
; +-> header ---> then.a ------+
31+
; | | | |
32+
; | | +-------+ |
33+
; | v v v
34+
; | midloop --> then.b --> shared_exit
35+
; | | | |
36+
; | | +-------+ |
37+
; | v v |
38+
; +--- latch |
39+
; | |
40+
; | +--------------------+
41+
; | |
42+
; v v
43+
; end
44+
45+
46+
define void @main(i1 %cond) {
47+
entry:
48+
br label %header
49+
50+
header:
51+
br i1 %cond, label %then.a, label %midloop
52+
53+
then.a:
54+
br i1 %cond, label %shared_exit, label %midloop
55+
56+
midloop:
57+
br i1 %cond, label %then.b, label %latch
58+
59+
then.b:
60+
br i1 %cond, label %shared_exit, label %latch
61+
62+
latch:
63+
br i1 %cond, label %end, label %header
64+
65+
shared_exit:
66+
br label %end
67+
68+
end:
69+
ret void
70+
}
71+
72+
; CHECK: define void @main

0 commit comments

Comments
 (0)