[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

Wed Mar 10 09:42:48 PST 2021

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