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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to