Constant not supported in GLSL shader binding style declaration

[theblackunknown]

Hi team,

I am trying to compile some of our GLSL shaders using slangc and I am running into issues because we are using constant in shader bindings declaration.
Currently we are using google/shaderc to compile our shader and we do not run into this compiler issue.

See below for a minimal repro case.
If you can point me to how to port it as a Slang test I would gladly contribute it if that helps.

Given the following constant-in-layout.slang file

#version 450 core

const uint DescriptorSet = 0;

const uint BindingPoint_Sampler = 0;
const uint BindingPoint_Scale = 1;
const uint BindingPoint_Bias = 2;

layout(location = 0) out mediump vec4 o_color;
layout(location = 0) in highp vec2 v_texCoord;
layout(location = 1) in highp float v_lodBias;
layout(set = DescriptorSet, binding = BindingPoint_Sampler) uniform highp usampler2D u_sampler;
layout(set = DescriptorSet, binding = BindingPoint_Scale) uniform buf0
{
    highp vec4 u_scale;
};
layout(set = DescriptorSet, binding = BindingPoint_Bias) uniform buf1
{
    highp vec4 u_bias;
};

void main()
{
    o_color = vec4(texelFetch(u_sampler, ivec2(v_texCoord), int(v_lodBias))) * u_scale + u_bias;
}

compiling the above file using the following command line
slangc.exe -target spirv -stage fragment -entry main constant-in-layout.slang

Will produce the following errors

tests\glsl\constant-in-layout.slang(15): error 20001: unexpected identifier, expected integer literal
layout(set = DescriptorSet, binding = BindingPoint_Sampler) uniform highp usampler2D u_sampler;
             ^~~~~~~~~~~~~
tests\glsl\constant-in-layout.slang(15): error 20001: unexpected identifier, expected integer literal
layout(set = DescriptorSet, binding = BindingPoint_Sampler) uniform highp usampler2D u_sampler;
                                      ^~~~~~~~~~~~~~~~~~~~
tests\glsl\constant-in-layout.slang(16): error 20001: unexpected identifier, expected integer literal
layout(set = DescriptorSet, binding = BindingPoint_Scale) uniform buf0
             ^~~~~~~~~~~~~
tests\glsl\constant-in-layout.slang(16): error 20001: unexpected identifier, expected integer literal
layout(set = DescriptorSet, binding = BindingPoint_Scale) uniform buf0
                                      ^~~~~~~~~~~~~~~~~~
tests\glsl\constant-in-layout.slang(20): error 20001: unexpected identifier, expected integer literal
layout(set = DescriptorSet, binding = BindingPoint_Bias) uniform buf1
             ^~~~~~~~~~~~~
tests\glsl\constant-in-layout.slang(20): error 20001: unexpected identifier, expected integer literal
layout(set = DescriptorSet, binding = BindingPoint_Bias) uniform buf1
                                      ^~~~~~~~~~~~~~~~~

[jkwak-work]

This appears to be a missing feature support for GLSL.
Slang doesn't support this way of using const variables at the moment.

[theblackunknown]

Thanks @jkwak-work, would you be able to point me at few places if I want to contribute to add this feature ?

[jkwak-work]

I think this can be a bit involving.
Currently the binding and set modifiers are processed in the parser.
But it will have to be deferred to a later part such as AST travers.

The following code needs to be changed from:

static NodeBase* parseLayoutModifier(Parser* parser, void* /*userData*/)
{
...omit...
            Token valToken = parser->ReadToken(TokenType::IntegerLiteral);
            // If wasn't the desired IntegerLiteral return that couldn't parse
            if (valToken.type == TokenType::IntegerLiteral)
            {
                // Work out the value
                auto value = getIntegerLiteralValue(valToken);

                if (nameText == "binding")
                {
                    attr->binding = int32_t(value);
                }
                else
                {
                    attr->set = int32_t(value);
                }
            }

to:

            Token valToken = parser->ReadToken();
            if (nameText == "binding")
                attr->binding = valToken;
            else
                attr->set = valToken;

And the logic that tries to read the value should be deferred until we can get the value assigned to variables.

[csyonghe]

In HLSL, const int x; in global scope means declaring a uniform parameter, i.e. equivalent to uniform int x;

the right syntax to define global compile time constant is to write:

static const int x=0;

[csyonghe]

We should modify attr->binding and attr->set to be a Expr* and modify the parsing logic to
attr->binding = parseExpr().

Then during CheckAttribute, we need to handle the case for this attribute and constant fold binding into an int.

[fairywreath]

I really want this to be supported as well and I am working on a patch. I need clarification regarding non-uniform-block uniform variables. Vulkan SPIR-V does not allow these, but slang compiles them with a warning. I noticed that global non-block uniforms are grouped into a single block, and any layout binding qualifiers are ignored. Can you confirm if this is the intended behavior?

Additionally, I would like this behavior to support other layout attributes, such as the input attachment index. Are there any objections to this? Thanks

[csyonghe]

Sure, no objections to add constant folding for any glsl flavored layout modifiers. Just need to change the type of GLSLBindingAttribute::binding and GLSLBindingAttribute::set from int32_t to IntVal* and deal with the consequences.

The checking logic should follow the similar path as NumThreadsAttribute and other attributes that holds an IntVal* instead of a plain int32_t.