[PATCH] D71005: [AST] Enable expression of OpenCL language address spaces an attribute

Thu Dec 5 06:58:10 PST 2019

bader added a comment.

>>> is there a reason this should be clang::opencl_private as opposed to opencl::private?
>> 
>> I'm okay with [[opencl::private]] as well. I have only one problem - currently OpenCL address spaces are exposed as keywords and using them in C++ breaks valid C++ code.
> 
> I'm not certain who controls the OpenCL spec, but this seems like it should be a decision that comes from there. Or is this functionality outside of the OpenCL spec?

I guess I owe you some background here. OpenCL spec is controlled by Khronos organization, but Khronos provides only C-based language specification "OpenCL C" and I think this functionality is outside of the OpenCL spec.

Today Khronos organization rely on SPIR-V standard to enable high-level languages. This standard defines binary form of low-level intermediate language, which can be targeted by high-level language compiler like C++/Python/Java/Haskel/etc. High level level languages usually have separate working groups or standardization committees defining language features.

In addition to that SYCL working group within Khronos organization defines a C++-based abstraction interface <https://www.khronos.org/sycl/> to program accelerators like GPU, FPGA, DPS. SYCL interfaces do not expose any new C++ keywords or attributes and can be implemented as library using standard C++, but this implementation won't be able to offload execution of C++ code to an accelerator as standard C++ do not provide required facilities. To enable execution on accelerators, we implemented "SYCL device compiler" mode in clang and enhanced SYCL library with a few non-standard C++ extensions (e.g. attributes). Our implementation targets SPIR-V format for accelerated code and OpenCL API to communicate with accelerators. AFAIK, Codeplay team uses CUDA to enable execution of the SYCL code on NVIDIA GPUs <https://github.com/intel/llvm/issues/879#issuecomment-560453423>.

In our implementation we are trying to re-use existing clang infrastructure developed for other GPU programming models OpenCL/OpenMP offload/CUDA/HIP, but due to design differences it can't always be re-used as is. For instance, if I understand it correctly, we can't use OpenCL address space attributes exposed as keywords as they might break valid C++ code - https://godbolt.org/z/fF5Ng5.
That's the basic motivation for this change.

> In attributes, when an identifier can be interpreted as a keyword it is required to be interpreted as an identifier. We have the same issue with [[gnu::const]]. See http://eel.is/c++draft/dcl.attr.grammar#4.sentence-5

If I understand it correctly, it's the reason why `const` attribute has "GCC" spelling, instead of "Keyword" here https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Basic/Attr.td#L940. Right?

> will other implementations of OpenCL want to have the same functionality? Or is this something we expect other OpenCL implementations to largely ignore?

Exposing these attributes as non-keywords is useful for SYCL implementations. Actually this patch was developed by Codeplay team for ComputeCPP compiler - another SYCL implementation and "donated" by @Naghasan to our open source implementation to avoid implementation divergence.

To summarize: SYCL implementation need some instruments to express OpenCL address spaces in C++ code w/o breaking existing C++ code. We find that attributes fit well for this use case, but I'm open for alternative ideas.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71005