search

KhronosGroup / SPIRV-Cross

SPIRV-Cross is a practical tool and library for performing reflection on SPIR-V and disassembling SPIR-V back to high level languages.
Apache License 2.0
2.01k stars 556 forks source link

MSL: Add support for op OpCompositeInsert in OpSpecConstantOp? #1736

Open billhollings opened 3 years ago

billhollings commented 3 years ago

Several Vulkan CTS tests use op OpCompositeInsert in OpSpecConstantOp, which is not currently supported in SPIRV-Cross. Consequently, these tests fail in MoltenVK.

I've had a look, but it is not at all clear to me how a modified constant composite can be created in MSL via SPIRV-Cross. The corresponding instruction for the non-constantOpCompositeInsert op relies on emitting statements to crate a temp var copy of the composite, and then modifying part of it, which won't be possible with a constant value.

For a constant expression, it would seem we will have to construct the new composite on the fly, with the appropriate element modified as the composite is being created, but unfortunately, the internal plumbing for that is beyond my understanding of SPIRV-Cross innards.

[edit]: Adding CTS tests affected:

dEQP-VK.spirv_assembly.instruction.compute.opspecconstantop.vector_related
dEQP-VK.spirv_assembly.instruction.graphics.opspecconstantop.vector_related_vert
dEQP-VK.spirv_assembly.instruction.graphics.opspecconstantop.vector_related_tessc
dEQP-VK.spirv_assembly.instruction.graphics.opspecconstantop.vector_related_tesse
dEQP-VK.spirv_assembly.instruction.graphics.opspecconstantop.vector_related_frag
HansKristian-Work commented 3 years ago

I suppose it's possible to implement OpCompositeInsert as a Construct, where one of the elements are replaced. It gets very hairy with things like spec constant arrays though, since we don't necessarily know the array size in SPIRV-Cross compile time, but MSL doesn't support array spec constants to begin with ...

billhollings commented 3 years ago

but MSL doesn't support array spec constants to begin with

Yes. WRT array lengths, that specific issue is probably something we can leave for a later revision or enhancement, but for now we need to live with that general restriction about spec constants under MSL.