Open slime73 opened 1 month ago
Thanks for another report, I'll look into this.
Turns out to be harder than expected.
Basically we know how to remap variables from arrays when they are indexed with an integer literal, because it is easy to append the index to the variable name, create a top level binding for it, and call it a day. When indexing an array with a variable, this trick doesn't work anymore since appending the variable name won't properly route to a top-level variable.
The only option I see to keep this behaviour transparent is to collapse array stacks entirely for opaque types. This is a bit more involved than I would like, and I'm not sure it is actually achievable.
Is it something that used to work but doesn't anymore ?
It's something that OpenGL GLSL supports, so my users have written shaders that use it which won't work on other backends.
I don't really like that it exists though, I could detect it and put in a warning if it's totally necessary but I'm hoping to avoid that to allow any OpenGL GLSL shader to work on Vulkan, Metal, etc. with my tool.
I think I managed to make something work, but it might change the way opaque uniforms locations are found. Considering I don't know how you handle them, would you mind giving it a try, see if it works for you ? Here is a link to the branch on my fork : https://github.com/sbourasse/glslang/tree/bourasse/array-collapse
If a sampler is in a struct and relaxed rules (-R) are used when generating SPIR-V code, it can have incorrect output.
Here's an example GLSL shader:
If I run
glslang -V -R --aml --amb myfile.frag
I get this if I disassemble the result. It uses an extramyLighti_shadowMap
uniform with no binding, which seems like it shouldn't happen.bad SPIR-V disassembly
``` ; SPIR-V ; Version: 1.0 ; Generator: Khronos Glslang Reference Front End; 11 ; Bound: 73 ; Schema: 0 OpCapability Shader %1 = OpExtInstImport "GLSL.std.450" OpMemoryModel Logical GLSL450 OpEntryPoint Fragment %main "main" %VarVertexCoord %FragColor OpExecutionMode %main OriginUpperLeft OpSource GLSL 450 OpName %main "main" OpName %accumulatedShadow "accumulatedShadow" OpName %i "i" OpName %myLighti_shadowMap "myLighti.shadowMap" OpName %VarVertexCoord "VarVertexCoord" OpName %otherShadowMap "otherShadowMap" OpName %FragColor "FragColor" OpName %myLight_0__shadowMap "myLight[0].shadowMap" OpName %myLight_1__shadowMap "myLight[1].shadowMap" OpName %MyLightStruct "MyLightStruct" OpMemberName %MyLightStruct 0 "/*shadowMap*/" OpName %gl_DefaultUniformBlock "gl_DefaultUniformBlock" OpMemberName %gl_DefaultUniformBlock 0 "myLight" OpName %_ "" OpDecorate %VarVertexCoord Location 0 OpDecorate %otherShadowMap DescriptorSet 0 OpDecorate %otherShadowMap Binding 0 OpDecorate %FragColor Location 0 OpDecorate %myLight_0__shadowMap DescriptorSet 0 OpDecorate %myLight_0__shadowMap Binding 0 OpDecorate %myLight_1__shadowMap DescriptorSet 0 OpDecorate %myLight_1__shadowMap Binding 0 OpMemberDecorate %MyLightStruct 0 Offset 0 OpDecorate %_arr_MyLightStruct_uint_2 ArrayStride 16 OpMemberDecorate %gl_DefaultUniformBlock 0 Offset 0 OpDecorate %gl_DefaultUniformBlock Block OpDecorate %_ DescriptorSet 0 OpDecorate %_ Binding 0 %void = OpTypeVoid %3 = OpTypeFunction %void %float = OpTypeFloat 32 %_ptr_Function_float = OpTypePointer Function %float %float_0 = OpConstant %float 0 %int = OpTypeInt 32 1 %_ptr_Function_int = OpTypePointer Function %int %int_0 = OpConstant %int 0 %int_2 = OpConstant %int 2 %bool = OpTypeBool %23 = OpTypeImage %float 2D 1 0 0 1 Unknown %24 = OpTypeSampledImage %23 %_ptr_UniformConstant_24 = OpTypePointer UniformConstant %24 %myLighti_shadowMap = OpVariable %_ptr_UniformConstant_24 UniformConstant %v2float = OpTypeVector %float 2 %_ptr_Input_v2float = OpTypePointer Input %v2float %VarVertexCoord = OpVariable %_ptr_Input_v2float Input %float_0_5 = OpConstant %float 0.5 %v3float = OpTypeVector %float 3 %int_1 = OpConstant %int 1 %otherShadowMap = OpVariable %_ptr_UniformConstant_24 UniformConstant %v4float = OpTypeVector %float 4 %_ptr_Output_v4float = OpTypePointer Output %v4float %FragColor = OpVariable %_ptr_Output_v4float Output %float_1 = OpConstant %float 1 %myLight_0__shadowMap = OpVariable %_ptr_UniformConstant_24 UniformConstant %myLight_1__shadowMap = OpVariable %_ptr_UniformConstant_24 UniformConstant %MyLightStruct = OpTypeStruct %int %uint = OpTypeInt 32 0 %uint_2 = OpConstant %uint 2 %_arr_MyLightStruct_uint_2 = OpTypeArray %MyLightStruct %uint_2 %gl_DefaultUniformBlock = OpTypeStruct %_arr_MyLightStruct_uint_2 %_ptr_Uniform_gl_DefaultUniformBlock = OpTypePointer Uniform %gl_DefaultUniformBlock %_ = OpVariable %_ptr_Uniform_gl_DefaultUniformBlock Uniform %main = OpFunction %void None %3 %5 = OpLabel %accumulatedShadow = OpVariable %_ptr_Function_float Function %i = OpVariable %_ptr_Function_int Function OpStore %accumulatedShadow %float_0 OpStore %i %int_0 OpBranch %14 %14 = OpLabel OpLoopMerge %16 %17 None OpBranch %18 %18 = OpLabel %19 = OpLoad %int %i %22 = OpSLessThan %bool %19 %int_2 OpBranchConditional %22 %15 %16 %15 = OpLabel %27 = OpLoad %24 %myLighti_shadowMap %31 = OpLoad %v2float %VarVertexCoord %34 = OpCompositeExtract %float %31 0 %35 = OpCompositeExtract %float %31 1 %36 = OpCompositeConstruct %v3float %34 %35 %float_0_5 %37 = OpCompositeExtract %float %36 2 %38 = OpImageSampleDrefImplicitLod %float %27 %36 %37 %39 = OpLoad %float %accumulatedShadow %40 = OpFAdd %float %39 %38 OpStore %accumulatedShadow %40 OpBranch %17 %17 = OpLabel %41 = OpLoad %int %i %43 = OpIAdd %int %41 %int_1 OpStore %i %43 OpBranch %14 %16 = OpLabel %45 = OpLoad %24 %otherShadowMap %46 = OpLoad %v2float %VarVertexCoord %47 = OpCompositeExtract %float %46 0 %48 = OpCompositeExtract %float %46 1 %49 = OpCompositeConstruct %v3float %47 %48 %float_0_5 %50 = OpCompositeExtract %float %49 2 %51 = OpImageSampleDrefImplicitLod %float %45 %49 %50 %52 = OpLoad %float %accumulatedShadow %53 = OpFAdd %float %52 %51 OpStore %accumulatedShadow %53 %57 = OpLoad %float %accumulatedShadow %58 = OpCompositeConstruct %v3float %57 %57 %57 %60 = OpCompositeExtract %float %58 0 %61 = OpCompositeExtract %float %58 1 %62 = OpCompositeExtract %float %58 2 %63 = OpCompositeConstruct %v4float %60 %61 %62 %float_1 OpStore %FragColor %63 OpReturn OpFunctionEnd ```If I then run SPIRV-Cross to generate a Metal shader from the SPIR-V via
spirv-cross --msl --msl-version 20100 frag.spv
, this is the result which also seems broken - it's accessing a singlemyLighti_shadowMap
uniform in the loop instead of accessing something different in each iteration.bad MSL code
```cpp #includeFYI @sbourasse
Tested with the latest glslang release (14.2.0).