> > > +/* If E does not lead to a thunk, simply redirect it to N. Otherwise
> > > create
> > > + one or more equivalent thunks for N and redirect E to the first in the
> > > + chain. */
> > > +
> > > +void
> > > +redirect_edge_duplicating_thunks (struct cgraph_edge *e, struct
> > > cgraph_node *n,
> > > + bitmap args_to_skip)
> > > +{
> > > + cgraph_node *orig_to = cgraph_function_or_thunk_node (e->callee);
> > > + if (orig_to->thunk.thunk_p)
> > > + n = duplicate_thunk_for_node (orig_to, n, args_to_skip);
> >
> > Is there anything that would pevent us from creating a new thunk for
> > each call?
>
> No, given how late we have discovered it, it probably only happens
> very rarely. Moreover, since you have plans to always inline only
> directly called thunks for the next release, which should be the
> ultimate solution, I did not think it was necessary or even
> appropriate at this stage.
A lot of code iterate over thunks/aliases and expect this to be cheap operation.
We thus need to be sure we won't create very many thunks or aliases of a given
function internally.
In order to trigger quadratic behaviour here, we only need a single function
call used very often in a big project, like mozilla, to create uncontrolled
numbers of thunks. I would suggest to just walk existing thunks before
creating new looking if there is one mathcing our needs. Same code is in
making local aliases. This change is pre-approved.
>
> >
> > Also I think you need to avoid this logic when THIS parameter is being
> > optimized out
> > (i.e. it is part of skip_args)
>
> You are of course right. However, skipping the creation of a new
> thunk when we are also removing parameter this leads to verification
> errors again, so I had to also teach the verifier that this case is
> actually OK. Moreover, although it seems that currently all
That is fine with me.
> non-this_adjusting thunks are expanded before IPA-CP runs, I made sure
> the skipping logic checked that flag.
Yes, we only keep the simple thunks in non-lowered form, but I do not
see how it makes difference for you.
>
> Accidently, the two original testcases are removing parameter this so
> I added a new one, which also shows how current trunk miscompiles
> stuff. Unfortunately, at the moment it relies on speculative edges
> and so when IPA-CP correctly redirects calls to a thunk, inlining
> gives up and removes the edge, so the IPA-CP transformation is not
> run-time checked. However, the cgraph verifier does see the edge
> before that happens and is OK with it.
You can probably play with anonymous namespaces and final flags to get
it devirtualized unconditnally.
>
> I have also took the liberty of removing an extra call to
> cgraph_function_or_thunk_node (clone_of_p calls it too) and a clearly
> obsolete comment from verify_edge_corresponds_to_fndecl.
>
> Bootstrapped and tested on x86_64-linux. OK for trunk?
>
> Thanks,
>
> Martin
>
>
> 2014-03-31 Martin Jambor <mjam...@suse.cz>
>
> * cgraph.h (cgraph_clone_node): New parameter added to declaration.
> Adjust all callers.
> * cgraph.c (clone_of_p): Also return true if thunks match.
> (verify_edge_corresponds_to_fndecl): Removed extraneous call to
> cgraph_function_or_thunk_node and an obsolete comment.
> * cgraphclones.c (build_function_type_skip_args): Moved upwards in the
> file.
> (build_function_decl_skip_args): Likewise.
> (set_new_clone_decl_and_node_flags): New function.
> (duplicate_thunk_for_node): Likewise.
> (redirect_edge_duplicating_thunks): Likewise.
> (cgraph_clone_node): New parameter args_to_skip, pass it to
> redirect_edge_duplicating_thunks which is called instead of
> cgraph_redirect_edge_callee.
> (cgraph_create_virtual_clone): Pass args_to_skip to cgraph_clone_node,
> moved setting of a lot of flags to set_new_clone_decl_and_node_flags.
>
> testsuite/
> * g++.dg/ipa/pr60640-1.C: New test.
> * g++.dg/ipa/pr60640-2.C: Likewise.
> * g++.dg/ipa/pr60640-3.C: Likewise.
OK, with the change above.
Honza
