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

Thu Mar 25 06:05:10 PDT 2021

Anastasia added a comment.

> Having multiple maps is not something new to the clang community. Similar approach AMDGPU target applies to customize address space mapping for OpenCL language (https://github.com/llvm/llvm-project/blob/main/clang/lib/Basic/Targets/AMDGPU.cpp#L354).
>
> I updated address space map names to avoid confusion that mapping change is specific to the language mode. Now they use the same naming scheme as AMDGPU maps.

Well AMDGPU is a specific target in contrast to SPIR. But the question is whether the map that has been renamed is only intended for SYCL or can it be used for OpenCL or other languages too? My worry is that the original design was intended to separate language and target information but somehow it is now driven towards tangling them together more and more.

> Using InferAddressSpace pass is not required for the targets supporting SPIR-V with pointers to generic address space, but it can be beneficial to produce more efficient native code.

Ok, that's good to know that it is only intended for optimisations!

> I also created a review request with SYCL design documentation - https://reviews.llvm.org/D99190

Thanks. I will take a look at the address space part. My first impression is that it would be good to abstract away from the implementation in clang and start from the language semantic that you are trying to get. Perhaps it will be easier if I provide concrete questions on the review.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89909/new/

https://reviews.llvm.org/D89909