No compiler error thrown when a non-constant ID is passed to subgroupBroadcast

[gfxstrand]

From the GL_KHR_shader_subgroup spec:

The function subgroupBroadcast() returns the <value> from the invocation
whose <gl_SubgroupInvocationID> is equal to <id>.  <id> must be an integral
constant expression.

However, putting in a non-constant expression works fine in GLSLang. Incidentally, it also works fine in our driver so I could easily see shader authors (apparently myself included) mistaking subgroupBroadcast for subgroupShuffle and a compile error would significantly help.

[johnkslang]

I did not write this area, but the tests themselves don't use a constant, and are based on gl_SubgroupInvocatioID, which is in, not const, so I wonder if really the spec. is wrong instead.

@sheredom, do you know?

[johnkslang]

The fix, if it should be fixed, is shown in https://github.com/KhronosGroup/glslang/tree/subgroupBroadcast-const-id.

[sheredom]

So the way I wrote the spec it should be an integral constant expression.

Does your fix work for specialisation constants too?

[johnkslang]

So the way I wrote the spec it should be an integral constant expression.

Right. But, the tests heavily rely on non-constants and would have to be rewritten, hence questioning what's appropriate, especially if the test pattern is a realistic one.

Does your fix work for specialisation constants too?

That should work as it does generally, meaning yes, where specialization constants are a subclass of constants.

[sheredom]

Honestly I can't remember now why we required it to be an integral constant expression.

For what its worth HLSL only requires their equivalent WaveReadLaneAt to be uniform https://docs.microsoft.com/en-us/windows/desktop/direct3dhlsl/wavereadlaneat

And on our implementation we only actually require it to be a uniform value.

[johnkslang]

The tests are written exclusively in terms of an in, so are not uniform. The id is:

(gl_SubgroupInvocationID + gl_SubgroupSize) % 4

[sheredom]

Right, so that should never have been valid for subgroupBroadcast - that is the differentiation between subgroupBroadcast and subgroupShuffle -> shuffle allows a fully dynamic id.

[johnkslang]

Okay, I will change the tests to use a constant.

[johnkslang]

Can you sign off on #1592?

[allanmac]

Broadcasting the last lane results in an error (using today's ToT glslang):

subgroupBroadcast(value,gl_SubgroupSize - 1)
>>> ERROR: ... : 'id' : argument must be compile-time constant

[gfxstrand]

@allanmac, that's correct. You really want to use subgroupShuffle in that case or use a specialization constant that you fill in from the physical device properties.

Also, be careful doing that as you don't know that the last lane is actually active. Even in compute shaders, there is no guarantee that all lanes are active.

[johnkslang]

gl_SubgroupSize is not a constant. See KhronosGroup/GLSL#19.

[allanmac]

Not being able to broadcast the final lane is unfortunate since a "prefix sum followed by a broadcast" is a very common compute idiom.

It's creating more work for the developer to have to define what is essentially subgroupBroadcastLast().

[johnkslang]

These points should be made in KhronosGroup/GLSL#19.