Closed js6i closed 2 months ago
This is impossible for me to debug. I don't have any Apple hardware.
Hi, @js6i !
Unless there were major changes, since I've worked on spvDescriptorArray
, it should not be used in context of emulation-layers such as MoltenVK. The goal of this class is to map runtime-sized array from GLSL to Metal3 argument buffer.
Code bellow makes no sense then:
struct spvDescriptorSetBuffer1
{
spvDescriptor<texture_buffer<uint>> t0 [[id(0)]][1] /* unsized array hack */;
};
This is a mix of T1-argument-buffer(spvDescriptorSetBuffer1
) and T2 via spvDescriptor
.
AFAIR what you need to to set following setting of compiler:
msl_options.argument_buffers = true; // emulate descriptor sets via ABuffer
msl_options.argument_buffers_tier = Options::ArgumentBuffersTier::Tier1; // disallow Tier2
@Try thanks for chiming in. Are you saying that msl_options.argument_buffers
are inherently in conflict with the Tier2 setting? MoltenVK needs variable sized arrays supported as well - is there a reason to require disabling msl_options.argument_buffers
, or maybe we should just not emit the spvDescriptor
wrappers in this case?
Besides, the generated code seems fine other than the weird bug that I addressed in #2314.
Are you saying that msl_options.argument_buffers are inherently in conflict with the Tier2 setting?
There were not intended to be uses together. Yet, have to say that I didn't followed spirv-cross for several months, and thing maybe different now. Maybe @billhollings knows better, than I am, if there is a use case for argument_buffers
+ Tier2
in MoltenVK.
In Metal T1-argument buffer is close analog to Vulkan descriptor-set, and T2 - more like descriptor-buffer, allowing pointer to other buffers/textures within the buffer.
Here, IMAO is misleading naming has place. msl_options.argument_buffers
de facto mean "use T1-argument buffer to emulate vulkan-descriptor set". And argument_buffers_tier=Tier2
, mean to emit array of descriptors as T2-argument buffer.
MoltenVK needs variable sized arrays supported as well
Don't think it needed to emulate vulkan. Vulkan descriptor-indexing model always requires to declare upper bound for each runtime-sized array, on C++ side. Even if VK_DESCRIPTOR_BINDING_VARIABLE_DESCRIPTOR_COUNT_BIT
is in use.
Don't think it needed to emulate vulkan. Vulkan descriptor-indexing model always requires to declare upper bound for each runtime-sized array, on C++ side. Even if
VK_DESCRIPTOR_BINDING_VARIABLE_DESCRIPTOR_COUNT_BIT
is in use.
Yeah, the issue with that is that then binding a smaller buffer (and the descriptor pool only needs to be large enough to fit the variable count) triggers a validation error.
Workaround is merged, so should be fine now.
This is something I found while working on https://github.com/KhronosGroup/MoltenVK/pull/2199. Enabling variable sized arrays caused a bunch of vkd3d tests that otherwise worked fine (modulo validation errors due to declaring huge arrays and binding small buffers to them) to crash the GPU (M1). I tried a few things, like using the regular
array<>
type instead ofspvDescriptor*
wrappers, but what ended up fixing the crash was making the entry point variables plain pointers.Below is one of the kernels that crashed (from vkd3d d3d12
test_register_space
), with the plain pointer version #if'd inline:The crash is
vkQueueSubmit MTLCommandBuffer on Queue 3-0" execution failed (code 3): Caused GPU Address Fault Error (0000000b:kIOGPUCommandBufferCallbackErrorPageFault