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
1.96k stars 549 forks source link

MSL: 1-size hack for runtime arrays causes GPU page fault #2337

Closed K0bin closed 1 week ago

K0bin commented 3 weeks ago

The unsized array hack:

struct spvDescriptorSetBuffer3
{
    spvDescriptor<texture2d<float>> albedo_global [[id(0)]][1] /* unsized array hack */;
};

causes a GPU page fault on the M2. Changing nothing except replacing the size 1 with something that is larger than the largest index that gets accessed fixes it.

~~It seems like Apple GPUs write the size into their buffer descriptor which they probably get from the size of the struct. https://gitlab.freedesktop.org/alyssa/mesa/-/blob/honeykrisp-20240501/src/asahi/vulkan/hk_descriptor_set.c?ref_type=heads#L163~~. Irrelevant according to Alyssa.

So it should probably be reworked to either emit an error when it encounters an unsized array or (at least for cases where that array is the only descriptor set binding) emit it directly as an argument for the main function like this const device spvDescriptor<texture2d<float>>* albedo_global [[buffer(3)]].

The easy workaround from an application POV is to just set an array size. Unfortunately the MSL resource binding struct in the C API is missing the size field: https://github.com/KhronosGroup/SPIRV-Cross/blob/main/spirv_cross_c.h#L384

HansKristian-Work commented 1 week ago

FWIW, if you use just a single variable count bindless array in a set, you can make it a discrete set. It should translate to plain tier-2 array then. As for mixed sets, there isn't much I can do about MSL lacking a proper unsized array. The PR adds the count variable to C API as requested.