Naghasan added inline comments.

================
Comment at: clang/docs/SYCLSupport.md:123
+traverse all symbols accessible from kernel functions and add them to the
+"device part" of the code marking them with the new SYCL device attribute.
+
----------------
bader wrote:
> Naghasan wrote:
> > ABataev wrote:
> > > bader wrote:
> > > > Naghasan wrote:
> > > > > OpenMP offload uses a similar approach isn't it? Might be worth to 
> > > > > describe how the 2 relates to each other and where they diverge. 
> > > > Do you mean the approach OpenMP compiler uses to outline single-source 
> > > > code parts to offload?
> > > > To be honest, I'm not sure... @ABataev, is there any description how 
> > > > OpenMP compiler outlines device code?
> > > > https://clang.llvm.org/docs/OpenMPSupport.html doesn't provide much 
> > > > details unfortunately.
> > > I don't think we have anything like this. Moreover, currently, there are 
> > > 2 different models, one with outlining by the frontend and another one 
> > > with the outlining by the LLVM.
> > I mentioned that as I know there is some support for CUDA and the clang 
> > driver openmp offload works with multiple frontend passes.
> > If the model(s) is too different then there is no point going further here. 
> > 
> > > Moreover, currently, there are 2 different models, one with outlining by 
> > > the frontend and another one with the outlining by the LLVM.
> > 
> > I do recall some brief conversations about that. Are they meant to work in 
> > pair or one aims to replace the other ?
> What do say if add a TODO here (or in a separate TODO document) to study more 
> about differences between SYCL/OpenMP-offload/CUDA implementation designs?
> It seems to be useful to understanding if we want to re-use other programming 
> model experience with implementing common tasks like device code outlining.
fine by me :)


================
Comment at: clang/docs/SYCLSupport.rst:198
+   class MyObj {
+     accessor<int> _ptr; // accessor<int> contains a pointer to the global 
address space.
+   public:
----------------
Oups sorry, I just noticed  I made a mistake in my suggestion, that's more in 
line with the statement `KernelFuncObj.A.__init(a);` in the pseudo  opencl


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99190

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

Reply via email to