rjmccall added a comment. 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. >> The right place to write the qualifier is the same place where you have to >> write attributes and/or `mutable`, i.e. in the lambda declarator, something >> like `[]() __local { ... }`. >> >> Arguably there ought to be a special-case rule allowing lambdas with no >> captures to be called in an arbitrary address space, but I don't think we >> need to go there in this patch. > > I am trying to understand what are the limitations here. Once we add ability > to qualify lambdas with an addr space it should just work? But also it should > be ok for all lambdas? Apart from captures won'twork for `__constant` addr > space in OpenCL. I think it should just work, yes. Let's ignore my off-hand suggestion about the no-captures special case. > To summarize what has to be done overall to support addr spaces with lambdas: > > 1. Add default AS on object param of internal lambda class member functions. Right, the same default AS as is used for methods generally. > 2. Allow addr space qualifier on lambda expression. Example: [&]() __local > {i++;}; Right. > 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. 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
