Skip to content

Commit

Permalink
Remove invalid assert in literal visitor (#6875)
Browse files Browse the repository at this point in the history
The code has an assert when processing an OpLoad in the literal visitor
to make sure that the result of the load is not a literal type. This is
not always true. If there is a compiler-generated, temporary variable
that gets its type from a literal, then the result type of the load will
have to be decuded by the literal visitor. That is not always possible.

However, the code already handle this situation correctly. If the result
of the load is a literal type, then the function will return true
without doing anything because `canDeduceTypeFromLitType` will return
false, as it should.

Fixes #6798
  • Loading branch information
s-perron authored Aug 23, 2024
1 parent 7dcf2b4 commit 9c6b2c1
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 1 deletion.
1 change: 0 additions & 1 deletion tools/clang/lib/SPIRV/LiteralTypeVisitor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,6 @@ bool LiteralTypeVisitor::visit(SpirvLoad *inst) {

assert(inst->hasAstResultType());
QualType resultType = inst->getAstResultType();
assert(!isLitTypeOrVecOfLitType(resultType));

if (!canDeduceTypeFromLitType(pointerType, resultType))
return true;
Expand Down
27 changes: 27 additions & 0 deletions tools/clang/test/CodeGenSPIRV/select.int.lit.to.bool.hlsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// RUN: %dxc -T ps_6_0 -HV 2021 -E main -fcgl %s -spirv | FileCheck %s

// In this case, the types for the literals in the select cannot be deduced
// because the cast to a bool does not force the literals to be any particular
// bitwidth. So the types should fall back to a 32-bit signed interger.
void foo(uint x) {
// CHECK: %temp_var_ternary = OpVariable %_ptr_Function_int Function
// CHECK: [[x:%[0-9]+]] = OpLoad %uint %x
// CHECK: [[cond:%[0-9]+]] = OpULessThan %bool [[x]] %uint_64
// CHECK: OpSelectionMerge %ternary_merge None
// CHECK: OpBranchConditional [[cond]] %ternary_lhs %ternary_rhs
// CHECK: %ternary_lhs = OpLabel
// CHECK: OpStore %temp_var_ternary %int_1
// CHECK: OpBranch %ternary_merge
// CHECK: %ternary_rhs = OpLabel
// CHECK: OpStore %temp_var_ternary %int_0
// CHECK: OpBranch %ternary_merge
// CHECK: %ternary_merge = OpLabel
// CHECK: [[ld:%[0-9]+]] = OpLoad %int %temp_var_ternary
// CHECK: [[res:%[0-9]+]] = OpINotEqual %bool [[ld]] %int_0
// CHECK: OpStore %value [[res]]
bool value = x < 64 ? 1 : 0;
}

void main() {
foo(2);
}

0 comments on commit 9c6b2c1

Please sign in to comment.