KhronosGroup / glslang

Khronos-reference front end for GLSL/ESSL, partial front end for HLSL, and a SPIR-V generator.
Other
2.9k stars 816 forks source link

Vulkan Relaxed Outputs Extra Sampler Variables #3526

Closed bjornbytes closed 3 months ago

bjornbytes commented 4 months ago

I updated glslang and noticed that in vulkan-relaxed mode it started outputting extra combined image sampler variables without set/binding decorations. Here's a simple example that demonstrates the issue:

#version 460

layout(set = 0, binding = 0) uniform texture2D tex;
layout(set = 0, binding = 1) uniform sampler samp;

layout(location = 0) out vec4 pixel;

void main() {
  pixel = texture(sampler2D(tex, samp), vec2(0.));
}

Compiling this with glslangValidator -V -R shader.frag and then validating with spirv-val --target-env vulkan1.1 frag.spv outputs the following:

error: line 26: UniformConstant id '13' is missing DescriptorSet decoration.
From Vulkan spec, section 14.5.2:
These variables must have DescriptorSet and Binding decorations specified
  %texsamp = OpVariable %_ptr_UniformConstant_11 UniformConstant

I bisected and landed on c59b876ca0f5b672d7cfeb4d591b346e97b1966c as the commit that introduced the issue.

Before the change (or when removing -R), the working shader looked like this:

       %main = OpFunction %void None %3
          %5 = OpLabel
         %13 = OpLoad %10 %tex
         %17 = OpLoad %14 %samp
         %19 = OpSampledImage %18 %13 %17
         %23 = OpImageSampleImplicitLod %v4float %19 %22
               OpStore %pixel %23
               OpReturn
               OpFunctionEnd

After the change, the combined image sampler was hoisted to top-level scope:

         %10 = OpTypeImage %float 2D 0 0 0 1 Unknown
         %11 = OpTypeSampledImage %10
%_ptr_UniformConstant_11 = OpTypePointer UniformConstant %11
    %texsamp = OpVariable %_ptr_UniformConstant_11 UniformConstant
...
       %main = OpFunction %void None %3
          %5 = OpLabel
         %14 = OpLoad %11 %texsamp
         %18 = OpImageSampleImplicitLod %v4float %14 %17
               OpStore %pixel %18
               OpReturn
               OpFunctionEnd

I don't really understand c59b876ca0f5b672d7cfeb4d591b346e97b1966c yet, but it looks like it might be overeager about converting samplers? There aren't any samplers in structs or function parameters here, so I wouldn't expect any change to the spirv, compared to non-relaxed mode.

bjornbytes commented 3 months ago

Fixed in #3558