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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
