[PATCH] D60455: [SYCL] Add support for SYCL device attributes

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

In D60455#1467018 <https://reviews.llvm.org/D60455#1467018>, @Anastasia wrote:

> Just to understand how this will work. I would imagine you can have a device function definition preceding its use. When the function is being parsed it's not known yet whether it will be called from the device or not, so it won't be possible to set the language mode correctly and hence provide the right diagnostics. So is the plan to launch a separate parsing phase then just to extract the call graph and annotate the device functions?


I'm not an expert in clang terminology but I will try briefly explain our current implementation approach. 
In SYCL all kernel functions should be template functions so these functions have a deferred instantiation. If we found that we instantiated a sycl kernel function - we add it to a special array with sycl device functions (you can see the corresponding code here <https://github.com/intel/llvm/blob/0df0754f9f0a519a0938fc60de15614b3e0059fd/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp#L5448> and here <https://github.com/intel/llvm/blob/0df0754f9f0a519a0938fc60de15614b3e0059fd/clang/lib/Sema/SemaSYCL.cpp#L785>, actually `AddSyclKernel` adds declaration to the special array inside the Sema).  After performing
pending instantiations we run a recursive AST visitor for each SYCL kernel to mark all device functions and add them to a special array with SYCL device functions (here <https://github.com/intel/llvm/blob/0df0754f9f0a519a0938fc60de15614b3e0059fd/clang/lib/Sema/Sema.cpp#L925> we start traverse AST from `MarkDevice` function, code of `MarkDevice` is here <https://github.com/intel/llvm/blob/0df0754f9f0a519a0938fc60de15614b3e0059fd/clang/lib/Sema/SemaSYCL.cpp#L788>).
To get a correct set of SYCL device functions in produced module we added a check for all declarations inside the CodeGen on  `sycl_device` attribute existence - so it will ignore declarations without `sycl_device` attribute if we are compiling for SYCL device (code is here <https://github.com/intel/llvm/blob/0df0754f9f0a519a0938fc60de15614b3e0059fd/clang/lib/CodeGen/CodeGenModule.cpp#L2188>). But with this check it's possible situation when function was parsed and ignored by the CodeGen before we added `sycl_device' attribute to it so we added yet another parsing action inside the clang::ParseAST to generate code for all SYCL device functions from the special array (code is here <https://github.com/intel/llvm/blob/0df0754f9f0a519a0938fc60de15614b3e0059fd/clang/lib/Parse/ParseAST.cpp#L169>).

> Ok, my question is whether you are planning to duplicate the same logic as for OpenCL kernel which doesn't really seem like an ideal design choice. Is this the only difference then we can simply add an extra check for SYCL compilation mode in this template handling case. The overall interaction between OpenCL and SYCL implementation is still a very big unknown to me so it's not very easy to judge about the implementations details...

Of course, if nothing prevents us to re-use OpenCL kernel attribute for SYCL I assume it would be good idea. 
But I'm thinking about the situation with https://reviews.llvm.org/D60454 . If we re-use OpenCL kernel attributes - we affected by OpenCL-related changes and OpenCL-related changes shouldn't violate SYCL semantics. Will it be usable for SYCL/OpenCL clang developers? @bader , what do you think about it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455