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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
