tra added inline comments.

================
Comment at: clang/lib/Sema/SemaCUDA.cpp:753
     return;
+  if (LI.Default == LCD_None && LI.Captures.size() == 0) {
+    Method->addAttr(CUDADeviceAttr::CreateImplicit(Context));
----------------
pfultz2 wrote:
> There should at least be a flag to enable capturing lambdas to be implicitly 
> HD. I dont really understand the rational for making capturing lambdas not 
> implicitly HD. It seems like its trying to prevent using an address to host 
> on the device, but I dont see how this prevents that at all. 
> 
> This will also break the compilation in rocm. Should we use a fork of llvm to 
> compile rocm?
@pfultz2:

> This will also break the compilation in rocm. Should we use a fork of llvm to 
> compile rocm?

Could you give an example to demonstrate current use and how it will break? My 
understanding that the patch *relaxes* the restrictions on lambdas so in theory 
not promoting capturing lambdas preserves the status quo. 

As for the fork, my response would be an empathic "no, please don't do it". 
Fork == different compiler == showstopper for various use cases. It would 
definitely be an issue for us at Google. 

Considering that we're still probing our way towards making lambdas more 
useful, it may be a bit premature to heavily depend on any particular 
implementation detail of an experimental feature, even if it happens to work. 
We'll need to figure out an approach that will be sustainable long-term and 
forked compiler is a rather large and hard-to-maintain hammer for this. In my 
experience, adapting source code ends up being more manageable long-term.


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

https://reviews.llvm.org/D78655



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

Reply via email to