pfultz2 added a comment.

> It captures addresses as seen at the point in time on the processor which 
> executed the constructor.

Yea and the same happens when assigning the address to a pointer, which is 
later used on a different device.

> Another point that capturing lambdas are not something ready for the prime 
> time.

The same issues exist with functions. We dont prevent passing a pointer to host 
memory to a device function. I guess because the analysis to do so is 
incomplete and expensive. A lambda capturing by reference does seem simpler to 
analyze at least for implicit HD.

> Could you elaborate? I'm not sure what you mean by we seem to delay this and 
> what does it have to do with the assertion that lambdas are not always 
> constexpr by default?

Lambdas are not always `constexpr`, and this patch doesnt make lambdas to 
always be generated for host and device, even though, it does always have a HD 
attribute. Instead it pushes the decision to emit the lambda for host or device 
to later when we are emitting the code for codegen(at least thats how I 
understand this happening, @yaxunl can correct me if I am wrong here).

> I think it's done here:

I actually meant how constexpr lambdas was implemented, which I can see here:

https://github.com/llvm/llvm-project/blob/master/clang/lib/Sema/SemaExpr.cpp#L16087

It doesn't annotate a lambda as `constexpr`. Instead it tries to evaluate all 
lambdas as `constexpr` when its used in `constexpr` context. This is similar to 
what we do for HD. We treat all lambdas as HD, but then only emit it for the 
device when its called in a device function. The big difference is that 
`constexpr` doesn't participate in overload resolution so constexpr lambdas are 
not observable by the user whereas host and device attributes are.

> We basically slap implicit HD on constexpr functions when we process function 
> declarations. It's likely that lambdas may go through a different code path 
> and miss this.

Yea, which wont work for lambdas since `NewD->isConstexpr()` will return 
false(unless the user explicitly adds `constexpr`). We could traverse the AST 
to see if the lambda only calls constexpr functions and then annotate it with 
HD(we could also extend this to HD functions as well). However, this seems 
costly.

It would be better to take the same approach for `constexpr` lambdas and treat 
all lambdas as potentially HD(which is what this patch seems to do).


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