Handling of Builtin Variables violates OpenCL 3.0 SPIR-V Environment Specification.
Describe the bug
The OpenCL 3.0 SPIR-V Environment specifies under 2.9. Built-in Variables that all builtin variables have to be in the Input Storage Class:
"An OpVariable in a SPIR-V module with the BuiltIn decoration represents a built-in variable. All built-in variables must be in the Input storage class."
But the generated sycl SPIR-V puts them into the Cross Workgroup Storage class.
This is a new requirement introduced by OpenCL 3.0, though earlier versions did not specify details of BuiltIn variables, using Cross Workgroup doesn't make any sense from a technical perspective either way.
To Reproduce This can be seen with any SyCL program using builtins.
- fetch the sycl sample app from https://intel.github.io/llvm-docs/GetStartedGuide.html#run-simple-dpc-application
- clang++ -fsycl -fsycl-device-only -fno-sycl-use-bitcode -fsycl-targets=spir64-unknown-unknown test.cpp -o - | spirv-dis
relevant bits of the SPIR-V:
OpDecorate %__spirv_BuiltInGlobalInvocationId LinkageAttributes "__spirv_BuiltInGlobalInvocationId" Import
OpDecorate %__spirv_BuiltInGlobalInvocationId BuiltIn GlobalInvocationId
%_ptr_CrossWorkgroup_uint = OpTypePointer CrossWorkgroup %uint
%__spirv_BuiltInGlobalInvocationId = OpVariable %_ptr_CrossWorkgroup_v3ulong CrossWorkgroup
Environment:
- OS: Linux
- DPC++ version:
clang version 16.0.0 (https://github.com/intel/llvm 22e3fc56c8c2bef6d92a19819b7bea106a7992d1)
Additional context
SyCL binaries shouldn't rely on spec violations to be accepted by OpenCL runtimes even though if requirements were introduced later on.
Also, this doesn't happen if upstream clang is used to compile normal OpenCL C applications using get_global_id to SPIR-V through https://github.com/KhronosGroup/SPIRV-LLVM-Translator
This is still an issue and sadly it's kinda critical because almost every SyCL binary is hitting this. Any chance that anybody could look at it? Otherwise I might try to dig into it myself and figure out what's going wrong.
Hi @karolherbst
We will take a look at this and provide you an update soon. Thanks for your patience.
Sincerely
I am able to reproduce the error with the latest intel/llvm compiler.
We have the following code here:
%_ptr_CrossWorkgroup_v3ulong = OpTypePointer CrossWorkgroup %v3ulong
...
%__spirv_BuiltInGlobalInvocationId = OpVariable %_ptr_CrossWorkgroup_v3ulong CrossWorkgroup
SPIR-V unified spec has the following wording here: Storage Class is the Storage Class of the memory holding the object. It must not be Generic. It must be the same as the Storage Class operand of the Result Type.
I can thus see where CrossWorkgroup StorageCalss comes from.
Investigating further...
Thanks
Hi @karolherbst
Sorry for break in pursuing this. I am actively looking at this now and provide a resolution. Interestingly, when I compile and run using the latest tools in intel/llvm repo, I am able to see the SPIR-V code snippet as reported in the description. But SPIR-V Tools do not emit any errors for this SPIR-V file. Will provide an update soon.
Thanks
But SPIR-V Tools do not emit any errors for this SPIR-V file.
The report says, that generated SPIR-V is not conformant to https://registry.khronos.org/OpenCL/specs/3.0-unified/html/OpenCL_Env.html . SPIR-V Tools only checks conformance to https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.pdf
Not sure about that. SPIR-V Tools do seem to cover OpenCL SPIR-V environment spec as well. Here is a list of issues being tracked there. https://github.com/KhronosGroup/SPIRV-Tools/milestone/6 This issue has also been reported there: https://github.com/KhronosGroup/SPIRV-Tools/issues/3355
A fix has been added here: https://github.com/KhronosGroup/SPIRV-LLVM-Translator/pull/2456 It will be pulled into intel/llvm in a timely manner.
Thanks
Based on reviews and other feedback, I have concluded that https://github.com/KhronosGroup/SPIRV-LLVM-Translator/pull/2456 is a difficult workaround to maintain and guarantee that it will not break anything else. I will try to come up with a more rounded solution.
Thanks