Skip to content

Commit

Permalink
Fix assertion on splat of groupshared scalar (#6930)
Browse files Browse the repository at this point in the history
When splatting a groupshared scalar, we would trip an "Invalid
constantexpr cast!" assertion. This would happen while evaluating the
ImplicitCastExpr to turn the groupshared scalar into a vector because
the scalar expression was in a different address space (groupshared) vs
the target vector (local). The fix is to ensure that when looking up the
vector member expression, insert an lvalue-to-rvalue cast if necessary;
i.e. when a swizzle contains duplicate elements.
  • Loading branch information
amaiorano authored Sep 25, 2024
1 parent d9a5e97 commit 0372fb7
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 12 deletions.
15 changes: 13 additions & 2 deletions tools/clang/lib/Sema/SemaHLSL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8444,8 +8444,19 @@ ExprResult HLSLExternalSource::LookupVectorMemberExprForHLSL(
ExprValueKind VK = positions.ContainsDuplicateElements()
? VK_RValue
: (IsArrow ? VK_LValue : BaseExpr.getValueKind());
HLSLVectorElementExpr *vectorExpr = new (m_context) HLSLVectorElementExpr(
resultType, VK, &BaseExpr, *member, MemberLoc, positions);

Expr *E = &BaseExpr;
// Insert an lvalue-to-rvalue cast if necessary
if (BaseExpr.getValueKind() == VK_LValue && VK == VK_RValue) {
// Remove qualifiers from result type and cast target type
resultType = resultType.getUnqualifiedType();
auto targetType = E->getType().getUnqualifiedType();
E = ImplicitCastExpr::Create(*m_context, targetType,
CastKind::CK_LValueToRValue, E, nullptr,
VK_RValue);
}
HLSLVectorElementExpr *vectorExpr = new (m_context)
HLSLVectorElementExpr(resultType, VK, E, *member, MemberLoc, positions);

return vectorExpr;
}
Expand Down
15 changes: 15 additions & 0 deletions tools/clang/test/CodeGenHLSL/SplatGroupSharedScalar.hlsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// RUN: %dxc -E main -T cs_6_0 -fcgl %s | FileCheck %s

// Validate that when swizzling requires an r-value (i.e. duplicate elements), that the result is stored to
// and loaded from a temporary.
// CHECK: store <1 x i32> %splat.splat, <1 x i32>* %tmp
// CHECK-NEXT: %1 = load <1 x i32>, <1 x i32>* %tmp
// CHECK-NEXT: %2 = shufflevector <1 x i32> %1, <1 x i32> undef, <4 x i32> zeroinitializer
// CHECK-NEXT: store <4 x i32> %2, <4 x i32>* %x

groupshared int a;
[numthreads(64, 1, 1)]
void main() {
a = 123;
int4 x = (a).xxxx;
}
12 changes: 6 additions & 6 deletions tools/clang/test/CodeGenSPIRV/op.vector.swizzle.hlsl
Original file line number Diff line number Diff line change
Expand Up @@ -118,18 +118,18 @@ void main() {
// Keep lhs.1
// So final selectors to write to lhs.(0, 1, 2, 3): 6, 1, 4, 5
// CHECK-NEXT: [[v22:%[0-9]+]] = OpLoad %v2float %v2f
// CHECK-NEXT: [[vs15:%[0-9]+]] = OpVectorShuffle %v3float [[v22]] [[v22]] 0 1 0
// CHECK-NEXT: [[vs15:%[0-9]+]] = OpVectorShuffle %v2float [[v22]] [[v22]] 1 0
// CHECK-NEXT: [[vs16:%[0-9]+]] = OpVectorShuffle %v3float [[vs15]] [[vs15]] 1 0 1
// CHECK-NEXT: [[v23:%[0-9]+]] = OpLoad %v4float %v4f2
// CHECK-NEXT: [[vs16:%[0-9]+]] = OpVectorShuffle %v4float [[v23]] [[vs15]] 6 1 4 5
// CHECK-NEXT: OpStore %v4f2 [[vs16]]
// CHECK-NEXT: [[vs17:%[0-9]+]] = OpVectorShuffle %v4float [[v23]] [[vs16]] 6 1 4 5
// CHECK-NEXT: OpStore %v4f2 [[vs17]]
v4f2.wzx.grb = v2f.gr.yxy; // select more than original, write to a part

// CHECK-NEXT: [[v24:%[0-9]+]] = OpLoad %v4float %v4f1
// CHECK-NEXT: OpStore %v4f2 [[v24]]
v4f2.wzyx.abgr.xywz.rgab = v4f1.xyzw.xyzw.rgab.rgab; // from original vector to original vector

// CHECK-NEXT: [[v24_0:%[0-9]+]] = OpAccessChain %_ptr_Function_float %v4f1 %int_2
// CHECK-NEXT: [[ce1:%[0-9]+]] = OpLoad %float [[v24_0]]
// CHECK-NEXT: [[v24_0:%[0-9]+]] = OpLoad %v4float %v4f1
// CHECK-NEXT: [[ce1:%[0-9]+]] = OpCompositeExtract %float [[v24_0]] 2
// CHECK-NEXT: [[ac4:%[0-9]+]] = OpAccessChain %_ptr_Function_float %v4f2 %int_1
// CHECK-NEXT: OpStore [[ac4]] [[ce1]]
v4f2.wzyx.zy.x = v4f1.xzyx.y.x; // from one element (rvalue) to one element (lvalue)
Expand Down
8 changes: 4 additions & 4 deletions tools/clang/test/CodeGenSPIRV/op.vector.swizzle.size1.hlsl
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ void main(float4 input: INPUT) {

// Selecting from resources
// CHECK: [[fptr:%[0-9]+]] = OpAccessChain %_ptr_Uniform_v4float %PerFrame %int_0 %uint_5 %int_0
// CHECK-NEXT: [[elem:%[0-9]+]] = OpAccessChain %_ptr_Uniform_float [[fptr]] %int_3
// CHECK-NEXT: {{%[0-9]+}} = OpLoad %float [[elem]]
// CHECK-NEXT: [[val:%[0-9]+]] = OpLoad %v4float [[fptr]]
// CHECK-NEXT: {{%[0-9]+}} = OpCompositeExtract %float [[val]] 3
v4f = input * PerFrame[5].f.www.r;
// CHECK: [[fptr_0:%[0-9]+]] = OpAccessChain %_ptr_Uniform_v4float %PerFrame %int_0 %uint_6 %int_0
// CHECK-NEXT: [[elem_0:%[0-9]+]] = OpAccessChain %_ptr_Uniform_float [[fptr_0]] %int_2
// CHECK-NEXT: {{%[0-9]+}} = OpLoad %float [[elem_0]]
// CHECK-NEXT: [[val_0:%[0-9]+]] = OpLoad %v4float [[fptr_0]]
// CHECK-NEXT: {{%[0-9]+}} = OpCompositeExtract %float [[val_0]] 2
sf = PerFrame[6].f.zzz.r * input.y;
}

0 comments on commit 0372fb7

Please sign in to comment.