glslang icon indicating copy to clipboard operation
glslang copied to clipboard

Broken SPIRV generated if using ternary operator ( " a ? b : c " ) as a direct argument to a texelFetch call

Open RikoOphorst opened this issue 3 months ago • 2 comments

#version 450

#extension GL_ARB_compute_shader : require

layout(set = 0, binding = 0) uniform texture2D globalTexture;
layout(set = 0, binding = 1) uniform sampler globalSampler;
layout(set = 0, binding = 2) uniform globalBufferType {
    int flip;
} globalBuffer;

void main()
{
    const uvec2 pixel = gl_GlobalInvocationID.xy;

#define GENERATE_BROKEN_SPIRV 1
#if GENERATE_BROKEN_SPIRV
    // breaks: ternary operation inside the texelFetch causes OpSampledImage to not be in the same block as the actual OpImage and the OpImageFetch
    const vec4 result = texelFetch(
        sampler2D(globalTexture, globalSampler),
        ivec2(int(pixel.x), globalBuffer.flip == 0 ? int(pixel.y) : 1280 - 1 - int(pixel.y)),
        0
    );
#else
    // works: take the ternary out of the texelFetch.
    ivec2 coord = ivec2(int(pixel.x), globalBuffer.flip == 0 ? int(pixel.y) : 1280 - 1 - int(pixel.y));
    const vec4 result = texelFetch(
        sampler2D(globalTexture, globalSampler),
        coord,
        0
    );
#endif
}

When above is compiled with glslangValidator from the latest SDK (1.3.280.0), using this command:

glslangValidator -V broken.comp

Broken SPIRV is generated by glslang:

; SPIR-V
; Version: 1.0
; Generator: Khronos Glslang Reference Front End; 11
; Bound: 63
; Schema: 0
               OpCapability Shader
          %1 = OpExtInstImport "GLSL.std.450"
               OpMemoryModel Logical GLSL450
               OpEntryPoint GLCompute %main "main" %gl_GlobalInvocationID
               OpExecutionMode %main LocalSize 1 1 1
               OpSource GLSL 450
               OpSourceExtension "GL_ARB_compute_shader"
               OpName %main "main"
               OpName %pixel "pixel"
               OpName %gl_GlobalInvocationID "gl_GlobalInvocationID"
               OpName %texelValue "texelValue"
               OpName %globalTexture "globalTexture"
               OpName %globalSampler "globalSampler"
               OpName %globalBufferType "globalBufferType"
               OpMemberName %globalBufferType 0 "flip"
               OpName %globalBuffer "globalBuffer"
               OpDecorate %gl_GlobalInvocationID BuiltIn GlobalInvocationId
               OpDecorate %globalTexture DescriptorSet 0
               OpDecorate %globalTexture Binding 0
               OpDecorate %globalSampler DescriptorSet 0
               OpDecorate %globalSampler Binding 1
               OpMemberDecorate %globalBufferType 0 Offset 0
               OpDecorate %globalBufferType Block
               OpDecorate %globalBuffer DescriptorSet 0
               OpDecorate %globalBuffer Binding 2
       %void = OpTypeVoid
          %3 = OpTypeFunction %void
       %uint = OpTypeInt 32 0
     %v2uint = OpTypeVector %uint 2
%_ptr_Function_v2uint = OpTypePointer Function %v2uint
     %v3uint = OpTypeVector %uint 3
%_ptr_Input_v3uint = OpTypePointer Input %v3uint
%gl_GlobalInvocationID = OpVariable %_ptr_Input_v3uint Input
      %float = OpTypeFloat 32
    %v4float = OpTypeVector %float 4
%_ptr_Function_v4float = OpTypePointer Function %v4float
         %19 = OpTypeImage %float 2D 0 0 0 1 Unknown
%_ptr_UniformConstant_19 = OpTypePointer UniformConstant %19
%globalTexture = OpVariable %_ptr_UniformConstant_19 UniformConstant
         %23 = OpTypeSampler
%_ptr_UniformConstant_23 = OpTypePointer UniformConstant %23
%globalSampler = OpVariable %_ptr_UniformConstant_23 UniformConstant
         %27 = OpTypeSampledImage %19
     %uint_0 = OpConstant %uint 0
%_ptr_Function_uint = OpTypePointer Function %uint
        %int = OpTypeInt 32 1
%globalBufferType = OpTypeStruct %int
%_ptr_Uniform_globalBufferType = OpTypePointer Uniform %globalBufferType
%globalBuffer = OpVariable %_ptr_Uniform_globalBufferType Uniform
      %int_0 = OpConstant %int 0
%_ptr_Uniform_int = OpTypePointer Uniform %int
       %bool = OpTypeBool
%_ptr_Function_int = OpTypePointer Function %int
     %uint_1 = OpConstant %uint 1
   %int_1279 = OpConstant %int 1279
      %v2int = OpTypeVector %int 2
       %main = OpFunction %void None %3
          %5 = OpLabel
      %pixel = OpVariable %_ptr_Function_v2uint Function
 %texelValue = OpVariable %_ptr_Function_v4float Function
         %45 = OpVariable %_ptr_Function_int Function
         %13 = OpLoad %v3uint %gl_GlobalInvocationID
         %14 = OpVectorShuffle %v2uint %13 %13 0 1
               OpStore %pixel %14
         %22 = OpLoad %19 %globalTexture
         %26 = OpLoad %23 %globalSampler
         %28 = OpSampledImage %27 %22 %26
         %31 = OpAccessChain %_ptr_Function_uint %pixel %uint_0
         %32 = OpLoad %uint %31
         %34 = OpBitcast %int %32
         %40 = OpAccessChain %_ptr_Uniform_int %globalBuffer %int_0
         %41 = OpLoad %int %40
         %43 = OpIEqual %bool %41 %int_0
               OpSelectionMerge %47 None
               OpBranchConditional %43 %46 %52
         %46 = OpLabel
         %49 = OpAccessChain %_ptr_Function_uint %pixel %uint_1
         %50 = OpLoad %uint %49
         %51 = OpBitcast %int %50
               OpStore %45 %51
               OpBranch %47
         %52 = OpLabel
         %54 = OpAccessChain %_ptr_Function_uint %pixel %uint_1
         %55 = OpLoad %uint %54
         %56 = OpBitcast %int %55
         %57 = OpISub %int %int_1279 %56
               OpStore %45 %57
               OpBranch %47
         %47 = OpLabel
         %58 = OpLoad %int %45
         %60 = OpCompositeConstruct %v2int %34 %58
         %61 = OpImage %19 %28
         %62 = OpImageFetch %v4float %61 %60 Lod %int_0
               OpStore %texelValue %62
               OpReturn
               OpFunctionEnd

spirv-val output:

error: line 67: All OpSampledImage instructions must be in the same block in which their Result <id> are consumed. OpSampledImage Result Type <id> '28[%28]' has a consumer in a different basic block. The consumer instruction <id> is '61[%61]'.
  %28 = OpSampledImage %27 %22 %26

If you set #define GENERATE_BROKEN_SPIRV 0, then the generated SPIRV will be valid and it will pass spirv-val. Hope this is enough for you guys to go on.

Thanks :)

RikoOphorst avatar Mar 27 '24 12:03 RikoOphorst

I have reproduced this and will investigate.

arcady-lunarg avatar Mar 27 '24 17:03 arcady-lunarg

Thanks @arcady-lunarg! Appreciate it!

RikoOphorst avatar Apr 02 '24 09:04 RikoOphorst