Re: [PATCH] Make redirection only for target_clones: V2 (PR ipa/85329).

[Jan Hubicka <hubicka at ucw dot cz>]

> On Fri, Apr 13, 2018 at 04:33:40PM +0200, Martin Liška wrote:
> > > Ah, but we emit the resolver only if we see a use of it.  That sounds quite
> > > broken, resolver in each TU that uses it?  Better to have one at each
> > > definition...
> > > 
> > > 	Jakub
> > > 
> > 
> > So after quite some time I would need some brainstorming as I'm not sure how to
> > fix that properly. Let's start how 'target' attribute works and I believe it should
> > behave same as target_clones attribute:
> 
> I think the target_clones behavior if you put the <function>.resolver into
> .text.<function> section in <function> comdat rather than
> .text.<function>.resolver and <function>.resolver comdat is reasonable.
> 
> The thing is that both the <function>.resolver and <function> symbols are
> defined in the section in which the resolver is defined.
> Maybe it would be nice if the resolver was made not exported out of the TU,
> and eventually, not necessarily now for GCC8, turn the specializations into
> non-exported symbols too and put them into the same section or different
> sections of the same comdat group.
> 
> For ctors and dtors we need extra care about the aliases, not sure if we can
> have an alias to ifunc symbol, or if we need to emit two ifunc symbols with
> the same resolver or what exactly.
> 
> And yes, it would be nice if the target attribute multiversioning worked
> similarly to this.  It would change behavior for it in ABI incompatible way,
> the question is how much work it would be as well.
> 
> What you can do right now with the target attribute multiversioning and
> couldn't do after the change would be e.g.
> mv.h:
> __attribute__((target ("default"))) void foo ();
> __attribute__((target ("avx"))) void foo ();
> mv1.C:
> #include "mv.h"
> __attribute__((target ("default"))) void foo () {}
> mv2.C:
> #include "mv.h"
> __attribute__((target ("avx"))) void foo () {}
> mv3.C:
> #include "mv.h"
> void bar () { foo (); }
> You couldn't this in the semantics closer to target_clones, all the
> definitions would need to be done in the same file which is where you'd also
> get the ifunc symbol.
> 
> IMHO it is worth changing the semantics anyway, because the current one
> isn't very well thought out, it doesn't really work well with comdats, can
> have too many resolvers with the associated costs, etc.
> 
> Honza, do you agree?

Yes, it looks reasonable.  I will give it bit more tought today (I am on a trip
with only sporadic internet access)

Honza
> 
> 	Jakub