[SYCL] Implement sycl_ext_oneapi_kernel_compiler_spirv

[0x12CC]

Implements the extension described in #11954. This PR includes the following changes:

• Adds a create_kernel_bundle_from_source overload for std::vector<std::byte> kernel sources.
• Adds new source_language::spirv.
• Defines SYCL_EXT_ONEAPI_KERNEL_COMPILER_SPIRV.
• Adds support for SPIR-V kernels created from std::vector<std::byte> sources.
• Moves sycl_ext_oneapi_kernel_compiler_spirv.asciidoc from proposed to experimental.

[gmlueck]

I think we should have tests for all the kernel argument types defined in this section of the extension spec:

https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/proposed/sycl_ext_oneapi_kernel_compiler_spirv.asciidoc#passing-kernel-arguments

I think the best way to do this is to write the kernel in SPIR-V instead of OpenCL C. (You could start with an OpenCL C kernel and compile it to SPIR-V in order to get the general framework. Then you can edit the SPIR-V file to make sure the details are right.)

The test should include the following kernel argument types:

• OpTypeInt, width 8, 16, 32, and 64.
• OpTypeFloat, width 16, 32 and 64.
• OpTypePointer with storage class CrossWorkgroup, pass argument as USM pointer.
• OpTypePointer with storage class CrossWorkgroup, pass argument as accessor.
• OpTypePointer with storage class Workgroup, pass argument as local_accessor.
• OpTypeStruct with members OpTypeInt, OpTypeFloat, and OpTypePointer (storage class CrossWorkgroup)

Note that OpTypeFloat with widths 16 and 64 are optional features. Therefore, the test will need to check the device aspects aspect::fp16 and aspect::fp64 before using these argument types.

[0x12CC]

I was able to add all but one your suggested tests by compiling from OpenCL C to SPIR-V. I disassembled the generated SPIR-V binary and verified it includes the required types for each OpFunctionParameter.

For the OpTypeStruct test case, I can't use the same approach. The struct parameter in the SPIR-V binary was represented as an OpTypePointer to OpTypeStruct. Also, OpenCL C disallows structs containing pointers.

I think the best way to do this is to write the kernel in SPIR-V instead of OpenCL C. (You could start with an OpenCL C kernel and compile it to SPIR-V in order to get the general framework. Then you can edit the SPIR-V file to make sure the details are right.)

Could you please clarify this suggestion? If I manually modify the generated binary, I would need to add it to this PR. How do we expect a user of this extension to create a SPIR-V binary with the OpTypeStruct parameter you're describing?

[gmlueck]

For the OpTypeStruct test case, I can't use the same approach. The struct parameter in the SPIR-V binary was represented as an OpTypePointer to OpTypeStruct. Also, OpenCL C disallows structs containing pointers.

Yes, good catch. I did some experimentation, and I agree that OpTypeStruct is not a valid kernel argument type. As you say, it should be OpTypePointer (with Storage Class of Function), where the pointer points to an OpTypeStruct. I'll update the extension specification to fix this.

Could you please clarify this suggestion? If I manually modify the generated binary, I would need to add it to this PR.

Yes, this is what I had in mind. It seems better for the test to include the SPIR-V file, rather than assuming the OpenCL C compiler generates a certain SPIR-V. I thought that this PR should include assembly-form SPIR-V modules as part of the test. The test can then use spirv-as to translate the assembly code into binary.

FWIW, it seems that Level Zero only supports SPIR-V 1.2, so you need to run spirv-as with the option --target-env spv1.2.

[gmlueck]

FYI: #12338 updates the spec regarding OpTypeStruct.

[0x12CC]

I've added all of the test cases you suggested.

Yes, this is what I had in mind. It seems better for the test to include the SPIR-V file, rather than assuming the OpenCL C compiler generates a certain SPIR-V

I used the LLVM assembly format for the kernels since spirv-as is not built inside our repository. I hope you think this is acceptable since the llvm-spirv translation is very close to the original assembly.

[gmlueck]

Thanks for adding these tests!

I used the LLVM assembly format for the kernels since spirv-as is not built inside our repository.

Would it be hard to change the build script to clone and build the Khronos SPIRV-Tools repo as an external dependency? I think we clone some other external repos already.

Using LLVM IR files for this test might not be ideal. I wonder if there is a risk of stability with the .ll files. Isn't it possible for the LLVM IR definition to change from one clang release to another? If we cannot add the SPIRV-Tools repo as a dependency, another option would be to add the binary SPIR-V modules to the test directory.

[0x12CC]

Would it be hard to change the build script to clone and build the Khronos SPIRV-Tools repo as an external dependency? I think we clone some other external repos already.

I don't know a simple way to do this. I'm not very familiar with the details of our build script implementation.

Using LLVM IR files for this test might not be ideal. I wonder if there is a risk of stability with the .ll files. Isn't it possible for the LLVM IR definition to change from one clang release to another? If we cannot add the SPIRV-Tools repo as a dependency, another option would be to add the binary SPIR-V modules to the test directory.

I agree with you here. It's possible for the IR definition to change between Clang releases, but I initially thought it would be a good alternative to using the binary modules. I've switched the kernel sources to SPV files.

[gmlueck]

This is OK with me, but I'm not sure if we have a policy against adding binary files to the repo. I'll let the other reviewers comment on that.

If someone thinks we could somehow add the SPIRV-Tools repo as a dependency, I think that would be a cleaner solution.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[cperkinsintel]

@gmlueck I definitely think creating kernel bundles from spirv binary ( .spv ) is the right thing.

But there is a tiny bit of a disconnect there, where we are creating "from source" and yet that source is .spv binary not .spt the textual human(*) readable representation of SPIR-V. Anyway, I'm not trying to make more work for anyone. Nor do I think there is any real demand for using .spt sources. But it does seem a bit funny.

(*) I guess .spt is "almost human" readable.

[gmlueck]

I agree that calling SPIR-V binary "source" is a bit of a misnomer, but all the APIs work out pretty well. We could easily add SPIR-V source support in the future by adding a new enumerator like source_language::spirv_source. However, I think there is no compelling reason to do that unless there is some request.