bader added a comment. In D80932#2085848 <https://reviews.llvm.org/D80932#2085848>, @Anastasia wrote:
> In D80932#2083185 <https://reviews.llvm.org/D80932#2083185>, @bader wrote: > > > In D80932#2080631 <https://reviews.llvm.org/D80932#2080631>, @Anastasia > > wrote: > > > > > > @Anastasia, if we make this change specific to SYCL mode, will it > > > > address your concerns? > > > > > > I can't answer this question for the reasons I have explained above. > > > > > > Sorry, I'm not sure that I get your concern correctly, so let me clarify: > > is it allowing conversion between pointers w/ and w/o address space > > annotation in non-SYCL mode or using OpenCL address space attributes in > > SYCL mode? > > > > Just to help you to understand the proposed design, I created the full > > patch for address space handling in SYCL: > > https://github.com/bader/llvm/pull/18. There are few CodeGen tests > > validating LLVM IR for SPIR target. Fee free to ask any questions on this > > PR. > > > Thanks! This is good but it is only an implementation of the design. Deducing > the design from an implementation is time-consuming and not sure it is even > feasible. I don't want to waste our time to provide you detailed feedback > based on my interpretation of your design and invalid assumptions. I don't > want to bind you to any particular format and we don't have any strict > requirement for this in LLVM either, but I would encourage you to take a look > at some of RFC threads sent to cfe-dev that explain new design concepts. > Perhaps, they can help you to understand what information can be provided as > a starting point to new design discussions. > > Particularly I would suggest covering these two points: > > - SYCL specifies address spaces as classes and it is not very obvious how did > you come from libraries classes to modifications in Clang? I believe there > are a number of design choices that you could make. One might think that you > can just implement address space classes using existing Clang functionality. > But if it is not sufficient it is important to understand what modifications > are required and why. > - Other aspects that are important to highlight whether your design has any > limitations. For example, I don't quite understand your need for > `SPIR*SYCLDeviceTargetInfo`. Is there anything in your design that limits > compilation to one particular target? > > Overall, I see that there are a lot of changes in CodeGen that are related > to the language semantic. I believe that CodeGen is supposed to be dialing > primarily with target-specific logic and I don't know if you should change > them to query target specific details instead. Also most of your CodeGen > changes are not related to OpenCL so I would make sure to loop @rjmccall in > this thread. @Anastasia, sorry for the delay. I've sent an email with RFC covering these questions to cfe-dev mailing list - http://lists.llvm.org/pipermail/cfe-dev/2020-June/066047.html. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80932/new/ https://reviews.llvm.org/D80932 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
