Handling of Builtin Variables violates OpenCL 3.0 SPIR-V Environment Specification.

[karolherbst]

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.

1. fetch the sycl sample app from https://intel.github.io/llvm-docs/GetStartedGuide.html#run-simple-dpc-application
2. 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

[karolherbst]

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.

[asudarsa]

Hi @karolherbst

We will take a look at this and provide you an update soon. Thanks for your patience.

Sincerely

[asudarsa]

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

[asudarsa]

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

[MrSidims]

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

[asudarsa]

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: KhronosGroup/SPIRV-Tools#3355

[asudarsa]

A fix has been added here: KhronosGroup/SPIRV-LLVM-Translator#2456
It will be pulled into intel/llvm in a timely manner.

Thanks

[asudarsa]

Based on reviews and other feedback, I have concluded that KhronosGroup/SPIRV-LLVM-Translator#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