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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/Basic/Attr.td:1000
+def SYCLDevice : InheritableAttr {
+  let Spellings = [GNU<"sycl_device">];
+  let Subjects = SubjectList<[Function, Var]>;
----------------
keryell wrote:
> 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.
If these are only being added implicitly by the compiler, then they should not be given any `Spelling`. See `AlignMac68k` for an example.


================
Comment at: clang/include/clang/Basic/Attr.td:1003
+  let LangOpts = [SYCL];
+  let Documentation = [Undocumented];
+}
----------------
keryell wrote:
> 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.
If the attribute is only ever added implicitly and the user cannot spell it in any way, then I think it's reasonable to elide the documentation. It's an implementation detail at that point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455