False positive validation for combined image sampler with comparison

[HansKristian-Work]

I'm trying to rev toolchain dependencies in SPIRV-Cross, and ran into a weird validation error that started appearing since last time I revved the toolchain.

GLSL:

#version 450

layout(set = 0, binding = 0) uniform texture2D uTex;
layout(set = 0, binding = 1) uniform sampler uSamp;
layout(location = 0) in vec2 vUV;
layout(location = 0) out float FragColor;

void main()
{
	FragColor = textureLod(sampler2DShadow(uTex, uSamp), vec3(vUV, 0.5), 0.0);
}

ASM:

; SPIR-V
; Version: 1.0
; Generator: Khronos Glslang Reference Front End; 11
; Bound: 32
; Schema: 0
               OpCapability Shader
          %1 = OpExtInstImport "GLSL.std.450"
               OpMemoryModel Logical GLSL450
               OpEntryPoint Fragment %main "main" %FragColor %vUV
               OpExecutionMode %main OriginUpperLeft
               OpSource GLSL 450
               OpName %main "main"
               OpName %FragColor "FragColor"
               OpName %uTex "uTex"
               OpName %uSamp "uSamp"
               OpName %vUV "vUV"
               OpDecorate %FragColor Location 0
               OpDecorate %uTex DescriptorSet 0
               OpDecorate %uTex Binding 0
               OpDecorate %uSamp DescriptorSet 0
               OpDecorate %uSamp Binding 1
               OpDecorate %vUV Location 0
       %void = OpTypeVoid
          %3 = OpTypeFunction %void
      %float = OpTypeFloat 32
%_ptr_Output_float = OpTypePointer Output %float
  %FragColor = OpVariable %_ptr_Output_float Output
          %9 = OpTypeImage %float 2D 0 0 0 1 Unknown
%_ptr_UniformConstant_9 = OpTypePointer UniformConstant %9
       %uTex = OpVariable %_ptr_UniformConstant_9 UniformConstant
         %13 = OpTypeSampler
%_ptr_UniformConstant_13 = OpTypePointer UniformConstant %13
      %uSamp = OpVariable %_ptr_UniformConstant_13 UniformConstant
         %17 = OpTypeImage %float 2D 1 0 0 1 Unknown
         %18 = OpTypeSampledImage %17
    %v2float = OpTypeVector %float 2
%_ptr_Input_v2float = OpTypePointer Input %v2float
        %vUV = OpVariable %_ptr_Input_v2float Input
  %float_0_5 = OpConstant %float 0.5
    %v3float = OpTypeVector %float 3
    %float_0 = OpConstant %float 0
       %main = OpFunction %void None %3
          %5 = OpLabel
         %12 = OpLoad %9 %uTex
         %16 = OpLoad %13 %uSamp
         %19 = OpSampledImage %18 %12 %16
         %23 = OpLoad %v2float %vUV
         %26 = OpCompositeExtract %float %23 0
         %27 = OpCompositeExtract %float %23 1
         %28 = OpCompositeConstruct %v3float %26 %27 %float_0_5
         %30 = OpCompositeExtract %float %28 2
         %31 = OpImageSampleDrefExplicitLod %float %19 %28 %30 Lod %float_0
               OpStore %FragColor %31
               OpReturn
               OpFunctionEnd

spirv-val commit b21dda0ee7a3ea4e0192a7b2b09db1df1de9d5e7:

error: line 41: Expected Image to have the same type as Result Type Image
  %19 = OpSampledImage %18 %12 %16

The problem happens in sampler2DShadow(tex, samp) where we now know the combined image sampler is a depth texture. I can't find any wording in the SPIR-V or Vulkan specs which state that the image type for OpTypeCombinedSampler must match the type of the image exactly.

Where in the spec does it say this? If so, that is a problem because otherwise, separate image samplers are not really possible to implement since an image can be used both as comparison sampler and non-comparison sampler in the same shader. Also, array of descriptors has to be considered where index 0 may be a depth texture and index 1 is not.

[HansKristian-Work]

@alan-baker I think it's this commit: 70ad4da. There is no reference to spec language in this commit.

This is breaking glslang output which has been used since day 1, which is a good indication this commit should be reverted.

[s-perron]

The description of OpSampledImage says:

Result Type must be the OpTypeSampledImage type whose Image Type operand is the type of Image.

https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#OpSampledImage

In this case, the image type changes because they have different depth:

          %9 = OpTypeImage %float 2D 0 0 0 1 Unknown
         %17 = OpTypeImage %float 2D 1 0 0 1 Unknown
         %18 = OpTypeSampledImage %17 
        %12 = OpLoad %9 %uTex
         %19 = OpSampledImage %18 %12 %16

[HansKristian-Work]

Filed an internal spec issue to relax this. Trying to validate for different Depth is the problem here, and "no-one" follows that rule, for good reason. Breaking the shader ecosystem this late is not ideal.

[alan-baker]

SPIR WG discussed this in the 2024-09-04 telecon. Decision was to relax the specific to allow the depth operand to mismatch due to the large existing corpus of shaders in violation of this rule.

Two steps for SPIRV-Tools:

1. revert the original PR
2. add validation that checks all the other operands of the image