Skip to content

Commit 2129fc6

Browse files
authored
Fix spurious push constant storage capabilities (#11134)
Fixes #10055. This stops the SPIR-V storage-capability scan from recursing through ordinary pointer types: a pointer's pointee is not stored inline in the surrounding push-constant/uniform/storage-buffer container, so its bit width should not cause capabilities such as `StoragePushConstant8` or `StoragePushConstant16` to be emitted for the outer container. The change preserves recursion through ref/out/borrowed parameter wrappers, and adds regression coverage for both the `uint8_t*` push-constant case from #10055 and a related BDA pointer-to-`uint16_t` case. Local validation: - `cmake --preset default` - `cmake --build --preset release --target slang-test slangc` - `./build/Release/bin/slang-test tests/spirv/push-constant-ptr-no-storage-push-constant8.slang tests/spirv/bda-pointer-no-spurious-storage-capability.slang` Note: reviewers, please @-mention `@thorn-agent` in review comments so I receive notifications for follow-up. Co-authored-by: thorn-agent <thorn-agent@users.noreply.github.com>
1 parent aaa5f89 commit 2129fc6

3 files changed

Lines changed: 63 additions & 1 deletion

File tree

source/slang/slang-emit-spirv.cpp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9680,7 +9680,6 @@ struct SPIRVEmitContext : public SourceEmitterBase, public SPIRVEmitSharedContex
96809680
visited);
96819681
break;
96829682

9683-
case kIROp_PtrType:
96849683
case kIROp_RefParamType:
96859684
case kIROp_BorrowInParamType:
96869685
case kIROp_OutParamType:
@@ -9693,6 +9692,17 @@ struct SPIRVEmitContext : public SourceEmitterBase, public SPIRVEmitSharedContex
96939692
visited);
96949693
break;
96959694

9695+
case kIROp_PtrType:
9696+
// A pointer's pointee is not stored inline in the outer container;
9697+
// it lives in whatever storage class the pointer's address space
9698+
// names. Do not recurse through pointers when deciding whether
9699+
// the outer container needs 8/16-bit-storage capabilities --
9700+
// otherwise a `uint16_t*` BDA argument in a push-constant block
9701+
// would spuriously require `StoragePushConstant16` (issue #11096).
9702+
// The pointee's own storage-class requirements are handled when
9703+
// that storage class is emitted.
9704+
break;
9705+
96969706
case kIROp_RateQualifiedType:
96979707
if (auto ptrType = as<IRRateQualifiedType>(type))
96989708
return checkTypeNeedsStorageCapability(
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// Regression for #11096: a `uint16_t*` kernel argument (BDA pointer) bundled
2+
// into the entry-point push-constant block used to spuriously require
3+
// `StoragePushConstant16`, because `checkTypeNeedsStorageCapability`
4+
// recursed through pointer types into their pointee. The pointee lives
5+
// in PhysicalStorageBuffer, not the outer container's storage class.
6+
7+
//TEST:SIMPLE(filecheck=CHECK): -target spirv-asm -entry main -stage compute
8+
9+
[shader("compute")]
10+
[numthreads(512, 1, 1)]
11+
void main(
12+
uint* param1,
13+
uint16_t* param2,
14+
uniform int iteration,
15+
uint3 groupThreadID : SV_GroupThreadID,
16+
uint3 groupId : SV_GroupID)
17+
{
18+
param1[groupId.x] = (uint)param2[groupThreadID.x] + iteration;
19+
}
20+
21+
// The push-constant block contains BDA addresses and a plain int32 only --
22+
// no inline 16-bit scalars -- so none of the *16 storage capabilities
23+
// should be required.
24+
// CHECK-NOT: OpCapability StoragePushConstant16
25+
// CHECK-NOT: OpCapability UniformAndStorageBuffer16BitAccess
26+
// CHECK: OpCapability PhysicalStorageBufferAddresses
27+
// The pointee is a uint16_t, so Int16 is still needed.
28+
// CHECK: OpCapability Int16
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Regression for #10055: a push-constant block containing a pointer to
2+
// uint8_t should not require StoragePushConstant8. The push constant stores a
3+
// physical-storage-buffer address; the pointed-to uint8_t lives in that storage
4+
// class, not inline in the push-constant block.
5+
6+
//TEST:SIMPLE(filecheck=CHECK): -target spirv-asm -entry main -stage compute
7+
8+
struct CSInput
9+
{
10+
uint8_t* output;
11+
};
12+
13+
[shader("compute")]
14+
[numthreads(32, 1, 1)]
15+
void main(uint DTid: SV_DispatchThreadID, uniform CSInput consts)
16+
{
17+
consts.output[DTid] = 1;
18+
}
19+
20+
// CHECK-NOT: OpCapability StoragePushConstant8
21+
// CHECK: OpCapability PhysicalStorageBufferAddresses
22+
// The pointee is a uint8_t, so Int8 is still needed.
23+
// CHECK: OpCapability Int8
24+
// CHECK-NOT: OpCapability StoragePushConstant8

0 commit comments

Comments
 (0)