Skip to content

spirv-val: Fix Offset not checking if in struct#6701

Open
spencer-lunarg wants to merge 1 commit into
KhronosGroup:mainfrom
spencer-lunarg:spencer-lunarg-untyped-pointers-the-gift-that-will-give-for-years-offset-fix
Open

spirv-val: Fix Offset not checking if in struct#6701
spencer-lunarg wants to merge 1 commit into
KhronosGroup:mainfrom
spencer-lunarg:spencer-lunarg-untyped-pointers-the-gift-that-will-give-for-years-offset-fix

Conversation

@spencer-lunarg

Copy link
Copy Markdown
Contributor

closes #6696

seems we currently were also just not validating Offset as well


if (target->opcode() != spv::Op::OpDecorationGroup) {
if (IsMemberDecorationOnly(decoration)) {
// SPIR-V spec bug? (...example lost in time)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... not sure where this is coming from, probably something when someone gets to #6594

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... guess the smoke test found more

@alan-baker what do you want to do here, clearly something is wrong as the Offset is not following the

image

unless it was modified for all these various extension cases, but I guess don't understand what

layout(binding = 0, offset = 4) uniform atomic_uint countArr[4];

even means and where the memory for these offsets are all defined

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll want to double check this PR against CTS. I remember the lack of spec language and existing tests was frustrating.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}

TEST_F(ValidateSpvEXTDescriptorHeap, BufferPointerEXTLayout) {
// TODO - Seems like VUID-StandaloneSpirv-Result-11346 is wrong because

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OpMemberDecorate %U 0 Offset 0
OpDecorateId %_runtimearr_17 ArrayStrideIdEXT %18
OpDecorate %_ptr_UniformConstant Offset 0
; OpDecorate %_ptr_Uniform Offset 0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree this shouldn't be there.

%_runtimearr_17 and %U do need explicit layouts (and have it). The pointers should not be laid out though.


if (target->opcode() != spv::Op::OpDecorationGroup) {
if (IsMemberDecorationOnly(decoration)) {
// SPIR-V spec bug? (...example lost in time)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll want to double check this PR against CTS. I remember the lack of spec language and existing tests was frustrating.

@alan-baker

Copy link
Copy Markdown
Contributor

Looking at the dxc smoketest results, apparently this is needed for meshing shading too.

alan-baker pushed a commit that referenced this pull request May 21, 2026
Pulled the `OffsetIdEXT` part out of
#6701 until I resolve
https://gitlab.khronos.org/spirv/SPIR-V/-/issues/937

This is mainly because people are bringing up `SPV_EXT_descriptor_heap`
in dxc/slang and want to not have this waste time hitting the "driver
bug" which is actually [not a driver
bug](https://gitlab.khronos.org/Tracker/vk-gl-cts/-/issues/6505)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

spirv-val: Missing OffsetIdEXT check on a struct

2 participants