Skip to content

Commit c6f0ea4

Browse files
authored
[spirv] Fix crash on some ternary ops (#4534)
Sometimes the Clang AST does not include an implicit cast wrapper around an integer-type condition in the ternary operator, which causes a crash when the SPIR-V generated has a non-bool type for the OpBranchConditional. This change checks for this case and casts the condition to bool. Example ASTs: ``` // | `-ConditionalOperator 0x24022c82d48 <col:10, col:19> 'bool' // | |-ImplicitCastExpr 0x24022c82d00 <col:10> 'bool' <IntegralToBoolean> // | | `-IntegerLiteral 0x24022c82c58 <col:10> 'literal int' 1 bool b1, b2; bool b = 1 ? b1 : b2; // | `-ConditionalOperator 0x1ede5137380 <col:18, col:27> 'SamplerState' // | |-IntegerLiteral 0x1ede5137308 <col:18> 'literal int' 1 SamplerState s1, s2; SamplerState s = 1 ? s1 : s2; ``` Fixes #3831
1 parent 24ea9e8 commit c6f0ea4

2 files changed

Lines changed: 34 additions & 0 deletions

File tree

tools/clang/lib/SPIRV/SpirvEmitter.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3381,6 +3381,15 @@ SpirvEmitter::doConditionalOperator(const ConditionalOperator *expr) {
33813381
return value;
33823382
}
33833383

3384+
// Usually integer conditional types in HLSL will be wrapped in an
3385+
// ImplicitCastExpr<IntegralToBoolean> in the Clang AST. However, some
3386+
// combinations of result types can result in a bare integer (literal or
3387+
// reference) as a condition, which still needs to be cast to bool.
3388+
if (cond->getType()->isIntegerType()) {
3389+
condition =
3390+
castToBool(condition, cond->getType(), astContext.BoolTy, loc, range);
3391+
}
3392+
33843393
// If we can't use OpSelect, we need to create if-else control flow.
33853394
auto *tempVar = spvBuilder.addFnVar(type, loc, "temp.var.ternary");
33863395
auto *thenBB = spvBuilder.createBasicBlock("if.true");

tools/clang/test/CodeGenSPIRV/ternary-op.cond-op.hlsl

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ void main() {
1616
// CHECK: %temp_var_ternary = OpVariable %_ptr_Function_mat2v3float Function
1717
// CHECK: %temp_var_ternary_0 = OpVariable %_ptr_Function_mat2v3float Function
1818
// CHECK: %temp_var_ternary_1 = OpVariable %_ptr_Function_type_sampler Function
19+
// CHECK: %temp_var_ternary_2 = OpVariable %_ptr_Function_type_sampler Function
1920

2021
bool b0;
2122
int m, n, o;
@@ -191,6 +192,30 @@ void main() {
191192
bool2x3 cond2x3;
192193
float2x3 true2x3, false2x3;
193194
float2x3 result2x3 = cond2x3 ? true2x3 : false2x3;
195+
196+
// Note that AST does not have a ImplicitCastExpr around intCond for this case:
197+
// | `-VarDecl 0x2a90094e0d0 <col:3, col:37> col:16 s 'SamplerState' cinit
198+
// | `-ConditionalOperator 0x2a90094e1b8 <col:20, col:37> 'SamplerState'
199+
// | |-DeclRefExpr 0x2a90094e140 <col:20> 'int' lvalue Var 0x2a90051e1c0 'intCond' 'int'
200+
// | |-DeclRefExpr 0x2a90094e168 <col:30> 'SamplerState' lvalue Var 0x2a9004c6f40 'gSS1' 'SamplerState'
201+
// | `-DeclRefExpr 0x2a90094e190 <col:37> 'SamplerState' lvalue Var 0x2a9004c7000 'gSS2' 'SamplerState'
202+
203+
// CHECK: [[intCond:%\d+]] = OpLoad %int %intCond
204+
// CHECK-NEXT: [[gSS1:%\d+]] = OpLoad %type_sampler %gSS1
205+
// CHECK-NEXT: [[gSS2:%\d+]] = OpLoad %type_sampler %gSS2
206+
// CHECK-NEXT: [[boolCond:%\d+]] = OpINotEqual %bool [[intCond]] %int_0
207+
// CHECK-NEXT: OpSelectionMerge %if_merge_2 None
208+
// CHECK-NEXT: OpBranchConditional [[boolCond]] %if_true_2 %if_false_2
209+
// CHECK-NEXT: %if_true_2 = OpLabel
210+
// CHECK-NEXT: OpStore %temp_var_ternary_2 [[gSS1]]
211+
// CHECK-NEXT: OpBranch %if_merge_2
212+
// CHECK-NEXT: %if_false_2 = OpLabel
213+
// CHECK-NEXT: OpStore %temp_var_ternary_2 [[gSS2]]
214+
// CHECK-NEXT: OpBranch %if_merge_2
215+
// CHECK-NEXT: %if_merge_2 = OpLabel
216+
// CHECK-NEXT: [[tempVar:%\d+]] = OpLoad %type_sampler %temp_var_ternary_2
217+
// CHECK-NEXT: OpStore %s [[tempVar]]
218+
SamplerState s = intCond ? gSS1 : gSS2;
194219
}
195220

196221
//

0 commit comments

Comments
 (0)