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

[Alexey Bader via Phabricator via llvm-commits]

bader added a comment.

> Did anyone conclude there that the language address spaces should be added for SYCL? I can't see any of this. In fact I don't even think there was any conclusion on the RFC. You should first make your design clear and agreed before going ahead with the implementation. I personally still think that address space AST attribute is not the right path for SYCL. And I haven't seen any evidence that it is the best solution to what you are trying to achieve.

I'll try to summarize my understanding of the current state. So far there were three options discussed in the mailing list thread:

1. We started with re-using OpenCL address space attributes (https://reviews.llvm.org/D80932), but there was a concern regarding deviation from OpenCL semantics in SYCL mode for these attributes. In same review, there was a proposal to implement a separate set of attributes to address this concern, which lead to the solution #2.
2. This patch (https://reviews.llvm.org/D89909) implements proposal from https://reviews.llvm.org/D80932 review comments to add SYCL specific attributes to avoid interference with OpenCL mode.
3. Bevin Hansson suggested to resolve semantic difference between SYCL and OpenCL mode for OpenCL address space attributes handling by re-using this work: https://reviews.llvm.org/D62574, which provides an infrastructure to customize address space conversion rules based on information available in ASTContext. This seems to be viable approach to implement the difference between OpenCL and SYCL modes.

Open source SYCL implementation from https://github.com/intel/llvm/tree/sycl currently implements option #1 and option #2 was used in the past, so I'm quite confident that these will work. I haven't tried to prototype option #3, but as mentioned in RFC, https://reviews.llvm.org/D62574 provides necessary infrastructure to customize address space attributes already supported by the compiler.
As we already agreed in the RFC, we should have address space attributes in AST for SYCL and the only question we need to answer is whether SYCL mode can re-using existing attributes or add something new.

My current plan is to continue with option #2, unless Bevin's patch is going to be committed soon. I'd like to have something committed to unblock other changes relying on address space attributes. Let me know if you have any concerns regarding this plan.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89909