Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix NonWritable check when vertexPipelineStoresAndAtomics not enabled #73

Closed
karl-lunarg opened this issue May 14, 2018 · 4 comments
Closed
Assignees
Milestone

Comments

@karl-lunarg
Copy link
Contributor

Issue by greg-lunarg (MIGRATED)
Wednesday Mar 28, 2018 at 21:29 GMT
Originally opened as KhronosGroup/Vulkan-LoaderAndValidationLayers#2526


The following Vulkan validation layer error starting popping up on code that previously executed without error:

SC(ERROR / SPEC): object: 0x0 type: 0 msgNum: 15 - Shader requires VkPhysicalDeviceFeatures::vertexPipelineStoresAndAtom
ics but is not enabled on the device
SC(ERROR): object: 0x0 type: 0 location: 1167 msg_code: 15: Object: 0x0 | Shader requires VkPhysicalDeviceFeatures::vert
exPipelineStoresAndAtomics but is not enabled on the device

We believe that this error is being reported due to the following recent change to Vulkan validation layers:

KhronosGroup/Vulkan-LoaderAndValidationLayers#2466

From the PR: While walking the shader for used descriptors, also determine whether any used buffers/images/texelbuffers are not decorated with NonWritable.

From the Vulkan spec:

vertexPipelineStoresAndAtomics indicates whether storage buffers and images support stores and atomic operations in the vertex, tessellation, and geometry shader stages. If this feature is not enabled, all storage image, storage texel buffers, and storage buffer variables used by these stages in shader modules must be decorated with the NonWriteable decoration (or the readonly memory qualifier in GLSL).

Here is the shader:

#version 310 es

layout(location = 0) in vec3 in_pos;
layout(location = 1) in vec3 in_normal;

layout(std140, set = 0, binding = 0) readonly buffer param_block {
vec3 light_pos;
vec3 light_color;
mat4 model;
mat4 view_projection;
float alpha;
} params;

layout(location = 0) out vec3 color;
layout(location = 1) out float alpha;

void main()
{
vec3 world_light = vec3(params.model * vec4(params.light_pos, 1.0));
vec3 world_pos = vec3(params.model * vec4(in_pos, 1.0));
vec3 world_normal = mat3(params.model) * in_normal;

vec3 light_dir = world_light - world_pos;
float brightness = dot(light_dir, world_normal) / length(light_dir) / length(world_normal);
brightness = abs(brightness);

gl_Position = params.view_projection * vec4(world_pos, 1.0);
color = params.light_color * brightness;
alpha = params.alpha;
}

and here is the SPIR-V:

// Module Version 10000
// Generated by (magic number): 80005
// Id's are bound by 114

                      Capability Shader
       1:             ExtInstImport  "GLSL.std.450"
                      MemoryModel Logical GLSL450
                      EntryPoint Vertex 4  "main" 38 67 89 102 109
                      Source ESSL 310
                      Name 4  "main"
                      Name 9  "world_light"
                      Name 12  "param_block"
                      MemberName 12(param_block) 0  "light_pos"
                      MemberName 12(param_block) 1  "light_color"
                      MemberName 12(param_block) 2  "model"
                      MemberName 12(param_block) 3  "view_projection"
                      MemberName 12(param_block) 4  "alpha"
                      Name 14  "params"
                      Name 34  "world_pos"
                      Name 38  "in_pos"
                      Name 49  "world_normal"
                      Name 67  "in_normal"
                      Name 70  "light_dir"
                      Name 75  "brightness"
                      Name 87  "gl_PerVertex"
                      MemberName 87(gl_PerVertex) 0  "gl_Position"
                      MemberName 87(gl_PerVertex) 1  "gl_PointSize"
                      Name 89  ""
                      Name 102  "color"
                      Name 109  "alpha"
                      MemberDecorate 12(param_block) 0 NonWritable
                      MemberDecorate 12(param_block) 0 Offset 0
                      MemberDecorate 12(param_block) 1 NonWritable
                      MemberDecorate 12(param_block) 1 Offset 16
                      MemberDecorate 12(param_block) 2 ColMajor
                      MemberDecorate 12(param_block) 2 NonWritable
                      MemberDecorate 12(param_block) 2 Offset 32
                      MemberDecorate 12(param_block) 2 MatrixStride 16
                      MemberDecorate 12(param_block) 3 ColMajor
                      MemberDecorate 12(param_block) 3 NonWritable
                      MemberDecorate 12(param_block) 3 Offset 96
                      MemberDecorate 12(param_block) 3 MatrixStride 16
                      MemberDecorate 12(param_block) 4 NonWritable
                      MemberDecorate 12(param_block) 4 Offset 160
                      Decorate 12(param_block) BufferBlock
                      Decorate 14(params) DescriptorSet 0
                      Decorate 14(params) Binding 0
                      Decorate 38(in_pos) Location 0
                      Decorate 67(in_normal) Location 1
                      MemberDecorate 87(gl_PerVertex) 0 BuiltIn Position
                      MemberDecorate 87(gl_PerVertex) 1 BuiltIn PointSize
                      Decorate 87(gl_PerVertex) Block
                      Decorate 102(color) Location 0
                      Decorate 109(alpha) Location 1
       2:             TypeVoid
       3:             TypeFunction 2
       6:             TypeFloat 32
       7:             TypeVector 6(float) 3
       8:             TypePointer Function 7(fvec3)
      10:             TypeVector 6(float) 4
      11:             TypeMatrix 10(fvec4) 4

12(param_block): TypeStruct 7(fvec3) 7(fvec3) 11 11 6(float)
13: TypePointer Uniform 12(param_block)
14(params): 13(ptr) Variable Uniform
15: TypeInt 32 1
16: 15(int) Constant 2
17: TypePointer Uniform 11
20: 15(int) Constant 0
21: TypePointer Uniform 7(fvec3)
24: 6(float) Constant 1065353216
37: TypePointer Input 7(fvec3)
38(in_pos): 37(ptr) Variable Input
52: TypeMatrix 7(fvec3) 3
53: 6(float) Constant 0
67(in_normal): 37(ptr) Variable Input
74: TypePointer Function 6(float)
87(gl_PerVertex): TypeStruct 10(fvec4) 6(float)
88: TypePointer Output 87(gl_PerVertex)
89: 88(ptr) Variable Output
90: 15(int) Constant 3
99: TypePointer Output 10(fvec4)
101: TypePointer Output 7(fvec3)
102(color): 101(ptr) Variable Output
103: 15(int) Constant 1
108: TypePointer Output 6(float)
109(alpha): 108(ptr) Variable Output
110: 15(int) Constant 4
111: TypePointer Uniform 6(float)

Each of the buffer variables in this shader (members of the shader storage block param_block) is decorated NonWritable, so no error should be emitted.

@karl-lunarg
Copy link
Contributor Author

Comment by greg-lunarg (MIGRATED)
Wednesday Mar 28, 2018 at 21:40 GMT


@Tony-LunarG

@chrisforbes
Copy link
Contributor

=> Mark as I don't have the free cycles to do this.

@zeux
Copy link

zeux commented Oct 7, 2018

Would it be possible to bump the priority for this issue?

As far as I understand,

  1. This happens for any read-only use of shader storage buffer objects with glslang. I just hit this myself on a Vulkan stream in the really basic example.
  2. The workaround involves enabling features that the application doesn't use - my guess is that people who aren't hitting this issue enable all features supported by the device which isn't a pattern that we should be encouraging.

@mark-lunarg
Copy link
Contributor

@zeux, yes, @chrisforbes bumped this and it is on my plate to be addressed in the next 2-4 weeks.

chrisforbes added a commit that referenced this issue Oct 9, 2018
Previously we only cared about whether the NonWritable decoration was
applied to an entire descriptor-backed OpVariable. This is problematic
for real-world shaders, because glslang emits per-member decorations
instead in all cases.

Adjust the logic to consider per-member decorations in these cases.
Fixes #73.
chrisforbes added a commit that referenced this issue Oct 10, 2018
Previously we only cared about whether the NonWritable decoration was
applied to an entire descriptor-backed OpVariable. This is problematic
for real-world shaders, because glslang emits per-member decorations
instead in all cases.

Adjust the logic to consider per-member decorations in these cases.
Fixes #73.
chrisforbes added a commit that referenced this issue Oct 11, 2018
Previously we only cared about whether the NonWritable decoration was
applied to an entire descriptor-backed OpVariable. This is problematic
for real-world shaders, because glslang emits per-member decorations
instead in all cases.

Adjust the logic to consider per-member decorations in these cases.
Fixes #73.
@shannon-lunarg shannon-lunarg added this to the sdk-1.1.temp.0 milestone Oct 19, 2018
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

No branches or pull requests

5 participants