eandrews added a comment.

In D158666#4611497 <https://reviews.llvm.org/D158666#4611497>, @erichkeane 
wrote:

> In D158666#4611494 <https://reviews.llvm.org/D158666#4611494>, @eandrews 
> wrote:
>
>> In D158666#4611481 <https://reviews.llvm.org/D158666#4611481>, @erichkeane 
>> wrote:
>>
>>> I think the .ifunc spelling was an oversight on my part when I implemented 
>>> this, I didn't spend enough time investigating GCC's behavior when 
>>> implementing this feature.  I think the alias is the right way about it, 
>>> but I think the .ifunc should be the alias (at least as far as I can think 
>>> it through right now). I think that works better because it supports a case 
>>> where the 'definition' of the target-clones function is generated with GCC, 
>>> but the 'caller' (also with target clones) comes from clang.  I THINK that 
>>> makes more sense? But perhaps try to chart out the behavior of the 
>>> GCC/Clang "Knows about TC"/"Doesn't know about TC" in each situation to see 
>>> which are troublesome?
>>>
>>> Additionally, this needs a release note.
>>
>> Thanks for taking a look! Can you explain why we need an alias? As in, if we 
>> just remove the .ifunc suffix in the 'ifunc' function here, it should work 
>> without an alias I think. I have to re-check but IIRC this is what GCC does
>
> At least short term, we have to have both .ifunc and <just function name> 
> exposed, else we end up breaking SOMEONE.  At the moment, we're breaking the 
> example you have, however if we just switch the .ifunc to be <function name>, 
> we end up breaking cases where 1 TU is built with Clang 18 and the other with 
> Clang-trunk.  So we need to figure out what the 'compatibility story' is 
> between Clang 18, Clang-Trunk, and GCC with each of them in the position of:
>
> 1- Generated the function TU
> 2- Consumed the function, not knowing about it being TC
> 3- Consumed the function, knowing it was TC.

Makes sense. I'll experiment a bit and update review!


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

https://reviews.llvm.org/D158666

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

Reply via email to