KhronosGroup / glslang

Khronos-reference front end for GLSL/ESSL, partial front end for HLSL, and a SPIR-V generator.
Other
2.98k stars 822 forks source link

SPIR-V: DecorationNonUniformEXT is not propagated into OpSampledImage #1826

Open HansKristian-Work opened 5 years ago

HansKristian-Work commented 5 years ago

I have this test shader:

#version 450
#extension GL_EXT_nonuniform_qualifier : require

layout(binding = 0) uniform texture2D uSamplers[];
layout(binding = 4) uniform sampler2D uCombinedSamplers[];
layout(binding = 1) uniform sampler uSamps[];
layout(location = 0) flat in int vIndex;
layout(location = 1) in vec2 vUV;
layout(location = 0) out vec4 FragColor;

layout(set = 0, binding = 2) uniform UBO 
{
    vec4 v[64];
} ubos[];

layout(set = 0, binding = 3) readonly buffer SSBO
{
    vec4 v[];
} ssbos[];

void main()
{
    int i = vIndex;
    FragColor = texture(sampler2D(uSamplers[nonuniformEXT(i + 10)], uSamps[nonuniformEXT(i + 40)]), vUV);
    FragColor = texture(uCombinedSamplers[nonuniformEXT(i + 10)], vUV);
    FragColor += ubos[nonuniformEXT(i + 20)].v[nonuniformEXT(i + 40)];
    FragColor += ssbos[nonuniformEXT(i + 50)].v[nonuniformEXT(i + 60)];
}

Commit: 4b4b41a63499d34c527ee4f714dde8072f60c900. The SPIR-V spec says that the parameter passed to sampling instructions must be marked NonUniformEXT, but only the image and samplers are marked as such here.

                              Decorate 26 DecorationNonUniformEXT
                              Decorate 34 DecorationNonUniformEXT
                              Decorate 37 DecorationNonUniformEXT

              25:     24(ptr) AccessChain 19(uSamplers) 23
              26:          16 Load 25
              31:      6(int) Load 8(i)
              33:      6(int) IAdd 31 32
              34:      6(int) CopyObject 33
              36:     35(ptr) AccessChain 30(uSamps) 34
              37:          27 Load 36
              39:          38 SampledImage 26 37
              43:   40(fvec2) Load 42(vUV)
              44:   13(fvec4) ImageSampleImplicitLod 39 43
alan-baker commented 5 years ago

Related to this, the Vulkan spec says:

If an instruction loads from or stores to a resource (including atomics and image instructions) and the resource descriptor being accessed is not dynamically uniform, then the operand corresponding to that resource (e.g. the pointer or sampled image operand) must be decorated with NonUniformEXT.

In the glslang test, spv.nonuniform2.frag:

#version 450
#extension GL_EXT_nonuniform_qualifier : require
layout(set=0,binding=4,rgba32f) uniform imageBuffer data[];
layout(location = 0) out vec4     FragColor;
layout(location = 3) in flat int  rIndex;
void main()
{
  FragColor = imageLoad(data[nonuniformEXT(rIndex)], 0);
}

Generates:

                              Decorate 18 DecorationNonUniformEXT
                              Decorate 21 DecorationNonUniformEXT
...
              17:     14(int) Load 16(rIndex)
              18:     14(int) CopyObject 17
              20:     19(ptr) AccessChain 13(data) 18
              21:          10 Load 20

It seems like id 20 should be decorated with NonUniformEXT according to the Vulkan spec.

alan-baker commented 5 years ago

@jeffbolznv any opinions on the specifics of what should be decorated with NonUniformEXT exactly?

jeffbolznv commented 5 years ago

The relevant part of the Vulkan SPIR-V environment is:

  * If an instruction loads from or stores to a resource (including atomics
    and image instructions) and the resource descriptor being accessed is
    not dynamically uniform, then the operand corresponding to that resource
    (e.g. the pointer or sampled image operand) must: be decorated with
    code:NonUniformEXT.

So in the first shader id 39 should be NonUniformEXT, and in the second shader id 21 should be NonUniformEXT. Additional ids being NonUniformEXT is harmless, but doesn't really have any effect.

The motivation for this is that implementations may need to know at call time whether they need to loop over all the unique resources.

alan-baker commented 5 years ago

If I modify the second shader to be:

#version 450
#extension GL_EXT_nonuniform_qualifier : require
layout(set=0,binding=4,rgba32f) uniform imageBuffer data[];
layout(location = 0) out vec4     FragColor;
layout(location = 3) in flat int  rIndex;

layout(set=1,binding=0) buffer bname { vec4 f; } ssbo_in[];

void main()
{
  vec4 a = ssbo_in[nonuniformEXT(rIndex)].f;
  FragColor = imageLoad(data[nonuniformEXT(rIndex)], 0);
}

I get:

                              Decorate 18 DecorationNonUniformEXT
                              Decorate 22 DecorationNonUniformEXT
...
              17:     14(int) Load 16(rIndex)
              18:     14(int) CopyObject 17
              21:     20(ptr) AccessChain 13(ssbo_in) 18 19
              22:    7(fvec4) Load 21

It seems like 21 should be decorated, but is not. Am I interpreting this correctly?

jeffbolznv commented 5 years ago

I agree, 21 should be decorated in that case.