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 cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits