KhronosGroup / KhronosGroup/glslang

Incorrect SPIR-V can be generated when using samplers in structs with relaxed rules

Open
#3,594 4 comments 0 reactions 0 assignees View on GitHub
relaxed
Dominant language
C++
Stars
3.6k
Forks
989
Avg merge
1d 2h
Merged PRs (30d)
31

Description

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:
```glsl
#version 450 core

in vec2 VarVertexCoord;

struct MyLightStruct {
sampler2DShadow shadowMap;
};

uniform sampler2DShadow otherShadowMap;
uniform MyLightStruct[2] myLight;
out vec4 FragColor;

void main() {
float accumulatedShadow = 0.0;

for (int i = 0; i < 2; i++) {
accumulatedShadow += texture(myLight[i].shadowMap, vec3(VarVertexCoord, 0.5));
}

accumulatedShadow += texture(otherShadowMap, vec3(VarVertexCoord, 0.5));

FragColor = vec4(vec3(accumulatedShadow), 1.0);
}
```

If I run `glslang -V -R --aml --amb myfile.frag` I get this if I disassemble the result. It uses an extra `myLighti_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 single `myLighti_shadowMap` uniform in the loop instead of accessing something different in each iteration.

bad MSL code

```cpp
#include
#include

using namespace metal;

struct MyLightStruct
{
int _shadowMap_;
};

struct _RESERVED_IDENTIFIER_FIXUP_gl_DefaultUniformBlock
{
MyLightStruct myLight[2];
};

struct main0_out
{
float4 FragColor [[color(0)]];
};

struct main0_in
{
float2 VarVertexCoord [[user(locn0)]];
};

fragment main0_out main0(main0_in in [[stage_in]], depth2d myLighti_shadowMap [[texture(0)]], depth2d otherShadowMap [[texture(1)]], sampler myLighti_shadowMapSmplr [[sampler(0)]], sampler otherShadowMapSmplr [[sampler(1)]])
{
main0_out out = {};
float accumulatedShadow = 0.0;
for (int i = 0; i < 2; i++)
{
float3 _36 = float3(in.VarVertexCoord, 0.5);
accumulatedShadow += myLighti_shadowMap.sample_compare(myLighti_shadowMapSmplr, _36.xy, _36.z);
}
float3 _49 = float3(in.VarVertexCoord, 0.5);
accumulatedShadow += otherShadowMap.sample_compare(otherShadowMapSmplr, _49.xy, _49.z);
out.FragColor = float4(float3(accumulatedShadow), 1.0);
return out;
}
```

FYI @sbourasse

Tested with the latest glslang release (14.2.0).

Contributor guide

Open the contributing guide

Research direction

Reproduce the issue with the GLSL shader and `glslang -V -R --aml --amb myfile.frag`, then inspect the SPIR-V disassembly and generated MSL shown in the report. Done means the loop does not use an extra unbound sampler uniform and preserves distinct sampler access for each struct-array element.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.