KhronosGroup / OpenCL-Docs

OpenCL API, OpenCL C, Extensions, SPIR-V Environment Specs, Ref page, and C++ for OpenCL doc sources.
Other
357 stars 113 forks source link

Allocating local mem in device functions is not explictly forbidden by the SPIR-V OpenCL Env #1221

Open pjaaskel opened 1 month ago

pjaaskel commented 1 month ago

Currently it's possible to create a SPIR-V that allocates local mem in non-kernel functions although this is explicitly forbidden in OpenCL C spec for obvious implementation challenges. It is even accepted by various drivers (which likely just inline everything anyhow, effectively making the allocation happen inside the kernel).

Shall we add a check to the SPIR-V OpenCL Env for this case?

bashbaug commented 1 month ago

The easiest thing to do here would be to add text similar to that in the OpenCL C spec, something like: https://registry.khronos.org/OpenCL/specs/3.0-unified/html/OpenCL_C.html#_usage_for_declaration_scopes_and_variable_types

Variables declared in the outermost compound statement inside the body of the kernel function can be qualified by the local or constant address spaces.

However, there are two real-world problems with this proposed resolution:

  1. We added a pass in the SPIR-V LLVM Translator a while back to handle the SPIR-V restriction that a SPIR-V function cannot call a SPIR-V kernel ("entry point"): https://github.com/KhronosGroup/SPIRV-LLVM-Translator/pull/1149. The "entry point wrapper" that gets inserted will therefore call a function with the contents of the kernel and hence the local memory declaration, meaning the local memory declaration will no longer be "inside the body of the kernel function".

  2. Additionally, Clang currently takes a local memory declaration within a kernel (or elsewhere!) and turns it into global scope declaration, which is faithfully translated to a global scope declaration by the SPIR-V LLVM Translator: https://godbolt.org/z/eE4Yd8sr4. This means with typical SPIR-V toolchains I believe a SPIR-V consumer will only see global scope local memory allocations and will never see a local memory allocation within a function.

Should we document the global scope requirement in the OpenCL SPIR-V environment spec, since this is what the usual SPIR-V generation toolchains is generating?

pjaaskel commented 1 month ago

Should we document the global scope requirement in the OpenCL SPIR-V environment spec, since this is what the usual SPIR-V generation toolchains is generating?

I think yes, and perhaps explicitly allow defining locals in device functions even for OpenCL C since it practically likely ("accidentally") works for every driver anyhow.

bashbaug commented 1 month ago

Actually, maybe we don't need to do anything. The SPIR-V spec already says: https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_logical_layout_of_a_module

All OpVariable instructions in a function must have a Storage Class of Function.

This means that the transformation Clang is doing is the only valid way to use local memory in SPIR-V.

pjaaskel commented 1 month ago

OK, so the main question that remains is that should we lift the OpenCL C restriction as well assuming all/most are using the Clang's implementation for locals.

bashbaug commented 3 weeks ago

Discussed in the August 20th teleconference. There is some interest in lifting the OpenCL C restriction (probably as an extension?), which should be a pretty light lift for most implementors that are using Clang. I've moved this back to "To do" until somebody is able to pick this up, since there is (currently) nothing more to discuss in the working group.