ABataev added inline comments.

================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2477
 
+  if (LangOpts.SYCLIsDevice && Global->hasAttr<SYCLKernelAttr>()) {
+    // SYCL kernels can be templated and not called from anywhere in the
----------------
bader wrote:
> ABataev wrote:
> > bader wrote:
> > > ABataev wrote:
> > > > Need to check if the decl must be emitted at all.
> > > Let me check that I get it right. You suggest adding `if 
> > > (MustBeEmitted(Global))`, right?
> > > ```
> > >   if (LangOpts.SYCLIsDevice && Global->hasAttr<SYCLKernelAttr>() && 
> > > MustBeEmitted(Global)) {
> > >     ...
> > >     addDeferredDeclToEmit(GD);
> > >     return;
> > >   }
> > > ```
> > Yes
> Okay. Making this change requires additional adjustments in the patch and I 
> have a few options.
> In this patch we do not add any logic forcing compiler to emit SYCL kernel. 
> This logic is supposed to be added by follow-up patch (currently under SYCL 
> working group review here https://github.com/intel/llvm/pull/249), which add 
> code emitting "externally visible" OpenCL kernel calling function object 
> passed to SYCL kernel function.
> 
> I can:
> 1) Temporally remove CodeGen test and add updated version back with the 
> follow-up patch
> 2) Do change making SYCL kernels "externally visible" and revert this change 
> with the follow-up patch (this is kind of current logic which emits SYCL 
> kernels unconditionally)
> 3) Merge two patches and submit them together, but I assume it will 
> significantly increase the size of the patch.
Probably, better would be to split the patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

Reply via email to