[PATCH] D78655: [CUDA][HIP] Let non-caputuring lambda be host device

Artem Belevich via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 22 15:35:52 PDT 2020


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





More information about the cfe-commits mailing list