search

KhronosGroup / MoltenVK

MoltenVK is a Vulkan Portability implementation. It layers a subset of the high-performance, industry-standard Vulkan graphics and compute API over Apple's Metal graphics framework, enabling Vulkan applications to run on macOS, iOS and tvOS.
Apache License 2.0
4.73k stars 411 forks source link

VkFFT fails to run with lots of "error: variable length arrays are not supported in Metal" shader compilation errors.. #1106

Open oscarbg opened 3 years ago

oscarbg commented 3 years ago

Hi, just wanted to test if possible to run fast VkFFT library (https://github.com/DTolm/VkFFT) under MoltenVK.. using latest MoltenVK from Vulkan SDK 1.2.154..

the library compiled fine using cmake 3.17 with a minor fix applied: changing autodetected variable: vulkan_LIBRARY=/Library/Frameworks/vulkan.framework to vulkan_LIBRARY=/Library/Frameworks/vulkan.framework/vulkan then VkFFT executable will be built but will fail to run as VkFFT has added double precision support and forces: deviceFeatures.shaderFloat64 = true; even for running single precision benchmarks (which are default).. commenting this runs, but fails to compile shaders to Metal: multiple times prints this error:

[mvk-error] VK_ERROR_INITIALIZATION_FAILED: Shader library compile failed (Error code 3):
Compilation failed: 

program_source:353:29: error: variable length arrays are not supported in Metal
    threadgroup float2 sdata[_891];
                            ^
.
[mvk-error] VK_ERROR_INVALID_SHADER_NV: Compute shader function could not be compiled into pipeline. See previous logged error.

curious thing is MoltenVK supports portability extension and can't see any optional portability extension feature mentioning VLA shader limitations on Metal vs Vulkan.. so question is: this is fixable by MoltenVK? it's a bug/feature request to report to Apple Metal devs? or as it's seems shader related may open an issue to SPIRV-Cross devs, which may be able to bring some workaround around this..

thanks..

cdavis5e commented 3 years ago

Is %891 a spec constant? EDIT: In the SPIR-V, I mean?

oscarbg commented 3 years ago

@cdavis5e yep seems so.. that's what the dev has said:

Hello, in most shaders shared memory size is defined through specialization constants, which are specified in the half-compiled shaders. I don't have much experience with MoltenVK, but maybe this is not possible there right now. Nothing really prohibits allocation of full 32KB array every time, except that this limits theoretical occupancy (this plays role on some smaller systems like 64x64x64). I attach shaders (single-precision, float folder) which have full shared memory allocated every time, they should solve this, but it is not the best solution. It is possible to specify these constants before the initial compilation, but this will require to perform a full recompilation with glslangvalidator for each fft system size.

he has provided modified shaders that work OK.. but as said earlier.. it's that not fixable by MoltenVK or SPIRV-Cross? if not, seems either a request to Apple to support that case, or an additional property on portability extension should be added..

thanks..

cdavis5e commented 3 years ago

That's what I thought. The problem is that Metal Clang currently doesn't let you specify the size of an array using a constant variable (i.e. a global in the constant address space). @billhollings filed FB8706147 for this, but until then, there's little we can do. There probably ought to be a bit in VkPhysicalDevicePortabilitySubsetFeaturesKHR for this. I filed KhronosGroup/Vulkan-Portability#10 for this.