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 validation fails for shadow sampler #8413

Closed
martin-ejdestig opened this issue Aug 20, 2024 · 9 comments
Closed

SPIR-V validation fails for shadow sampler #8413

martin-ejdestig opened this issue Aug 20, 2024 · 9 comments
Assignees
Labels
ShaderVal Shader Validation (SPIR-V related) SpecChange Issues that require a change in the Vulkan Spec

Comments

@martin-ejdestig
Copy link

martin-ejdestig commented Aug 20, 2024

Environment:

  • OS: Windows and Linux
  • GPU and driver version: Nvidia GeForce GTK 970 and Radeon RX 6700, latest drivers as of this writing (do not think it matters)
  • SDK or header version if building from repo: Noticed when upgrading to Vulkan SDK 1.3.290 from 1.3.280. Also reproduces with main (654d2ef).
  • Options enabled (synchronization, best practices, etc.): VK_VALIDATION_FEATURE_ENABLE_SYNCHRONIZATION_VALIDATION_EXT

Describe the Issue

After upgrading to Vulkan SDK 1.3.290, VVL outputs the following to debug callback when passing SPIR-V generated from GLSL that uses a shadow sampler to vkCreateShaderModule() and vkCreateGraphicsPipelines():

Validation Error: [ VUID-VkShaderModuleCreateInfo-pCode-08737 ] | MessageID = 0xa5625282 | vkCreateShaderModule(): pCreateInfo->pCode (spirv-val produced an error):
Expected Image to have the same type as Result Type Image
  %373 = OpSampledImage %372 %368 %370
. The Vulkan spec states: If pCode is a pointer to SPIR-V code, pCode must adhere to the validation rules described by the Validation Rules within a Module section of the SPIR-V Environment appendix (https://vulkan.lunarg.com/doc/view/1.3.290.0/windows/1.3-extensions/vkspec.html#VUID-VkShaderModuleCreateInfo-pCode-08737)
Validation Error: [ VUID-VkPipelineShaderStageCreateInfo-pSpecializationInfo-06849 ] | MessageID = 0x437c19d3 | vkCreateGraphicsPipelines(): pCreateInfos[0].pStages[1] After specialization was applied, VkShaderModule 0x5b26f60000000100[] produces a spirv-val error (stage VK_SHADER_STAGE_FRAGMENT_BIT):
Expected Image to have the same type as Result Type Image
  %373 = OpSampledImage %372 %368 %370
. The Vulkan spec states: If a shader module identifier is not specified, the shader code used by the pipeline must be valid as described by the Khronos SPIR-V Specification after applying the specializations provided in pSpecializationInfo, if any, and then converting all specialization constants into fixed constants (https://vulkan.lunarg.com/doc/view/1.3.290.0/windows/1.3-extensions/vkspec.html#VUID-VkPipelineShaderStageCreateInfo-pSpecializationInfo-06849)

Relevant SPIR-V code:

         %57 = OpTypeImage %float 2D 0 0 0 1 Unknown
%_ptr_UniformConstant_57 = OpTypePointer UniformConstant %57
...
%shadow_linear_compare_sampler = OpVariable %_ptr_UniformConstant_357 UniformConstant
        %371 = OpTypeImage %float 2D 1 0 0 1 Unknown
        %372 = OpTypeSampledImage %371
...
%shadow_map_sample_pcf2x2_t21_vf2_f1_ = OpFunction %float None %66
%shadow_map_atlas_0 = OpFunctionParameter %_ptr_UniformConstant_57
       %uv_0 = OpFunctionParameter %_ptr_Function_v2float
%shadow_pos_z = OpFunctionParameter %_ptr_Function_float
         %71 = OpLabel
               OpLine %10 188 0
        %368 = OpLoad %57 %shadow_map_atlas_0
        %370 = OpLoad %357 %shadow_linear_compare_sampler
        %373 = OpSampledImage %372 %368 %370
        %374 = OpLoad %v2float %uv_0
        %375 = OpLoad %float %shadow_pos_z
        %376 = OpCompositeExtract %float %374 0
        %377 = OpCompositeExtract %float %374 1
        %378 = OpCompositeConstruct %v3float %376 %377 %375
        %379 = OpCompositeExtract %float %378 2
        %380 = OpImageSampleDrefImplicitLod %float %373 %378 %379
               OpReturnValue %380
               OpFunctionEnd

Generated from the following GLSL code:

...
layout(set = 0, binding = 2) uniform sampler shadow_nearest_sampler;
layout(set = 0, binding = 3) uniform sampler shadow_linear_compare_sampler;
layout(set = 0, binding = 4) uniform texture2D shadow_map_atlas_directional;
layout(set = 0, binding = 5) uniform texture2D shadow_map_atlas_punctual;
...

// shadow_map_atlas_directional (and shadow_map_atlas_punctual) passed as argument.
float shadow_map_sample_pcf2x2(texture2D shadow_map_atlas, vec2 uv, float shadow_pos_z)
{
    return texture(sampler2DShadow(shadow_map_atlas, shadow_linear_compare_sampler), vec3(uv, shadow_pos_z));
}

I have not had time to test with a minimum example, the above is extracted from a much larger shader. But I think I have included all the relevant SPIR-V (and GLSL it is generated from) that triggers the error messages.

@martin-ejdestig
Copy link
Author

martin-ejdestig commented Aug 20, 2024

Searched through external/ as well and it looks like the "Expected Image to have the same type as Result Type Image" error message was added in KhronosGroup/SPIRV-Tools#5695 .

@martin-ejdestig
Copy link
Author

martin-ejdestig commented Aug 20, 2024

Below is complete debug SPIR-V assembly of minimal shader that warns when passed to vkCreateShaderModule(). There is no longer any warning when calling vkCreateGraphicsPipelines() since specialization constants are removed (I assume).

; SPIR-V
; Version: 1.6
; Generator: Google Shaderc over Glslang; 11
; Bound: 36
; Schema: 0
               OpCapability Shader
          %2 = OpExtInstImport "GLSL.std.450"
               OpMemoryModel Logical GLSL450
               OpEntryPoint Fragment %main "main" %shadow_map_atlas_directional %shadow_linear_compare_sampler %out_color
               OpExecutionMode %main OriginUpperLeft
          %1 = OpString "shaders/mesh_custom_test.frag"
               OpSource GLSL 460 %1 "#version 460

layout(location = 0) out vec4 out_color;

layout(set = 0, binding = 3) uniform sampler shadow_linear_compare_sampler;
layout(set = 0, binding = 4) uniform texture2D shadow_map_atlas_directional;

void main()
{
    float x = texture(sampler2DShadow(shadow_map_atlas_directional, shadow_linear_compare_sampler), vec3(0.5));
    out_color = vec4(vec3(x), 1.0);
}
"
               OpSourceExtension "GL_GOOGLE_cpp_style_line_directive"
               OpSourceExtension "GL_GOOGLE_include_directive"
               OpName %main "main"
               OpName %x "x"
               OpName %shadow_map_atlas_directional "shadow_map_atlas_directional"
               OpName %shadow_linear_compare_sampler "shadow_linear_compare_sampler"
               OpName %out_color "out_color"
               OpModuleProcessed "entry-point main"
               OpModuleProcessed "client vulkan100"
               OpModuleProcessed "target-env spirv1.6"
               OpModuleProcessed "target-env vulkan1.3"
               OpModuleProcessed "entry-point main"
               OpDecorate %shadow_map_atlas_directional DescriptorSet 0
               OpDecorate %shadow_map_atlas_directional Binding 4
               OpDecorate %shadow_linear_compare_sampler DescriptorSet 0
               OpDecorate %shadow_linear_compare_sampler Binding 3
               OpDecorate %out_color Location 0
       %void = OpTypeVoid
          %4 = OpTypeFunction %void
      %float = OpTypeFloat 32
%_ptr_Function_float = OpTypePointer Function %float
         %10 = OpTypeImage %float 2D 0 0 0 1 Unknown
%_ptr_UniformConstant_10 = OpTypePointer UniformConstant %10
%shadow_map_atlas_directional = OpVariable %_ptr_UniformConstant_10 UniformConstant
         %14 = OpTypeSampler
%_ptr_UniformConstant_14 = OpTypePointer UniformConstant %14
%shadow_linear_compare_sampler = OpVariable %_ptr_UniformConstant_14 UniformConstant
         %18 = OpTypeImage %float 2D 1 0 0 1 Unknown
         %19 = OpTypeSampledImage %18
    %v3float = OpTypeVector %float 3
  %float_0_5 = OpConstant %float 0.5
         %23 = OpConstantComposite %v3float %float_0_5 %float_0_5 %float_0_5
    %v4float = OpTypeVector %float 4
%_ptr_Output_v4float = OpTypePointer Output %v4float
  %out_color = OpVariable %_ptr_Output_v4float Output
    %float_1 = OpConstant %float 1
               OpLine %1 8 11
       %main = OpFunction %void None %4
          %6 = OpLabel
          %x = OpVariable %_ptr_Function_float Function
               OpLine %1 10 0
         %13 = OpLoad %10 %shadow_map_atlas_directional
         %17 = OpLoad %14 %shadow_linear_compare_sampler
         %20 = OpSampledImage %19 %13 %17
         %24 = OpCompositeExtract %float %23 2
         %25 = OpImageSampleDrefImplicitLod %float %20 %23 %24
               OpStore %x %25
               OpLine %1 11 0
         %29 = OpLoad %float %x
         %30 = OpCompositeConstruct %v3float %29 %29 %29
         %32 = OpCompositeExtract %float %30 0
         %33 = OpCompositeExtract %float %30 1
         %34 = OpCompositeExtract %float %30 2
         %35 = OpCompositeConstruct %v4float %32 %33 %34 %float_1
               OpStore %out_color %35
               OpReturn
               OpFunctionEnd

Error message:

Expected Image to have the same type as Result Type Image
  %20 = OpSampledImage %19 %13 %17
. The Vulkan spec states: If pCode is a pointer to SPIR-V code, pCode must adhere to the validation rules described by the Validation Rules within a Module section of the SPIR-V Environment appendix (https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-VkShaderModuleCreateInfo-pCode-08737)

@martin-ejdestig
Copy link
Author

martin-ejdestig commented Aug 20, 2024

Two things:

  1. I wonder if passing sampler instead of samplerShadow to sampler2DShadow() is valid. Reading the GLSL 4.60.8 specification section 5.4.5. Texture-Combined Sampler Constructors, it looks like it should be.
  2. If I change shadow_linear_compare_sampler to a samplerShadow in the simplified code (included in SPIR-V output), Depth in %18 = OpTypeImage changes to 1 (https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#OpTypeImage) and the VVL error message goes away. This does not work in the full shader. There is no difference in the SPIR-V generated assembly and I do not understand why. shadow_map_sample_pcf2x2() in extracted code from first post is the only place where shadow_linear_compare_sampler is used.

2 is moot if 1 is valid? There should be no warning here? I would also expect glslang/shaderc to error out if it was not valid. 🤷

@martin-ejdestig
Copy link
Author

martin-ejdestig commented Aug 20, 2024

Looks like it should be valid. Found test for it in https://github.com/KhronosGroup/glslang/blob/592de6cf78e0fb359fc3e2854c9dc0f3cf6d4820/Test/vulkan.frag#L25 . git blame on that line leads to KhronosGroup/glslang@7d4c9a0 which links to KhronosGroup/GLSL#22 .

@spencer-lunarg
Copy link
Contributor

So I am like 95% certain you hit this KhronosGroup/glslang#3653

@martin-ejdestig
Copy link
Author

martin-ejdestig commented Aug 20, 2024

Is it not "just" that the validation added in KhronosGroup/SPIRV-Tools#5695 is too strict? Is there something in the SPIR-V specification that says valid SPIR-V must be what SPIRV-Tools checks for? But then, that would be hard to align with what GLSL (and HLSL?) allow?

@spencer-lunarg
Copy link
Contributor

I guess it will depend if there was a case where some hardware vendor actually wasn't doing it correctly, therefor the old "valid" SPIR-V was invalid actually and glsl/hlsl will need to generate different logic... this for sure will require some discussion internally in a SPIR-V meeting to sort out, but hopefully will be sorted out soon

@spencer-lunarg spencer-lunarg added ShaderVal Shader Validation (SPIR-V related) SpecChange Issues that require a change in the Vulkan Spec labels Aug 20, 2024
@spencer-lunarg spencer-lunarg self-assigned this Aug 20, 2024
@spencer-lunarg
Copy link
Contributor

@martin-ejdestig we pulled in the spirv-val fix from KhronosGroup/SPIRV-Tools#5789 in #8516 and I am now 99% sure this is fixed now

this hit a LOT of people in the ecosystem

@martin-ejdestig
Copy link
Author

Verified that I do not get any warnings with ef846ac .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ShaderVal Shader Validation (SPIR-V related) SpecChange Issues that require a change in the Vulkan Spec
Projects
None yet
Development

No branches or pull requests

2 participants