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

Incorrect SPIR-V generated when using a fixed-size array of sampler2D with -R (relaxed vulkan rules) #3536

Closed slime73 closed 3 months ago

slime73 commented 4 months ago

I'm using glslang's SPIR-V generator together with SPIRV-Cross to cross-compile OpenGL GLSL to MSL. I believe this bug is in glslang rather than SPIRV-Cross but I'm happy to move this report if that's not the case.

I'm using this command to generate SPIR-V: glslang -V -R --amb --aml test.frag test.frag has the following code:

#version 450 core

out vec4 outColor;

uniform sampler2D mainTex;
uniform sampler2D textures[5];

void main()
{
    outColor = texture(textures[1], vec2(0.5)) + texture(textures[3], vec2(0.5)) + texture(mainTex, vec2(0.5));
}

And then I run spirv-cross --msl --msl-version 20100 frag.spv to get the final MSL shader. With -R (as above), this is the result. it's only declaring and sampling from one of the textures in the original array, and the name of the texture is slightly mangled:

bad SPIR-V disassembly ``` ; SPIR-V ; Version: 1.0 ; Generator: Khronos Glslang Reference Front End; 11 ; Bound: 31 ; Schema: 0 OpCapability Shader %1 = OpExtInstImport "GLSL.std.450" OpMemoryModel Logical GLSL450 OpEntryPoint Fragment %main "main" %outColor OpExecutionMode %main OriginUpperLeft OpSource GLSL 450 OpName %main "main" OpName %outColor "outColor" OpName %textures_1_ "textures[1]" OpName %mainTex "mainTex" OpName %textures "textures" OpDecorate %outColor Location 0 OpDecorate %mainTex DescriptorSet 0 OpDecorate %mainTex Binding 0 OpDecorate %textures DescriptorSet 0 OpDecorate %textures Binding 0 %void = OpTypeVoid %3 = OpTypeFunction %void %float = OpTypeFloat 32 %v4float = OpTypeVector %float 4 %_ptr_Output_v4float = OpTypePointer Output %v4float %outColor = OpVariable %_ptr_Output_v4float Output %10 = OpTypeImage %float 2D 0 0 0 1 Unknown %11 = OpTypeSampledImage %10 %_ptr_UniformConstant_11 = OpTypePointer UniformConstant %11 %textures_1_ = OpVariable %_ptr_UniformConstant_11 UniformConstant %v2float = OpTypeVector %float 2 %float_0_5 = OpConstant %float 0.5 %17 = OpConstantComposite %v2float %float_0_5 %float_0_5 %mainTex = OpVariable %_ptr_UniformConstant_11 UniformConstant %uint = OpTypeInt 32 0 %uint_5 = OpConstant %uint 5 %_arr_11_uint_5 = OpTypeArray %11 %uint_5 %_ptr_UniformConstant__arr_11_uint_5 = OpTypePointer UniformConstant %_arr_11_uint_5 %textures = OpVariable %_ptr_UniformConstant__arr_11_uint_5 UniformConstant %main = OpFunction %void None %3 %5 = OpLabel %14 = OpLoad %11 %textures_1_ %18 = OpImageSampleImplicitLod %v4float %14 %17 %19 = OpLoad %11 %textures_1_ %20 = OpImageSampleImplicitLod %v4float %19 %17 %21 = OpFAdd %v4float %18 %20 %23 = OpLoad %11 %mainTex %24 = OpImageSampleImplicitLod %v4float %23 %17 %25 = OpFAdd %v4float %21 %24 OpStore %outColor %25 OpReturn OpFunctionEnd ```
bad MSL code ```cpp #include #include using namespace metal; struct main0_out { float4 outColor [[color(0)]]; }; fragment main0_out main0(texture2d textures_1_ [[texture(0)]], texture2d mainTex [[texture(1)]], sampler textures_1_Smplr [[sampler(0)]], sampler mainTexSmplr [[sampler(1)]]) { main0_out out = {}; out.outColor = (textures_1_.sample(textures_1_Smplr, float2(0.5)) + textures_1_.sample(textures_1_Smplr, float2(0.5))) + mainTex.sample(mainTexSmplr, float2(0.5)); return out; } ```

If I remove -R from the initial glslang command, I get working MSL code after converting:

good SPIR-V disassembly ``` ; SPIR-V ; Version: 1.0 ; Generator: Khronos Glslang Reference Front End; 11 ; Bound: 35 ; Schema: 0 OpCapability Shader %1 = OpExtInstImport "GLSL.std.450" OpMemoryModel Logical GLSL450 OpEntryPoint Fragment %main "main" %outColor OpExecutionMode %main OriginUpperLeft OpSource GLSL 450 OpName %main "main" OpName %outColor "outColor" OpName %textures "textures" OpName %mainTex "mainTex" OpDecorate %outColor Location 0 OpDecorate %textures DescriptorSet 0 OpDecorate %textures Binding 1 OpDecorate %mainTex DescriptorSet 0 OpDecorate %mainTex Binding 0 %void = OpTypeVoid %3 = OpTypeFunction %void %float = OpTypeFloat 32 %v4float = OpTypeVector %float 4 %_ptr_Output_v4float = OpTypePointer Output %v4float %outColor = OpVariable %_ptr_Output_v4float Output %10 = OpTypeImage %float 2D 0 0 0 1 Unknown %11 = OpTypeSampledImage %10 %uint = OpTypeInt 32 0 %uint_5 = OpConstant %uint 5 %_arr_11_uint_5 = OpTypeArray %11 %uint_5 %_ptr_UniformConstant__arr_11_uint_5 = OpTypePointer UniformConstant %_arr_11_uint_5 %textures = OpVariable %_ptr_UniformConstant__arr_11_uint_5 UniformConstant %int = OpTypeInt 32 1 %int_1 = OpConstant %int 1 %_ptr_UniformConstant_11 = OpTypePointer UniformConstant %11 %v2float = OpTypeVector %float 2 %float_0_5 = OpConstant %float 0.5 %24 = OpConstantComposite %v2float %float_0_5 %float_0_5 %int_3 = OpConstant %int 3 %mainTex = OpVariable %_ptr_UniformConstant_11 UniformConstant %main = OpFunction %void None %3 %5 = OpLabel %20 = OpAccessChain %_ptr_UniformConstant_11 %textures %int_1 %21 = OpLoad %11 %20 %25 = OpImageSampleImplicitLod %v4float %21 %24 %27 = OpAccessChain %_ptr_UniformConstant_11 %textures %int_3 %28 = OpLoad %11 %27 %29 = OpImageSampleImplicitLod %v4float %28 %24 %30 = OpFAdd %v4float %25 %29 %32 = OpLoad %11 %mainTex %33 = OpImageSampleImplicitLod %v4float %32 %24 %34 = OpFAdd %v4float %30 %33 OpStore %outColor %34 OpReturn OpFunctionEnd ```
good MSL code ```cpp #include #include using namespace metal; struct main0_out { float4 outColor [[color(0)]]; }; fragment main0_out main0(array, 5> textures [[texture(0)]], texture2d mainTex [[texture(5)]], array texturesSmplr [[sampler(0)]], sampler mainTexSmplr [[sampler(5)]]) { main0_out out = {}; out.outColor = (textures[1].sample(texturesSmplr[1], float2(0.5)) + textures[3].sample(texturesSmplr[3], float2(0.5))) + mainTex.sample(mainTexSmplr, float2(0.5)); return out; } ```

The binding numbers also don't appear correct in the disassembled SPIR-V when -R is used in that example. In my main project that's causing additional issues but I guess the spirv-cross command I used for this test might be working around that part of the problem.

EDIT: I just saw #3526 after creating this issue. It's not identical but maybe it's related?

slime73 commented 4 months ago

FYI @sbourasse, I just tested with a build of c59b876 (Implement relaxed rule for opaque struct members) as well as a build of the commit before that one, and the problem exists with that build but not the previous commit, so it looks like that change introduced this regression.

sbourasse commented 4 months ago

I'll look into it ASAP, thanks for letting me know.

slime73 commented 3 months ago

0015dc9 fixes the issues I was having. Thanks!