Fznamznon added inline comments.
================ Comment at: clang/include/clang/Basic/Attr.td:1017 + let LangOpts = [SYCL]; + let Documentation = [Undocumented]; +} ---------------- bader wrote: > Anastasia wrote: > > Ok, I thought the earlier request was not to add undocumented attributes > > with the spelling? > > > > Also did `__kernel` attribute not work at the end? > > > > I can't quite get where the current disconnect comes from but I find it > > extremely unhelpful. > Hi @Anastasia, let me try to help. > > > Ok, I thought the earlier request was not to add undocumented attributes > > with the spelling? > > Right. @Fznamznon, could you document `sycl_kernel` attribute, please? > > > Also did __kernel attribute not work at the end? > > Maria left a comment with the summary of our experiment: > https://reviews.llvm.org/D60455#1472705. There is a link to pull request, > where @keryell and @agozillon expressed preference to have separate SYCL > attributes. Let me copy their feedback here: > > @keryell : > > > Thank you for the experiment. > > That looks like a straight forward change. > > The interesting part is that it does not expose any advantage from reusing > > OpenCL __kernel marker.... So I am not more convinced that it is the way to > > go, because we would use any other keyword or attribute and it would be the > > same... > > > > @agozillon : > > > Just my two cents, I think a separation of concerns and having separate > > attributes will simplify things long-term. > > > > While possibly similar just now, the SYCL specification is evolving and may > > end up targeting more than just OpenCL. So the semantics of the attributes > > may end up being quite different, even if at the moment the SYCL attribute > > is there mostly just to mark something for outlining. > > > > If it doesn't then the case for refactoring and merging them in a future > > patch could be brought up again. > > To summarize: we don't have good arguments to justify re-use of OpenCL > `__kernel` keyword for SYCL mode requested by @aaron.ballman here > https://reviews.llvm.org/D60455#1469150. > > > I can't quite get where the current disconnect comes from but I find it > > extremely unhelpful. > > Let me know how I can help here. > > Additional note. I've submitted initial version of SYCL compiler design > document to the GItHub: > https://github.com/intel/llvm/blob/sycl/sycl/doc/SYCL_compiler_and_runtime_design.md. > Please, take a look and let me know if you have questions. >> Ok, I thought the earlier request was not to add undocumented attributes >> with the spelling? > Right. @Fznamznon, could you document sycl_kernel attribute, please? Do we really need add documentation for attribute which is not presented in SYCL spec and used for internal implementation details only because it has spelling? 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
