"SPIR-V only" khr extension handling

[karolherbst]

The issue came original up in https://github.com/KhronosGroup/OpenCL-CTS/pull/1735 adding cl_khr_expect_assume to the CTS.

Just opening the issue here so the discussion won't be lost.

[karolherbst]

Side note: I don't think we should allow SPIR-V only extensions under khr, because they just fragment how compilers actually implement those extensions in their OpenCL C frontend and we might end up with implementations doing it differently. We should rather just define this alongside the SPIR-V side so it's all consistent across various OpenCL C to SPIR-V compilers, which in almost every case will be clang anyway

Can you say a little more about how this might work? I'd love to have a way to streamline SPIR-V-only extensions but I haven't thought of a clever way to do this just yet.

I think we could either just formalize as OpenCL C what clang is currently doing or come up with new APIs which get included in clang.

custom OpenCL C to SPIR-V compilers will use whatever API they think is right, so better just define how it should look like on the OpenCL C side otherwise code targeting multiple of those OpenCL C to SPIR-V compilers might end up with annoying #ifdefs like we see in normal C code dealing with multiple compilers.

Note that some of these SPIR-V-only extensions may not have any representation in OpenCL C (cl_khr_spirv_linkonce_odr is a good example), but they're still useful so kernels written in other source languages may executed via an OpenCL runtime.

yeah, and I think that's fine, though my rule of thumb would be "anything you code in OpenCL C should be defined in extensions as well" and therefore we should limit "SPIR-V only" extensions to things you really can't express on the OpenCL C side.

[bashbaug]

It may be wise to separate any issues specific to cl_khr_expect_assume from a general policy/philosophy. I'd forgotten about the various issues we encountered with cl_khr_expect_assume, but for Khronos folks they're documented in internal MR !173, and for non-Khronos folks a brief summary is:

• There can be a LOT of overloads for this feature depending on the types (especially vector types) we choose to support. Supporting them all requires more code and test coverage.
• It's unclear whether side effects should be evaluated when assume built-in functions are called (they aren't for the Clang assume intrinsic).
• It's unclear how to represent the condition for the assume function (bool or something else?).
• It's unclear which integer types should be supported for the expect function (the Clang intrinsic uses long, but long is invalid in the embedded profile).

In the end, because these functions only provide optimization hints to the compiler and are therefore optional, we decided to reduce scope and complexity and to only formally support the functionality at the SPIR-V level, and informally via Clang intrinsics on some (many? most?) implementations, for now at least.

Philosophically, I think we need to decide when it is acceptable to have a "SPIR-V only" extension and when we really ought to have an OpenCL C representation as well. Unfortunately, I don't whether we can or cannot support a feature in OpenCL C will be a simple decision in all cases - as an example, some language features are much easier to describe when C++ templates are supported. This doesn't mean that an OpenCL C representation is impossible, but it'd be a lot more work. Is this a case where a SPIR-V only extension is acceptable?

This is not a hypothetical question and there is a new feature that should be appearing shortly that has this property, so any guideance would be appreciated - thanks!

[karolherbst]

I think if it's really not feasible to actually formalize this, it might be enough to do something similar to what was done with cl_khr_expect_assume where there is sample code or maybe even requiring implementations to choose a fixed name to implement them. Like "If an implementations however does provide an OpenCL C interface, the builtins shall be called __builtin_expect(gentype, gentype) and __builtin_assume(igentype) and exposed via an extension feature macro"

[karolherbst]

In either case, I think it should be up to implementations to either only expose those as API extensions or also as an OpenCL C extension as long as they more or less follow the suggestion.