KhronosGroup / Vulkan-ValidationLayers

Vulkan Validation Layers (VVL)
https://vulkan.lunarg.com/doc/sdk/latest/linux/khronos_validation_layer.html
Other
727 stars 396 forks source link

Unexpected VUID-RuntimeSpirv-NonWritable-06340 and VUID-RuntimeSpirv-NonWritable-06341 following check for VUID-RuntimeSpirv-Location-06428 #8223

Open j-e10 opened 3 days ago

j-e10 commented 3 days ago

Environment:

Describe the Issue

Starting on commit https://github.com/KhronosGroup/Vulkan-ValidationLayers/commit/08ec2b683f7a54c5be80f7a5ac834ec6a7be17d2, I'm getting VUID-RuntimeSpirv-NonWritable-06340 and VUID-RuntimeSpirv-NonWritable-06341 errors (e.g., vertex/fragment shaders making use of writable storage buffers w/o the vertexStoresAndAtomics/fragmentStoresAndAtomics feature enabled). Earlier VVL commits do not trigger this.

The GLSL shaders in question do use storage buffers but they are all marked readonly.

Expected behavior

No VUID-RuntimeSpirv-NonWritable-06340 and VUID-RuntimeSpirv-NonWritable-06341 errors.

The commit mentioned concerns VUID-RuntimeSpirv-Location-06428 so I think it's a regression.

Valid Usage ID

VUID-RuntimeSpirv-NonWritable-06340 VUID-RuntimeSpirv-NonWritable-06341

spencer-lunarg commented 1 day ago

@j-e10 it would be GREATLY helpful if you could provide the GLSL (or the SPIR-V is better) which causes this false positives as it will help speed up fixing this... I want to make sure this is correct before the next SDK release

j-e10 commented 1 day ago

Example GLSL:

fragment

#version 450
layout (location=0) in float i;
layout (location=0) out vec4 o;
void main(void)
{
    o = vec4(i);
}

vertex

#version 450
layout(set=0, binding=16, std430) readonly buffer b_ { float v; } b;
layout(location=0) out float o;
void main(void)
{
    o = b.v;
}

I have a somewhat complex shader building process, but spirv-cross gives me the following GLSL from the final SPIR-V in that process (which matches what I'd expect):

fragment

#version 450

layout(location = 0) out vec4 _9;
layout(location = 0) in float _11;

void main()
{
    _9 = vec4(_11);
}

vertex

#version 450

layout(set = 0, binding = 16, std430) readonly buffer _9_11
{
    float _m0;
} _11;

layout(location = 0) out float _8;

void main()
{
    _8 = _11._m0;
}

Attempting to create a pipeline gives me:

validation layer: Validation Error: [ VUID-RuntimeSpirv-NonWritable-06341 ] Object 0: handle = 0x3b500000003b5, type = VK_OBJECT_TYPE_SHADER_MODULE; | MessageID = 0x116350f | vkCreateGraphicsPipelines(): pCreateInfos[0].pStages[0] vertexPipelineStoresAndAtomics was not enabled. The Vulkan spec states: If vertexPipelineStoresAndAtomics is not enabled, then all storage image, storage texel buffer, and storage buffer variables in the vertex, tessellation, and geometry stages must be decorated with the NonWritable decoration (https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-RuntimeSpirv-NonWritable-06341)
spencer-lunarg commented 1 day ago

So I took

#version 450
layout(set=0, binding=16, std430) readonly buffer b_ { float v; } b;
layout(location=0) out float o;
void main(void) 
{
    o = b.v;
}

and ran glslangValidator -V in.vert -o out.spv and if I run spirv-dis on it I can see

OpMemberDecorate %b_ 0 NonWritable

which is what the error message is pointing out

glslang takes the readonly and maps it to NonWritable in SPIR-V (all Vulkan shader VUs will reference SPIR-V terms, not GLSL terms)

j-e10 commented 1 day ago

Right.

If vertexPipelineStoresAndAtomics is not enabled, then all storage image, storage texel buffer, and storage buffer variables in the vertex, tessellation, and geometry stages must be decorated with the NonWritable decoration

I keep vertexPipelineStoresAndAtomics disabled. Therefore, all of my storage images/buffers must be NonWritable; which is the case in the example. Though it's still emitting the error.

spencer-lunarg commented 23 hours ago

Sorry I misread this, I think I see the issue, the check is not correctly checking the members of the struct... will get some tests and a fix next week (off for til Monday)