[PATCH] D89372: [OpenCL] Remove unused extensions

Wed Oct 14 12:32:27 PDT 2020

Anastasia added a comment.

In D89372#2330822 <https://reviews.llvm.org/D89372#2330822>, @yaxunl wrote:

> In D89372#2330362 <https://reviews.llvm.org/D89372#2330362>, @Anastasia wrote:
>
>> In D89372#2330217 <https://reviews.llvm.org/D89372#2330217>, @yaxunl wrote:
>>
>>> With this change, clang basically will have no knowledge about the removed extensions, i.e., it will not know which extension is supported in which version of OpenCL and have no way to enable/disable those extensions. There will be no way to define corresponding macros in clang.
>>>
>>> Basically the responsibility of defining those macros will be shifted to OpenCL runtime for JIT and shifted to users for offline compilation. They need to have knowledge about which extensions are supported in which version of OpenCL and which cpu/platform supports them. I am not sure whether this is the direction we want to move to.
>>
>> But why do you think anyone would need to use those macros in OpenCL C?
>>
>> Let's take `cl_khr_icd` as an exmaple: https://www.khronos.org/registry/OpenCL//sdk/2.2/docs/man/html/cl_khr_icd.html
>>
>> `cl_khr_icd - Extension through which the Khronos OpenCL installable client driver loader (ICD Loader) may expose multiple separate vendor installable client drivers (Vendor ICDs) for OpenCL.`
>>
>> Why would anyone need any macro while compiling OpenCL C code for this functionality? It is dialing with the driver loader that runs on the host before compiling anything.
>>
>> I believe that the addition of such extensions into the kernel language is accidental and we should try to improve this. There is no need to have something that isn't needed. We have enough code and complexity to maintain that is useful. Let's try to simplify by at least removing what is not needed.
>>
>> On a separate note, the extensions that need macro definition and don't require the functionality in the clang parsing also doesn't have to be in the clang source code. I have mentioned it in my RFC as well: http://lists.llvm.org/pipermail/cfe-dev/2020-September/066911.html
>
> Does the spec requires cl_* macro to be defined if an extension is enabled?

The extension spec currently has:

  Every extension which affects the OpenCL language semantics, syntax or adds built-in functions to the language must create a preprocessor #define that matches the extension name string. This #define would be available in the language if and only if the extension is supported on a given implementation.

Those extensions don't affect the language or add any BIFs so my reading from this the macro shouldn't be available.

> If it is not useful, shouldn't the spec be fixed first? Otherwise, how do we make sure users are not using those macros?

I have first attemted to resolve this with Khonos but there was slow progress so it got stalled several times https://github.com/KhronosGroup/OpenCL-Docs/issues/82 (see also PR). I gather we are the main affected stakeholders so perhaps it is up to us to improve this.

As for the spec I would like to point out that clang doesn't implement spec presicely from very early standards. The issue is that it has never been documented but we plan to improve at least from the documentation side in the course of OpenCL 3.0 development.

http://lists.llvm.org/pipermail/cfe-dev/2020-September/066912.html

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89372/new/

https://reviews.llvm.org/D89372