[PATCH] D78655: [HIP] Add -fhip-lambda-host-device

Paul Fultz II via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue May 5 17:17:23 PDT 2020


pfultz2 added a comment.

> A slight variation on that, that might be better: lambdas with no lambda-capture are implicitly HD; lambdas with any lambda-capture (which must therefore have an enclosing function) inherit the enclosing function's HDness.

Lambdas should already inherit the enclosing functions HDness. Keeping capturing lambdas as implictly HD matches closer the behavior in HIP/HCC, and as we are porting code it is not always clear which lambdas need explicit HD annotation since running on the device is an implementation detail.

Capturing lambdas has its pitfalls but they are no different from the same pitfalls that happen with asynchronous programming or signal callbacks.



================
Comment at: clang/lib/Sema/SemaCUDA.cpp:733
+      (Method->isConstexpr() ||
+       (getLangOpts().HIPLambdaHostDevice && !LambdaHasRefCaptures(LI)))) {
+    Method->addAttr(CUDADeviceAttr::CreateImplicit(Context));
----------------
rsmith wrote:
> The reference captures check seems quite strange to me. A copy capture of a pointer could have the same problem, as could a copy capture of a class that contains a reference or a pointer. As could an init-capture.
> 
> These kinds of quirky language rules are usually more trouble than they're worth.
Capturing by value is not always an error, only when copying a pointer to a host variable. but this requires a lot more static analysis to diagnose. However, capturing by reference is almost always wrong(at least with the current HIP) when the context is host and the lambda is called on the device.

Therefore, we avoid this scenario by not making such lambdas implicitly HD, but the error message may not be quite as clear. It is a quirky language rule, and we could remove this restriction and rely on a warning or static analysis to diagnose the issue.


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

https://reviews.llvm.org/D78655





More information about the cfe-commits mailing list