Skip to content

Commit 3bb9bee

Browse files
damyanpCopilotgithub-actions[bot]
authored
[SimplifyCFG] Fix illegal-width bitmap from switch lookup table (#8444)
SwitchLookupTable built bitmap APInts of width `TableSize * ResultBitWidth`, producing non-standard integer widths (i9, i17, i26, i33, i40, ...) that DXIL validation rejects - DXIL only allows i1/i8/i16/i32/i64. Round the bitmap width up to a legal width, and cap the optimization at 32 bits to avoid silently promoting to i64 (which would set the Int64Ops shader flag and add a 64-bit-integer capability requirement the source shader did not have). Wider tables fall back to the array path, or - for i1 results - preserve the original switch. Closes #8421 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent 275426a commit 3bb9bee

3 files changed

Lines changed: 257 additions & 1 deletion

File tree

‎lib/Transforms/Utils/SimplifyCFG.cpp‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3855,7 +3855,13 @@ SwitchLookupTable::SwitchLookupTable(
38553855
// If the type is integer and the table fits in a register, build a bitmap.
38563856
if (WouldFitInRegister(DL, TableSize, ValueType)) {
38573857
IntegerType *IT = cast<IntegerType>(ValueType);
3858-
APInt TableInt(TableSize * IT->getBitWidth(), 0);
3858+
// HLSL Change Begin: Round bitmap width up to size supported by DXIL (i16
3859+
// or i32)
3860+
uint64_t RawBitMapWidth = TableSize * IT->getBitWidth();
3861+
uint64_t BitMapWidth =
3862+
NextPowerOf2(std::max(UINT64_C(15), RawBitMapWidth - 1));
3863+
APInt TableInt(BitMapWidth, 0);
3864+
// HLSL Change End
38593865
for (uint64_t I = TableSize; I > 0; --I) {
38603866
TableInt <<= IT->getBitWidth();
38613867
// Insert values into the bitmap. Undef values are set to zero.
@@ -3950,6 +3956,11 @@ bool SwitchLookupTable::WouldFitInRegister(const DataLayout &DL,
39503956
// Avoid overflow, fitsInLegalInteger uses unsigned int for the width.
39513957
if (TableSize >= UINT_MAX/IT->getBitWidth())
39523958
return false;
3959+
// HLSL Change Begin: Cap at 32 bits so the bitmap stays in a DXIL-supported
3960+
// integer width and never promotes to i64 (which would set Int64Ops).
3961+
if (TableSize * IT->getBitWidth() > 32)
3962+
return false;
3963+
// HLSL Change End
39533964
return DL.fitsInLegalInteger(TableSize * IT->getBitWidth());
39543965
}
39553966

‎tools/clang/test/HLSLFileCheck/passes/llvm/simplifycfg/dxil_switch_bitmap_avoids_int64.ll‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
; RUN: %opt %s -simplifycfg -S | FileCheck %s
2+
;
3+
; Companion test for https://github.com/microsoft/DirectXShaderCompiler/issues/8421
4+
;
5+
; A switch with 33 boolean cases would naively build an i33 bitmap. Rounding up
6+
; to i64 would emit i64 ops and silently set the Int64Ops shader flag, so the
7+
; bitmap optimization is capped at 32 bits and the switch is preserved instead.
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+
; CHECK-LABEL: @switch_bool_33_cases
13+
; CHECK-NOT: i33
14+
; CHECK-NOT: lshr i64
15+
; CHECK-NOT: trunc i64
16+
; CHECK: switch i32
17+
; CHECK: ret i1
18+
19+
define i1 @switch_bool_33_cases(i32 %x) {
20+
entry:
21+
switch i32 %x, label %default [
22+
i32 1, label %case_true
23+
i32 2, label %case_true
24+
i32 3, label %case_false
25+
i32 4, label %case_true
26+
i32 5, label %case_false
27+
i32 6, label %case_true
28+
i32 7, label %case_true
29+
i32 8, label %case_false
30+
i32 9, label %case_false
31+
i32 10, label %case_true
32+
i32 11, label %case_true
33+
i32 12, label %case_false
34+
i32 13, label %case_true
35+
i32 14, label %case_false
36+
i32 15, label %case_true
37+
i32 16, label %case_true
38+
i32 17, label %case_false
39+
i32 18, label %case_true
40+
i32 19, label %case_false
41+
i32 20, label %case_false
42+
i32 21, label %case_true
43+
i32 22, label %case_true
44+
i32 23, label %case_false
45+
i32 24, label %case_true
46+
i32 25, label %case_false
47+
i32 26, label %case_true
48+
i32 27, label %case_false
49+
i32 28, label %case_true
50+
i32 29, label %case_true
51+
i32 30, label %case_false
52+
i32 31, label %case_true
53+
i32 32, label %case_false
54+
i32 33, label %case_true
55+
]
56+
57+
case_true:
58+
br label %end
59+
60+
case_false:
61+
br label %end
62+
63+
default:
64+
br label %end
65+
66+
end:
67+
%result = phi i1 [ true, %case_true ], [ false, %case_false ], [ false, %default ]
68+
ret i1 %result
69+
}

‎tools/clang/test/HLSLFileCheck/passes/llvm/simplifycfg/dxil_switch_bitmap_legal_types.ll‎

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
; RUN: %opt %s -simplifycfg -S | FileCheck %s
2+
;
3+
; Regression test for https://github.com/microsoft/DirectXShaderCompiler/issues/8421
4+
;
5+
; SimplifyCFG's switch-to-lookup-table built bitmaps of width
6+
; "TableSize * ValueBitWidth", producing illegal widths like i9, i17, i26 or
7+
; i48. The fix rounds the bitmap up to i16 or i32, and skips the lookup table
8+
; entirely if the bitmap would exceed 32 bits.
9+
10+
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"
11+
target triple = "dxil-ms-dx"
12+
13+
; 5 boolean cases with mixed values would naively build an i5 bitmap; rounded
14+
; to i16 by the fix.
15+
;
16+
; CHECK-LABEL: @switch_bool_5_cases
17+
; CHECK: switch.lookup:
18+
; CHECK: lshr i16
19+
; CHECK-NOT: i5
20+
; CHECK: ret i1
21+
22+
define i1 @switch_bool_5_cases(i32 %x) {
23+
entry:
24+
switch i32 %x, label %default [
25+
i32 0, label %case_t
26+
i32 1, label %case_t
27+
i32 2, label %case_f
28+
i32 3, label %case_t
29+
i32 4, label %case_f
30+
]
31+
32+
case_t:
33+
br label %end
34+
35+
case_f:
36+
br label %end
37+
38+
default:
39+
br label %end
40+
41+
end:
42+
%result = phi i1 [ true, %case_t ], [ false, %case_f ], [ false, %default ]
43+
ret i1 %result
44+
}
45+
46+
; 9 boolean cases would naively build an i9 bitmap; rounded to i16 by the fix.
47+
;
48+
; CHECK-LABEL: @switch_bool_9_cases
49+
; CHECK: switch.lookup:
50+
; CHECK: lshr i16
51+
; CHECK-NOT: i9
52+
; CHECK: ret i1
53+
54+
define i1 @switch_bool_9_cases(i32 %x) {
55+
entry:
56+
switch i32 %x, label %default [
57+
i32 0, label %case_true
58+
i32 1, label %case_true
59+
i32 3, label %case_true
60+
i32 4, label %case_true
61+
i32 5, label %case_true
62+
i32 6, label %case_true
63+
i32 7, label %case_true
64+
i32 8, label %case_true
65+
]
66+
67+
case_true:
68+
br label %end
69+
70+
default:
71+
br label %end
72+
73+
end:
74+
%result = phi i1 [ true, %case_true ], [ false, %default ]
75+
ret i1 %result
76+
}
77+
78+
; 17 boolean cases would naively build an i17 bitmap; rounded to i32 by the
79+
; fix.
80+
;
81+
; CHECK-LABEL: @switch_bool_17_cases
82+
; CHECK: switch.lookup:
83+
; CHECK: lshr i32
84+
; CHECK-NOT: i17
85+
; CHECK: ret i1
86+
87+
define i1 @switch_bool_17_cases(i32 %x) {
88+
entry:
89+
switch i32 %x, label %default [
90+
i32 0, label %case_true
91+
i32 1, label %case_true
92+
i32 3, label %case_true
93+
i32 4, label %case_true
94+
i32 5, label %case_true
95+
i32 7, label %case_true
96+
i32 8, label %case_true
97+
i32 10, label %case_true
98+
i32 11, label %case_true
99+
i32 12, label %case_true
100+
i32 14, label %case_true
101+
i32 15, label %case_true
102+
i32 16, label %case_true
103+
]
104+
105+
case_true:
106+
br label %end
107+
108+
default:
109+
br label %end
110+
111+
end:
112+
%result = phi i1 [ true, %case_true ], [ false, %default ]
113+
ret i1 %result
114+
}
115+
116+
; 26 boolean cases (the original #8421 repro) would naively build an i26
117+
; bitmap; rounded to i32 by the fix.
118+
;
119+
; CHECK-LABEL: @switch_bool_26_cases
120+
; CHECK: switch.lookup:
121+
; CHECK: lshr i32
122+
; CHECK-NOT: i26
123+
; CHECK: ret i1
124+
125+
define i1 @switch_bool_26_cases(i32 %x) {
126+
entry:
127+
switch i32 %x, label %default [
128+
i32 1, label %case_true
129+
i32 6, label %case_true
130+
i32 11, label %case_true
131+
i32 16, label %case_true
132+
i32 21, label %case_true
133+
i32 26, label %case_true
134+
]
135+
136+
case_true:
137+
br label %end
138+
139+
default:
140+
br label %end
141+
142+
end:
143+
%result = phi i1 [ true, %case_true ], [ false, %default ]
144+
ret i1 %result
145+
}
146+
147+
; Non-i1 result: 3 i16 cases would naively build an i48 bitmap. The fix makes
148+
; bitmaps wider than 32 bits skip the lookup table; the original switch is
149+
; preserved.
150+
;
151+
; CHECK-LABEL: @switch_i16_3_cases
152+
; CHECK: switch i32
153+
; CHECK-NOT: switch.lookup
154+
; CHECK-NOT: i48
155+
; CHECK: ret i16
156+
157+
define i16 @switch_i16_3_cases(i32 %x) {
158+
entry:
159+
switch i32 %x, label %default [
160+
i32 0, label %c0
161+
i32 1, label %c1
162+
i32 2, label %c2
163+
]
164+
165+
c0: br label %end
166+
c1: br label %end
167+
c2: br label %end
168+
default: br label %end
169+
170+
end:
171+
; Non-linear values prevent the LinearMap fast path so the bitmap path is
172+
; the one that would have been chosen, but since this would want to use an i48
173+
; (which isn't valid in DXIL) the switch is preserved.
174+
%result = phi i16 [ 73, %c0 ], [ 42, %c1 ], [ 88, %c2 ], [ 0, %default ]
175+
ret i16 %result
176+
}

0 commit comments

Comments
 (0)