erichkeane added a comment.

In D76620#2328305 <https://reviews.llvm.org/D76620#2328305>, @rjmccall wrote:

> In D76620#2328031 <https://reviews.llvm.org/D76620#2328031>, @erichkeane 
> wrote:
>
>> In D76620#2328011 <https://reviews.llvm.org/D76620#2328011>, @rjmccall wrote:
>>
>>> In D76620#2327988 <https://reviews.llvm.org/D76620#2327988>, @erichkeane 
>>> wrote:
>>>
>>>> In D76620#2327976 <https://reviews.llvm.org/D76620#2327976>, @rjmccall 
>>>> wrote:
>>>>
>>>>> In D76620#2327910 <https://reviews.llvm.org/D76620#2327910>, @erichkeane 
>>>>> wrote:
>>>>>
>>>>>> In D76620#2327901 <https://reviews.llvm.org/D76620#2327901>, @rjmccall 
>>>>>> wrote:
>>>>>>
>>>>>>> You know on both sides that a lambda is used as a kernel, yes?  Why not 
>>>>>>> simply introduce that into the mangling of lambdas, so that the subset 
>>>>>>> of lambdas used as kernels form a stable sequence?
>>>>>>
>>>>>> Coming up with said stable sequence is somewhat difficult as well.  A 
>>>>>> strict integer ordering didn't end up being stable, as some of the 
>>>>>> kernel handling can cause instantiations to happen in a slightly 
>>>>>> different ordering, which messed that up, as would use of the builtin. 
>>>>>> We wanted to find a way that was dependent on the source code alone.  
>>>>>> The "Quick and Dirty" solution was line/column numbers, though we 
>>>>>> considered a hash of that same information to at least make it shorter.
>>>>>
>>>>> Certainly you wouldn't want a *global* sequence ID.  However, lambdas 
>>>>> can't just occur globally, they're always part of some declaration that 
>>>>> they can be scoped to, so that you have e.g. "the third kernel lambda 
>>>>> within function X".  I fail to see how that wouldn't address the concern 
>>>>> about instantiation order, and it's still source-directed.
>>>>
>>>> Hmm... I'll have to consider that.  That is an interesting thought. 
>>>> Presumably we could just copy the 'getLambdaManglingNumber' stuff in that 
>>>> case, and place the value in the same location in the mangling, with some 
>>>> sort of discriminator (to avoid conflicting manglings).
>>>
>>> The Itanium mangling already produces a different lambda mangling number 
>>> for different lambda signatures.  You just need the kernel-ness to be part 
>>> of the signature.
>>
>> It does it via the getLambdaManglingNumber I believe, right?  The idea would 
>> be to have a getKernelLambdaManglingNumber that does something similar, and 
>> needs something to avoid conflicting, which is the discriminator I was 
>> mentioning.
>
> Right, so if you look how that's ultimately derived in 
> ItaniumNumberingContext::getManglingNumber, you'll see that every context has 
> different sequences for unique lambda-sigs.  If you can just put "this is a 
> kernel" into the lambda-sig, you'll automatically get a stable sequence for 
> kernel lambdas.  But that would rely on immediately knowing that a lambda 
> will be used as a kernel.
>
>>>> The only other concern I have is WHEN we know that a lambda is used in a 
>>>> kernel, which we don't until it is called (and can cause confusion when it 
>>>> is a template parameter and called later).
>>>
>>> Oh, yes, if you don't know that a lambda is used as a kernel locally then 
>>> this falls apart a bit.  You could of course pretend for ABI purposes that 
>>> there's a new intermediate lambda at the kernel use point when the lambda 
>>> is not local to the current function.  I don't think there's anything which 
>>> relies on mangling lambdas before the function they're contained within is 
>>> fully type-checked.
>>
>> One thing that might save us from this, is we aren't actually modifying the 
>> LAMBDAs mangling, we're modifying the KERNEL's mangling. The lambda itself 
>> keeps its mangling, but the kernel that calls it (which is the device/host 
>> boundary) is what has its name changed.  So at least at that point we know 
>> some about it.  And, actually, since we are generating the 'integeration 
>> header' lookup table AND the kernel in identical (if not the same) 
>> invocation, perhaps a global counter WOULD work...  My coworker is 
>> investigating this now, so hopefully she'll be able to prove out this idea 
>> and give further feedback here.
>
> I feel obliged to point out that, if you're worried about a single function 
> potentially having a different set of lambdas in different translation modes, 
> that's also going to affect things like template specialization identity.  
> That is, suppose a kernel function is formed for a particular lambda, and in 
> one translation mode it's the first lambda with that signature in a function 
> and in another mode it's the second.  That's going to change the immediate 
> mangling of the lambda, but it's also going to change the mangling of 
> templates used with that lambda.  For example, if you have

We typically disallow the kernel itself having a different lambda in each mode, 
as that doesn't make much sense.  The SYCL language is a little coy with the 
ODR when it comes to macros, but typically the kernel calls themselves have to 
be consistent in a single application.

>   template <class F> void run_wrapped_as_kernel(const F &f) {
>     use_as_kernel([=] { f(); });  // this lambda is always the first in its 
> context, but the identity of that context is contingent on something unstable
>   }
>   
>   void foo() {
>   #if mode1
>     auto blah = []{}; // a lambda that's only present in one language mode
>   #endif
>     use_as_kernel([] { ... }); // the mangling number for this is disturbed
>   }
>
> So if you're going to try to solve this (instead of just making it the user's 
> responsibility to not do), it's almost like you need some way to 
> retroactively number every lambda implicated in any way by the host/kernel 
> interface, and then have a way to mangle things completely from scratch using 
> those stable numberings for this kernel-numbering thing.

THAT right there is exactly the problem we ran into, the 'blah' changing the 
kernel numbering. Our previous 'retroactive way' of numbering was line&column 
numbers + the same for macro invocations as it was stable with that. Mangling 
them retroactively is perhaps a possibility, though it would be great if we 
could force a number based on textual order (complicated further by template 
instantiations).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76620

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to