rjmccall added a comment. In D69938#1742026 <https://reviews.llvm.org/D69938#1742026>, @Anastasia wrote:
> In D69938#1741713 <https://reviews.llvm.org/D69938#1741713>, @rjmccall wrote: > > > In D69938#1741080 <https://reviews.llvm.org/D69938#1741080>, @Anastasia > > wrote: > > > > > In D69938#1737196 <https://reviews.llvm.org/D69938#1737196>, @rjmccall > > > wrote: > > > > > > > It does make logical sense to be able to qualify the call operator of a > > > > lambda. The lambda always starts as a temporary, so presumably we want > > > > the default address space qualifier on lambdas to be compatible with > > > > the address space of temporaries. However, that's probably also true > > > > of the default address qualifier on methods in general or you wouldn't > > > > be able to call them on temporaries and locals. Thus, we should be > > > > able to use the same default in both cases, which seems to be what > > > > you've implemented. > > > > > > > > It even makes sense to be able to qualify a lambda with an address > > > > space that's not compatible with the address space of temporaries; that > > > > just means that the lambda has to be copied elsewhere before it can be > > > > invoked. > > > > > > > > > Just to clarify... Do you mean we should be able to compile this example: > > > > > > [&] __global { > > > i++; > > > } (); > > > > > > > > > > > > Or should this result in a diagnostic about an addr space mismatch? > > > > > > It should result in a diagnostic on the call. But if you assigned that > > lambda into global memory (somehow) it should work. > > > Ok, makes sense so something like this should compile fine: > > __global auto glob = [&] __global { ... }; Right, a call to `glob()` should work in this case. I don't know if `__global` would parse here without `()`, though; in the grammar, it looks like attributes and so on have to follow an explicit parameter clause. >>> 3. Diagnose address space mismatch between variable decl and lambda expr >>> qualifier Example: __private auto llambda = [&]() __local {i++;}; // error >>> no legal conversion b/w __private and __local >>> >>> I think 1 is covered by this patch. I would like to implement 2 as a >>> separate patch though to be able to commit fix for 1 earlier and unblock >>> some developers waiting for the fix. I think 3 already works and I will >>> just update the diagnostic in a separate patch too. >> >> I agree that 3 should just fall out. And yeah, implementing 2 as a separate >> patch should be fine. Please make sure 3 is adequately tested in this patch. > > Ok, since we can't qualify lambda expr with addr spaces yet there is not much > I can test at this point. `__constant` lambda variable seems to be the only > case with a diagnostic for OpenCL. You can certainly copy a lambda type into a different address space, or construct a pointer to a qualified lambda type, e.g. `(*(__constant decltype(somelambda) *) nullptr)()`. John. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69938/new/ https://reviews.llvm.org/D69938 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
