aaron.ballman added a comment. In D89909#2623211 <https://reviews.llvm.org/D89909#2623211>, @Anastasia wrote:
> In D89909#2617194 <https://reviews.llvm.org/D89909#2617194>, @bader wrote: > >> @Anastasia, do you suggest we copy >> https://github.com/intel/llvm/blob/sycl/sycl/doc/CompilerAndRuntimeDesign.md >> document to clang/docs within this patch? > > For the purpose of this specific topic, you could indeed move "Address spaces > handling" section into the clang documentation. I would suggest creating a > dedicated page where you could gather SYCL specific internals for the clang > community. You could also link the github page to it for more comprehensive > details. > > I would recommend extending the documentation slightly to focus on what > behavior is expected to be implemented rather than how you propose to > implement it in clang too. This is a general guideline for the clang > contributions that suggest that the documentation should be detailed such > that it would be possible to implement it in other frontend/compiler. You > could for example include some more information on expected language semantic > i.e. what you inherit from embedded C and what you inherent from OpenCL or > any other available language features with relevant spec/doc references > including SYCL spec. This should facilitate anyone who needs to understand > the implementation to find further details. > > It would probably make sense to create a separate review to avoid adding too > much noise here. Once you create a review we can link it here and also refine > any necessary details as we go along. These all seem like good suggestions, from my limited perspective, so thank you for them! I'm not opposed to the documentation requests, however, I think that is work that is separable from the proposed changes in this review and could be done with post-commit follow-ups, unless you think the current documentation is wrong as it relates to this patch. I worry that back-and-forth on documentation clarity (while certainly important) is going to hold this patch up for several more months, which is a burden. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89909/new/ https://reviews.llvm.org/D89909 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
