glslang places the Restrict decoration on structure members

[gfxstrand]

This is not allowed by the SPIR-V spec. The spec for Restrict says:

Apply to a variable, to indicate the compiler may compile as if there is no aliasing. See the Aliasing section for more detail.

[johnkslang]

I think there may be a specification issue here. GLSL allows restrict on individual members of a block.

However, it is somewhat illogical that this can be per member, as will as with other memory qualifier, as the entire block is a single binding.

Do you think we should

• change GLSL to have these only at the full block level
• promote from member to block when making SPIR-V
• change SPIR-V to accept per-member Restrict

[gfxstrand]

I would really rather we keep SPIR-V sensible and not allow per-member Restrict. Of the three options, I think the first makes the most sense unless there's content out there which depends on per-member restrict.

[anholt]

This is still causing Mesa to throw warnings about invalid SPIR-V. Any chance of getting a resolution to it?

[greg-lunarg]

Can you please supply example shaders of concern?

[charlie-ht]

This is still causing Mesa to throw warnings about invalid SPIR-V. Any chance of getting a resolution to it?

Specifically, there are over 16k instances of this warning coming exclusively from dEQP-VK.tessellation.* tests, which https://gerrit.khronos.org/c/vk-gl-cts/+/7557 fixes a subset of.

Can you please supply example shaders of concern?

One example from dEQP-VK.tessellation.invariance.primitive_set.triangles_equal_spacing_cw,

GLSL,

#version 310 es
#extension GL_EXT_geometry_shader : require

layout(triangles) in;
layout(triangle_strip, max_vertices = 3) out;

layout(location = 0) in VertexData {
    vec4 in_gs_tessCoord;
    int  in_gs_primitiveID;
} ib_in[];

struct PerPrimitive {
    int  patchPrimitiveID;
    int  primitiveID;
    vec4 tessCoord[3];
};

layout(set = 0, binding = 0, std430) coherent restrict buffer Output {
    int          numPrimitives;
    PerPrimitive primitive[];
} sb_out;

void main (void)
{
    int index = atomicAdd(sb_out.numPrimitives, 1);
    sb_out.primitive[index].patchPrimitiveID = ib_in[0].in_gs_primitiveID;
    sb_out.primitive[index].primitiveID      = index;
    sb_out.primitive[index].tessCoord[0]     = ib_in[0].in_gs_tessCoord;
    sb_out.primitive[index].tessCoord[1]     = ib_in[1].in_gs_tessCoord;
    sb_out.primitive[index].tessCoord[2]     = ib_in[2].in_gs_tessCoord;

    gl_Position = vec4(0.0);
    EmitVertex();

    gl_Position = vec4(0.0);
    EmitVertex();

    gl_Position = vec4(0.0);
    EmitVertex();
}

SPIR-V,

; SPIR-V
; Version: 1.0
; Generator: Khronos Glslang Reference Front End; 10
; Bound: 62
; Schema: 0
OpCapability Geometry
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Geometry %4 "main" %30 %55
OpExecutionMode %4 Triangles
OpExecutionMode %4 Invocations 1
OpExecutionMode %4 OutputTriangleStrip
OpExecutionMode %4 OutputVertices 3
OpDecorate %13 ArrayStride 16
OpMemberDecorate %14 0 Offset 0
OpMemberDecorate %14 1 Offset 4
OpMemberDecorate %14 2 Offset 16
OpDecorate %15 ArrayStride 64
OpMemberDecorate %16 0 Coherent
OpMemberDecorate %16 0 Restrict    <<<<<<<
OpMemberDecorate %16 0 Offset 0
OpMemberDecorate %16 1 Coherent
OpMemberDecorate %16 1 Restrict    <<<<<<<
OpMemberDecorate %16 1 Offset 16
OpDecorate %16 BufferBlock
OpDecorate %18 DescriptorSet 0
OpDecorate %18 Binding 0
OpDecorate %27 Block
OpDecorate %30 Location 0
OpMemberDecorate %53 0 BuiltIn Position
OpMemberDecorate %53 1 BuiltIn PointSize
OpDecorate %53 Block
%2 = OpTypeVoid
%3 = OpTypeFunction %2
%6 = OpTypeInt 32 1
%7 = OpTypePointer Function %6
%9 = OpTypeFloat 32
%10 = OpTypeVector %9 4
%11 = OpTypeInt 32 0
%12 = OpConstant %11 3
%13 = OpTypeArray %10 %12
%14 = OpTypeStruct %6 %6 %13
%15 = OpTypeRuntimeArray %14
%16 = OpTypeStruct %6 %15
%17 = OpTypePointer Uniform %16
%18 = OpVariable %17 Uniform
%19 = OpConstant %6 0
%20 = OpTypePointer Uniform %6
%22 = OpConstant %6 1
%23 = OpConstant %11 1
%24 = OpConstant %11 0
%27 = OpTypeStruct %10 %6
%28 = OpTypeArray %27 %12
%29 = OpTypePointer Input %28
%30 = OpVariable %29 Input
%31 = OpTypePointer Input %6
%39 = OpConstant %6 2
%40 = OpTypePointer Input %10
%43 = OpTypePointer Uniform %10
%53 = OpTypeStruct %10 %9
%54 = OpTypePointer Output %53
%55 = OpVariable %54 Output
%56 = OpConstant %9 0
%57 = OpConstantComposite %10 %56 %56 %56 %56
%58 = OpTypePointer Output %10
%4 = OpFunction %2 None %3
%5 = OpLabel
%8 = OpVariable %7 Function
%21 = OpAccessChain %20 %18 %19
%25 = OpAtomicIAdd %6 %21 %23 %24 %22
OpStore %8 %25
%26 = OpLoad %6 %8
%32 = OpAccessChain %31 %30 %19 %22
%33 = OpLoad %6 %32
%34 = OpAccessChain %20 %18 %22 %26 %19
OpStore %34 %33
%35 = OpLoad %6 %8
%36 = OpLoad %6 %8
%37 = OpAccessChain %20 %18 %22 %35 %22
OpStore %37 %36
%38 = OpLoad %6 %8
%41 = OpAccessChain %40 %30 %19 %19
%42 = OpLoad %10 %41
%44 = OpAccessChain %43 %18 %22 %38 %39 %19
OpStore %44 %42
%45 = OpLoad %6 %8
%46 = OpAccessChain %40 %30 %22 %19
%47 = OpLoad %10 %46
%48 = OpAccessChain %43 %18 %22 %45 %39 %22
OpStore %48 %47
%49 = OpLoad %6 %8
%50 = OpAccessChain %40 %30 %39 %19
%51 = OpLoad %10 %50
%52 = OpAccessChain %43 %18 %22 %49 %39 %39
OpStore %52 %51
%59 = OpAccessChain %58 %55 %19
OpStore %59 %57
OpEmitVertex
%60 = OpAccessChain %58 %55 %19
OpStore %60 %57
OpEmitVertex
%61 = OpAccessChain %58 %55 %19
OpStore %61 %57
OpEmitVertex
OpReturn
OpFunctionEnd

In addition to Restrict qualifiers (I'll create another report if this is noise here), there is a relatively smaller number (66) of warnings related to Invariant as well, for example,

SPIR-V WARNING:
    In file ../src/compiler/spirv/spirv_to_nir.c:1106
    Decoration not allowed on struct members: SpvDecorationInvariant
    504 bytes into the SPIR-V binary

GLSL,

#version 450
layout(location = 0) in highp vec4 a_input;
layout(location = 0) out mediump vec4 v_unrelated;
invariant gl_Position;
void main ()
{
        v_unrelated = a_input.xzxz + (1.0e20*a_input.x*a_input.xxxx + 1.0e20*a_input.y*a_input.yyyy) * (1.08 * a_input.zyzy * a_input.xzxz) * 1.0e-20 * (a_input.z * a_input.
zzxz - a_input.z * a_input.zzxz) + (1.0e20*a_input.x*a_input.xxxx + 1.0e20*a_input.y*a_input.yyyy) / 1.0e20;
        gl_Position = a_input + (1.0e20*a_input.x*a_input.xxxx + 1.0e20*a_input.y*a_input.yyyy) * 1.0e-20;
        
}

SPIR-V

; SPIR-V
; Version: 1.0
; Generator: Khronos Glslang Reference Front End; 10
; Bound: 95
; Schema: 0
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Vertex %4 "main" %9 %11 %75
OpDecorate %9 RelaxedPrecision
OpDecorate %9 Location 0
OpDecorate %11 Location 0
OpMemberDecorate %73 0 Invariant     <<<<<<<
OpMemberDecorate %73 0 BuiltIn Position
OpMemberDecorate %73 1 BuiltIn PointSize
OpMemberDecorate %73 2 BuiltIn ClipDistance
OpMemberDecorate %73 3 BuiltIn CullDistance
OpDecorate %73 Block
%2 = OpTypeVoid
...

I'm less sure whether this is related or not...

[greg-lunarg]

I am happy to address the problem with these shaders, although I think the initial problem will remain. Neither of these shaders require that the qualifier be attached to a member of a block in the SPIR-V, so they can be completely addressed without engaging in the title issue: that GLSL and glslang want to attach qualifiers to members of a block, but SPIR-V does not support it. I think the answer is either to allow this in SPIR-V or disallow it in GLSL. The fact that GLSL and SPIR-V disagree here is a cognitive dissonance which needs to be resolved.

The fact that GLSL allows this says to me that someone thought this was useful at one time. So my proposal is to create an issue to the SPIR-V WG to ask them to either confirm their current position or make changes to allow this.

I know JohnK is doubtful about the utility in GLSL and JasonE seem unsupportive of adding this to SPIR-V, but I would like to here it from the entire SPIR-V WG before I make changes to glslang.

@gnl21 Thoughts?

[greg-lunarg]

To be clear, I would not propose that Restrict (or Invariant) decorations be allowed on members of any struct, only to members of structs decorated with Block.

[gfxstrand]

I'm not sure it makes sense to allow it per-member. As @johnkslang said above, restrict is really a property of the binding. In particular, it says that you're not going to bind that same API buffer anywhere else in the shader. With enough gymnastics, you can construct a case where this matters:

layout(set = 0, binding = 0, std430) coherent buffer {
    restrict vec4 a;
    vec4 b;
} A;
layout(set = 0, binding = 1, std430) coherent buffer {
    vec4 b;
    restrict vec4 c;
} B;

and then, on the API side bind them so that they overlap just slightly such that the bs match up. That said, this is pretty useless and, if GLSLang just dropped the restrict decorations in this case, it'd be perfectly valid. A valid way to compile a shader which puts restrict on certain block members would be to put restrict on the whole variable if all struct members are restrict. Dropping a restrict decoration will never make the shader invalid.

[greg-lunarg]

Thanks @jekstrand. I will take yours and John's opinion that this is about the binding and not about the members.

I haven't looked into this yet, but I am presuming that the bookkeeping is being done at the member level, so my plan is to back the restrict qualifier out to the variable only if all members are so qualified. Otherwise the restrict qualifier will be dropped.

Any thoughts about Invariant? It seems dropping that could have adverse consequences. Is it really only used for gl_Position (in which case this is not an issue since gl_Position is a standalone builtin), or do we need to be able to apply this to block members?

[gfxstrand]

Invariant is allowed on members and we should probably leave it alone.