bader added a comment. In D89909#2606180 <https://reviews.llvm.org/D89909#2606180>, @Anastasia wrote:
> In D89909#2600859 <https://reviews.llvm.org/D89909#2600859>, @aaron.ballman > wrote: > >> Just a few minor nits from me, but I'm mostly wondering: where are we at >> with this and are there still substantive changes required? (I looked >> through the comments, but there's a lot of back-and-forth since Oct and I'm >> not certain what's holding the patch back currently.) > > To make it short, from my side I am not very clear about the overall design. > From the SYCL spec side, there is no indication of what compiler extensions > are needed and if at all. As a result, some of the design choices are unclear > to me - in particular why SPIR target would need a separate address space map > for SYCL. This is not how it was intended originally and I am worried that > this will create issues for the consumers of IR to handle two different > formats. But in general, if the community is now to maintain this code we > should at least have some deeper understanding of it. > > I would suggest starting from some high-level documentation that provides the > details of the compiler extension being implemented. Perhaps the > documentation that @bader has linked earlier could be used as a starting > point with some more details that would allow assessing and reviewing the > changes. @Anastasia, do you suggest we copy https://github.com/intel/llvm/blob/sycl/sycl/doc/CompilerAndRuntimeDesign.md document to clang/docs within this patch? 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
