Skip to content

Commit

Permalink
Remove unstructured loop exits: Don't introduce loops
Browse files Browse the repository at this point in the history
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
  • Loading branch information
dneto0 committed Jun 6, 2024
1 parent 8206fbd commit 260a19c
Show file tree
Hide file tree
Showing 3 changed files with 218 additions and 0 deletions.
20 changes: 20 additions & 0 deletions lib/Transforms/Scalar/DxilRemoveUnstructuredLoopExits.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,26 @@ static bool RemoveUnstructuredLoopExitsIteration(BasicBlock *exiting_block,
}

BasicBlock *latch_exit = GetExitBlockForExitingBlock(L, latch);

// Ensure the latch-exit is "dedicated": no block outside the loop
// branches to it.
//
// Suppose this iteration successfully moves an exit block X until
// after the latch block. It will do so by rewiring the CFG so
// the latch *exit* block will branch to X. If the latch exit
// block is already reachable from X, then the rewiring will
// create an unwanted loop.
// So prevent this from happening by ensuring the latch exit is
// "dedicated": the only branches to it come from inside the
// loop, and hence not from X.
for (auto *pred : predecessors(latch_exit)) {
if (!L->contains(pred)) {
SplitEdge(latch, latch_exit, DT, LI);
// Quit early and recalculate exit blocks.
return true;
}
}

BasicBlock *exit_block = GetExitBlockForExitingBlock(L, exiting_block);

// If exiting block already dominates latch, then no need to do anything.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
; RUN: opt %s -analyze -loops | FileCheck -check-prefix=LOOPBEFORE %s
; RUN: opt %s -dxil-remove-unstructured-loop-exits -o %t.bc
; RUN: opt %t.bc -S | FileCheck %s
; RUN: opt %t.bc -analyze -loops | FileCheck -check-prefix=LOOPAFTER %s

; Ensure the pass works when the latch exit is reachable from the exit block,
; and does not introduce an extra loop.
; If an exit block could reach the latch exit block, then an old version
; of the pass would introduce a loop because after modification, the
; latch-exit block would branch to the exit block (which has since been
; repositioned in the CFG).

;
; entry
; |
; v
; +-> header ---> then --+
; | | | |
; | | +---------+ |
; | | | v
; | v v exit0
; +--- latch |
; | v
; | exit1
; | |
; | v
; | exit2
; | |
; | +--------------+
; | |
; v v
; end # 'end' is the latch-exit block

; Before performing the loop exit restructuring, split the latch -> end edge, like this:
;
; entry
; |
; v
; +-> header ---> then --+
; | | | |
; | | +---------+ |
; | | | v
; | v v exit0
; +--- latch |
; | v
; | exit1
; v |
; latch.end_crit_edge v
; | exit2
; | |
; | +--------------+
; | |
; v v
; end

; Then it will be safe to rewire 'then' block as follows.
; This achieves the goal of making all exiting blocks dominate
; the latch. And crucially, a new loop is not created.
;
; entry
; |
; v
; +-> header ---> then
; | | |
; | | +---------+
; | | |
; | v v
; | dx.struct.new_exiting
; | | |
; | v |
; +--- latch |
; | +-----+
; v v
; latch.end_crit_edge --> exit0
; | |
; | v
; | exit1
; | |
; | v
; | exit2
; | |
; | +----------------+
; | |
; v v
; end

; LOOPBEFORE: Loop at depth 1 containing: %header<header>,%then<exiting>,%latch<latch><exiting>
; LOOPBEFORE-NOT: Loop at depth

; Don't create an extra loop
; LOOPAFTER-NOT: Loop at depth {{.*}} containing: {{.*}}%end
; LOOPAFTER-NOT: Loop at depth {{.*}} containing: {{.*}}%exit0
; LOOPAFTER-NOT: Loop at depth {{.*}} containing: {{.*}}%exit1
; LOOPAFTER-NOT: Loop at depth {{.*}} containing: {{.*}}%exit2
; LOOPAFTER: Loop at depth 1 containing: %header<header>,%then,%dx.struct_exit.new_exiting<exiting>,%latch<latch><exiting>
; LOOPAFTER-NOT: Loop at depth

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"
target triple = "dxil-ms-dx"

define void @main(i1 %cond) {
entry:
br label %header

header:
br i1 %cond, label %then, label %latch

then:
br i1 %cond, label %exit0, label %latch

latch:
br i1 %cond, label %end, label %header

; a long chain that eventually gets to %end
exit0:
br label %exit1
exit1:
br label %exit2
exit2:
br label %end

end:
ret void
}

; CHECK: define void @main
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
; RUN: opt %s -analyze -loops | FileCheck -check-prefix=LOOPBEFORE %s
; RUN: opt %s -dxil-remove-unstructured-loop-exits -o %t.bc
; RUN: opt %t.bc -S | FileCheck %s
; RUN: opt %t.bc -analyze -loops | FileCheck -check-prefix=LOOPAFTER %s

; Two exiting blocks target the same exit block. This should work.
; Also, the pass should not introduce a new loop, particularly not among
; the latch-exiting blocks.

; LOOPBEFORE: Loop at depth 1 containing: %header<header>,%then.a<exiting>,%midloop,%then.b<exiting>,%latch<latch><exiting>
; LOOPBEFORE-NOT: Loop at depth

; Don't create a loop containing: %end<header>,%0<exiting>,%shared_exit<latch>
; LOOPAFTER-NOT: Loop at depth {{.*}} containing: {{.*}}%end
; LOOPAFTER-NOT: Loop at depth {{.*}} containing: {{.*}}%0
; LOOPAFTER-NOT: Loop at depth {{.*}} containing: {{.*}}%shared_exit
; 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>
; LOOPAFTER-NOT: Loop at depth

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"
target triple = "dxil-ms-dx"

; The input has two if-then-else structures in a row:
; header/then.a/midloop
; midloop/then.b/latch
;
; entry
; |
; v
; +-> header ---> then.a ------+
; | | | |
; | | +-------+ |
; | v v v
; | midloop --> then.b --> shared_exit
; | | | |
; | | +-------+ |
; | v v |
; +--- latch |
; | |
; | +--------------------+
; | |
; v v
; end


define void @main(i1 %cond) {
entry:
br label %header

header:
br i1 %cond, label %then.a, label %midloop

then.a:
br i1 %cond, label %shared_exit, label %midloop

midloop:
br i1 %cond, label %then.b, label %latch

then.b:
br i1 %cond, label %shared_exit, label %latch

latch:
br i1 %cond, label %end, label %header

shared_exit:
br label %end

end:
ret void
}

; CHECK: define void @main

0 comments on commit 260a19c

Please sign in to comment.