Fix mesh shader per primitive spirv annotation

[jsimmons]

Currently the spirv PerPrimitiveEXT decoration is only being applied to mesh shader outputs. SPIRV requires this decoration to also be applied to any fragment shader inputs which consume the per-primitive output from the mesh shader.

This PR is a quick approach to resolving that, by adding a new perprimitive modifier to slang. I'm unsure whether this is a reasonable approach, maybe it should be a spirv-specific annotation like [vk::perprimitive] instead, or the linker could check whether the associated output is already marked as per-primitive and update the pixel shader input annotation.

It's also a bit weird whether it's really a 'interpolation mode' in the first place, I guess it's more about storage + disabling interpolation. So that might skew towards this being the wrong approach.

The HLSL functional spec notes that this should specific case should be be handled automatically by drivers, so the same issue shouldn't be present there. I have not investigated the situation in other backends.

This PR fixes issues with per-primitive output on the RADV driver. I'm unsure whether other drivers have different behavior in the presence of missing PerPrimitive decorations.

edit: Actually it looks like in hlsl it should be [[vk::perprimitive]] on the pixel shader input. So copying that is probably the way to go rather than adding a new thing.

[CLAassistant]

All committers have signed the CLA.

[csyonghe]

The perprimitive modifier is fine. We should make sure to include more tests.

[csyonghe]

Do we have test to make sure this works on:

1. Directly on entry point parameters.
2. Directly on global parameters.
3. Indirectly on nested struct fields.
4. Array types parameters.

[jsimmons]

Now I added an array type and it seems to also get the location wrong (or I misunderstand how it's supposed to work) on the fragment shader input. :')

[jsimmons]

The perprimitive modifier is fine. We should make sure to include more tests.

Yeah tests sound good. Along the way I also noticed that PerPrimitiveEXT is also not applied to mesh shader outputs decorated with SV_CullPrimitive. Which violates VUID-CullPrimitiveEXT-CullPrimitiveEXT-07038 So I guess a pass over all the tests here is in order.

[jsimmons]

The tests are expanded now, however I'm not sure I understand how needFlatDecorationForBuiltinVar works. As far as I can tell if I put bool cull : SV_CullPrimitive; inside the primitives output for the mesh shader, then it drops the PerPrimitive annotation along the way, do I need to expand the concept of flat in this code? I hacked around this by just handling that case, but it seems generally wrong and I'm not sure how it should work.

Secondly, in expanding the tests it seems that either I've missed something, or the location annotations are not being respected properly in my test code. in_normal is being given location 2, even though it's annotated with 1.

Finally, out_values is not being annotated with PerPrimitive, seemingly because it's an array. But maybe I've just misunderstood how this is supposed to work with the decomposition of a struct into global parameters during legalization?

[csyonghe]

looks like we need to duplicate isIntegralScalarOrCompositeType and have a utility function named isIntegralOrLogicalScalarOrCompositeType that returns true for bool scalars and vectors. And then we make needFlatDecorationForBuiltinVar call isIntegralOrLogicalScalarOrCompositeType instead, so that isFlat will be true for the CullPrimitive builtin. That way we can get rid of this special case logic.

[jsimmons]

Hmm, sorry for the back and forth, I've updated the PR again (including the test), but I'm not sure I've understood you correctly here.

I run into a problem with the difference between the same builtin being used as an input or as an output. The cull flag is only used as a mesh output, so that case is straightforward. But if this is applied to other builtins, it gets weird. For example from my (updated) test case, when applied to outputs...

OpDecorate %82 BuiltIn PrimitiveTriangleIndicesEXT
OpDecorate %82 PerPrimitiveEXT

Which isn't right, and then on the input side there's the opposite problem...

in perprimitive uint in_primitive_id : SV_PrimitiveID;

OpDecorate %gl_PrimitiveID_0 BuiltIn PrimitiveId
OpDecorate %gl_PrimitiveID_0 Flat

Because I'm heuristically choosing between flat and perprimitive as well. I wonder if the best course of action here is to also rely on the manual markup for builtins, and to bypass both this automatic flat annotation, and attempts to infer the perprimitive annotation?

Assuming I didn't misunderstand your suggestion.