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

[Ronan Keryell via Phabricator via cfe-commits]

keryell added a comment.

While we are thinking about recycling some attributes across languages, we have to think about the fact that real applications are often made from various languages, such as SYCL + OpenMP 5 or whatever.
It would be nice not to forbid such a combination inside the same TU by design.



================
Comment at: clang/include/clang/Basic/Attr.td:1000
+def SYCLDevice : InheritableAttr {
+  let Spellings = [GNU<"sycl_device">];
+  let Subjects = SubjectList<[Function, Var]>;
----------------
Fznamznon wrote:
> aaron.ballman wrote:
> > Is there a reason to not also introduce a C++11 and C2x style spelling in the `clang` namespace? e.g., `[[clang::sycl_device]]`
> I don't think that it makes sense because these attributes not for public consumption. These attributes is needed to separate code which is supposed to be offloaded from regular host code. I think SYCLDevice attribute actually doesn't need a spelling because it will be added only implicitly by compiler.

If we go towards this direction, `[[clang::sycl::device]]` or `[[clang::sycl::kernel]]` look more compatible with the concept of name space.
While not a public interface, if we have a kind of "standard" outlining in Clang/LLVM, some people might want to use it in some other contexts too.


================
Comment at: clang/include/clang/Basic/Attr.td:1003
+  let LangOpts = [SYCL];
+  let Documentation = [Undocumented];
+}
----------------
Fznamznon wrote:
> aaron.ballman wrote:
> > No new, undocumented attributes, please.
> As I said, these attributes are not for public consumption. Should I add documentation in this case too?
Yes, documentation and comments are always appreciated.


================
Comment at: clang/include/clang/Basic/Attr.td:1010
+  let LangOpts = [SYCL];
+  let Documentation = [Undocumented];
+}
----------------
Cf supra.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455