Skip to content

Commit e4b9b88

Browse files
authored
Fixed a crash in unroller when exiting blocks use switch instead of branch (microsoft#5779)
The code to structurize loop exits assumed exiting blocks always use BranchInst and never SwitchInst. This caused a crash. This change makes the structurizer preemptively check whether this assumption is true before attempting.
1 parent 5adc7bc commit e4b9b88

File tree

5 files changed

+117
-0
lines changed

5 files changed

+117
-0
lines changed

lib/Transforms/Scalar/DxilLoopUnroll.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,11 +137,13 @@ class DxilLoopUnroll : public LoopPass {
137137
GetPassOptionUnsigned(O, "MaxIterationAttempt", &MaxIterationAttempt,
138138
false);
139139
GetPassOptionBool(O, "OnlyWarnOnFail", &OnlyWarnOnFail, false);
140+
GetPassOptionBool(O, "StructurizeLoopExits", &StructurizeLoopExits, false);
140141
}
141142
void dumpConfig(raw_ostream &OS) override {
142143
LoopPass::dumpConfig(OS);
143144
OS << ",MaxIterationAttempt=" << MaxIterationAttempt;
144145
OS << ",OnlyWarnOnFail=" << OnlyWarnOnFail;
146+
OS << ",StructurizeLoopExits=" << StructurizeLoopExits;
145147
}
146148
void RecursivelyRemoveLoopOnSuccess(LPPassManager &LPM, Loop *L);
147149
void RecursivelyRecreateSubLoopForIteration(LPPassManager &LPM, LoopInfo *LI,

lib/Transforms/Scalar/DxilRemoveUnstructuredLoopExits.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,9 @@
112112
// but the code will execute for one iteration even if the exit condition
113113
// is met.
114114
//
115+
// - If any exiting block uses a switch statement to conditionally exit the
116+
// loop, we currently do not handle that case.
117+
//
115118
// These limitations can be fixed in the future as needed.
116119
//
117120
//===----------------------------------------------------------------------===//
@@ -561,6 +564,17 @@ bool hlsl::RemoveUnstructuredLoopExits(
561564
if (!L->isLCSSAForm(*DT))
562565
return false;
563566

567+
// Check that the exiting blocks in the loop have BranchInst terminators (as
568+
// opposed to SwitchInst). At the moment we only handle BranchInst case.
569+
{
570+
llvm::SmallVector<BasicBlock *, 4> exiting_blocks;
571+
L->getExitingBlocks(exiting_blocks);
572+
for (BasicBlock *BB : exiting_blocks) {
573+
if (!isa<BranchInst>(BB->getTerminator()))
574+
return false;
575+
}
576+
}
577+
564578
// Give up if loop is not rotated somehow
565579
if (BasicBlock *latch = L->getLoopLatch()) {
566580
if (!cast<BranchInst>(latch->getTerminator())->isConditional())
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// RUN: %dxc %s /T ps_6_0 | FileCheck %s
2+
3+
// Regression test for a crash when unrolling a loop whose exiting block
4+
// uses a switch and not a branch.
5+
6+
// CHECK: call float @dx.op.unary.f32(i32 13
7+
// CHECK: call float @dx.op.unary.f32(i32 13
8+
// CHECK: call float @dx.op.unary.f32(i32 13
9+
// CHECK: call float @dx.op.unary.f32(i32 13
10+
// CHECK-NOT: call float @dx.op.unary.f32(i32 13
11+
12+
Texture1D<float> t0 : register(t0);
13+
14+
float foo(int cond) {
15+
float ret = 0;
16+
[unroll]
17+
for (int i = 0; i < 4; i++) {
18+
ret += 1;
19+
switch (cond) {
20+
case 0:
21+
ret += 10;
22+
break;
23+
case 1:
24+
ret += 20;
25+
break;
26+
default:
27+
return 42;
28+
}
29+
ret += sin(t0[cond + i]);
30+
}
31+
return ret;
32+
}
33+
34+
[RootSignature("DescriptorTable(SRV(t0))")]
35+
float main (int cond : COND) : SV_Target {
36+
float ret = 0;
37+
ret += foo(cond);
38+
return ret;
39+
}
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
; RUN: %opt %s -dxil-loop-unroll,StructurizeLoopExits=1 -S | FileCheck %s
2+
3+
; CHECK: call float @llvm.sin.f32(
4+
; CHECK: call float @llvm.sin.f32(
5+
; CHECK: call float @llvm.sin.f32(
6+
; CHECK: call float @llvm.sin.f32(
7+
; CHECK-NOT: call float @llvm.sin.f32(
8+
9+
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"
10+
target triple = "dxil-ms-dx"
11+
12+
declare float @llvm.sin.f32(float %Val) #0
13+
14+
; Function Attrs: nounwind
15+
define float @main(i32 %cond) #1 {
16+
entry:
17+
br label %for.body
18+
19+
for.body:
20+
%i.0 = phi i32 [ 0, %entry ], [ %inc, %sw.epilog ]
21+
%ret.0 = phi float [ 0.000000e+00, %entry ], [ %add7, %sw.epilog ]
22+
%add = fadd fast float %ret.0, 1.000000e+00
23+
switch i32 %cond, label %end [
24+
i32 0, label %sw.bb
25+
i32 1, label %sw.bb.3
26+
]
27+
28+
sw.bb:
29+
%add2 = fadd fast float %add, 1.000000e+01
30+
br label %sw.epilog
31+
32+
sw.bb.3:
33+
%add4 = fadd fast float %add, 2.000000e+01
34+
br label %sw.epilog
35+
36+
sw.epilog:
37+
%ret.1 = phi float [ %add4, %sw.bb.3 ], [ %add2, %sw.bb ]
38+
%Sin = call float @llvm.sin.f32(float 0.0)
39+
%add7 = fadd fast float %ret.1, %Sin
40+
%inc = add nsw i32 %i.0, 1
41+
%cmp = icmp slt i32 %inc, 4
42+
br i1 %cmp, label %for.body, label %end, !llvm.loop !3
43+
44+
end: ; preds = %sw.epilog, %for.body
45+
%retval.0 = phi float [ 4.200000e+01, %for.body ], [ %add7, %sw.epilog ]
46+
ret float %retval.0
47+
}
48+
49+
attributes #0 = { nounwind readnone }
50+
attributes #1 = { nounwind }
51+
attributes #2 = { nounwind readonly }
52+
53+
!llvm.module.flags = !{!0}
54+
!pauseresume = !{!1}
55+
!llvmdent = !{!2}
56+
57+
!0 = !{i32 2, !"Debug Info Version", i32 3}
58+
!1 = !{!"hlsl-dxilemit", !"hlsl-dxilload"}
59+
!2 = !{!"dxc(private) 1.7.0.14003 (main, a5b0488bc-dirty)"}
60+
!3 = distinct !{!3, !4}
61+
!4 = !{!"llvm.loop.unroll.full"}

utils/hct/hctdb.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2250,6 +2250,7 @@ def add_pass(name, type_name, doc, opts):
22502250
add_pass('dxil-loop-unroll', 'DxilLoopUnroll', 'DxilLoopUnroll', [
22512251
{'n':'MaxIterationAttempt', 't':'unsigned', 'c':1, 'd':'Maximum number of iterations to attempt when iteratively unrolling.'},
22522252
{'n':'OnlyWarnOnFail', 't':'bool', 'c':1, 'd':'Whether to just warn when unrolling fails.'},
2253+
{'n':'StructurizeLoopExits', 't':'bool', 'c':1, 'd':'Whether the unroller should try to structurize loop exits first.'}
22532254
])
22542255
add_pass('dxil-erase-dead-region', 'DxilEraseDeadRegion', 'DxilEraseDeadRegion', [])
22552256
add_pass('dxil-remove-dead-blocks', 'DxilRemoveDeadBlocks', 'DxilRemoveDeadBlocks', [])

0 commit comments

Comments
 (0)