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

SPIR-V output fails VUID-StandaloneSpirv-Flat-06202, VUID-StandaloneSpirv-Flat-04744 #2032

Closed
jimblandy opened this issue Aug 27, 2022 · 4 comments · Fixed by #2035
Closed
Labels
area: back-end Outputs of shader conversion help wanted Extra attention is needed kind: bug Something isn't working lang: SPIR-V Binary SPIR-V input and output

Comments

@jimblandy
Copy link
Member

jimblandy commented Aug 27, 2022

Using naga e7ddd35 to compile wgpu test and example shaders to SPIR-V produces code that fails the standalone validation rules VUID-StandaloneSpirv-Flat-06202 and VUID-StandaloneSpirv-Flat-04744.

For example:

$ cd wgpu/wgpu/tests/shader_primitive_index
$ naga primitive_index.wgsl primitive_index.spv
$ spirv-val --target-env vulkan1.0 primitive_index.spv 
error: line 29: [VUID-StandaloneSpirv-Flat-04744] Fragment OpEntryPoint operand 27 with Input interfaces with integer or float type must have a Flat decoration for Entry Point id 31.
  %gl_PrimitiveID = OpVariable %_ptr_Input_uint Input

$ cd ../../examples/water/
$ naga water.wgsl water.spv
$ spirv-val --target-env vulkan1.0 water.spv 
error: line 139: [VUID-StandaloneSpirv-Flat-06202] OpEntryPoint interfaces variable must not be vertex execution model with an input storage class for Entry Point id 479.
  %465 = OpVariable %_ptr_Input_v2int Input

$ git rev-parse --short HEAD
2cd08a1c
@jimblandy jimblandy added kind: bug Something isn't working lang: SPIR-V Binary SPIR-V input and output area: back-end Outputs of shader conversion help wanted Extra attention is needed labels Aug 29, 2022
@nical
Copy link
Contributor

nical commented Aug 31, 2022

The primitive_index test case can be reduced down to:

@fragment
fn fs_main(@builtin(primitive_index) index: u32) -> @location(0) vec4<f32> {
    return vec4<f32>(0.0, 0.0, 0.0, 0.0);
}
$ naga reduced.wgsl reduced.spv && spirv-val --target-env vulkan1.0 reduced.spv
error: line 15: [VUID-StandaloneSpirv-Flat-04744] Fragment OpEntryPoint operand 8 with Input interfaces with integer or float type must have a Flat decoration for Entry Point id 13.
  %gl_PrimitiveID = OpVariable %_ptr_Input_uint Input
$ spirv-dis ./reduced.spv                     
; SPIR-V
; Version: 1.0
; Generator: Khronos; 28
; Bound: 17
; Schema: 0
               OpCapability Shader
               OpCapability Geometry
          %1 = OpExtInstImport "GLSL.std.450"
               OpMemoryModel Logical GLSL450
               OpEntryPoint Fragment %13 "fs_main" %gl_PrimitiveID %11
               OpExecutionMode %13 OriginUpperLeft
               OpDecorate %gl_PrimitiveID BuiltIn PrimitiveId
               OpDecorate %11 Location 0
       %void = OpTypeVoid
      %float = OpTypeFloat 32
    %float_0 = OpConstant %float 0
       %uint = OpTypeInt 32 0
    %v4float = OpTypeVector %float 4
%_ptr_Input_uint = OpTypePointer Input %uint
%gl_PrimitiveID = OpVariable %_ptr_Input_uint Input
%_ptr_Output_v4float = OpTypePointer Output %v4float
         %11 = OpVariable %_ptr_Output_v4float Output
         %14 = OpTypeFunction %void
         %13 = OpFunction %void None %14
          %7 = OpLabel
         %10 = OpLoad %uint %gl_PrimitiveID
               OpBranch %15
         %15 = OpLabel
         %16 = OpCompositeConstruct %v4float %float_0 %float_0 %float_0 %float_0
               OpStore %11 %16
               OpReturn
               OpFunctionEnd

The issue appears to be with what naga generates for the argument@builtin(primitive_index) index: u32.

@nical
Copy link
Contributor

nical commented Aug 31, 2022

Inserting OpDecorate %gl_PrimitiveID Flat in the text version near the other OpDecorate instructions appears to fix the problem.

I tested that using:

$ spirv-dis ./reduced.spv > spvtext

then edited the spvtext file and validated it with:

$ spirv-as --target-env vulkan1.0 spvtext && spirv-val --target-env vulkan1.0 out.spv

@nical
Copy link
Contributor

nical commented Aug 31, 2022

Moving the investigation of the water example to #2036.

@jimblandy
Copy link
Member Author

jimblandy commented Aug 31, 2022

I got some background on what's going on here. These are rules that have long been in the Vulkan spec, but the validation layers and spirv-val have not caught up to the spec. Recently there's been some contributor who's been going around submitting PRs that implement more validation, and the SPIRV-Tools maintainers have been merging them, since they're correctly following the spec.

So it's just coincidence that we were getting away with it before, and we can expect more changes in the future raising the standard of the actual validation tools closer to the spec.

nical added a commit to nical/naga that referenced this issue Sep 1, 2022
nical added a commit to nical/naga that referenced this issue Sep 1, 2022
nical added a commit to nical/naga that referenced this issue Sep 1, 2022
jimblandy pushed a commit to nical/naga that referenced this issue Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: back-end Outputs of shader conversion help wanted Extra attention is needed kind: bug Something isn't working lang: SPIR-V Binary SPIR-V input and output
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants