> On 12/12/2013 03:08 PM, Jan Hubicka wrote:
> >So only reason why this is optimize_size only is the fact that we can't rely
> >on inliner
> >to fix up the wrappers?
>
> I was just being conservative. In fact, the inliner seems to handle
> small [cd]tors well, inlining them into the wrappers and then into
> callers. Do you think I should enable it more generally?
Yep, inliner is designed to handle wrappers well (C++ programs are full of
them). I see two
possible problems
1) the rule about not inlining/clonning functions with comdat local calls will
inhibit
some propagation where we need to propagate constants across the wrapper.
I am not sure if that is much of problem in practice though.
2) I am not sure if debug info is right? I suppose the wrapper can be just
abstract
>
> >In longer term, it would be nice to disable frontend driven clonning and just
> >produce the wrappers/virtual clones to be handled by middle-end.
>
> Agreed. The only snag is handling variadic functions.
Yep, it is sad we have those to go specially here and in thunks :(
> diff --git a/gcc/ipa-inline-transform.c b/gcc/ipa-inline-transform.c
> index 7fb4ab9..87c2479 100644
> --- a/gcc/ipa-inline-transform.c
> +++ b/gcc/ipa-inline-transform.c
> @@ -272,6 +272,18 @@ inline_call (struct cgraph_edge *e, bool update_original,
> inline_update_overall_summary (to);
> new_size = inline_summary (to)->size;
>
> + if (callee->calls_comdat_local)
> + to->calls_comdat_local = true;
> + else if (to->calls_comdat_local && symtab_comdat_local_p (callee))
> + {
> + struct cgraph_edge *se = to->callees;
> + for (; se; se = se->next_callee)
> + if (se->inline_failed && symtab_comdat_local_p (se->callee))
> + break;
> + if (se == NULL)
> + to->calls_comdat_local = false;
> + }
Actually this is still not quite right - when we have !inline_failed case,
we need to check if callee calls comdat local.
Moreover when we turn comdat_local to false, we need to recomute also
function it is inlined into.
while (to->callers && !to->callers->inline_failed)
recompute for to->callers->caller
to = to->callers->caller
This path won't execute since we won't inline the wrapper before we inline
into the wrapper, so perhaps we can just have assert with comment that
this will be needed in future. I think it is just better to be sure
we won't forget the flag mistakely on.
> diff --git a/gcc/symtab.c b/gcc/symtab.c
> index dc700e7..a448dea 100644
> --- a/gcc/symtab.c
> +++ b/gcc/symtab.c
> @@ -538,6 +538,7 @@ symtab_dissolve_same_comdat_group_list (symtab_node *node)
> {
> next = n->same_comdat_group;
> n->same_comdat_group = NULL;
> + DECL_COMDAT_GROUP (n->decl) = NULL_TREE;
> n = next;
> }
> while (n != node);
I think you should rather do it only for statics, DECL_ONE_ONLY functions go
through
specially in visibility and you may end up making function !ONE_ONLY before it
is
processed because we decided other function in the group to be promoted to
static.
In this case we may end up to handle the other as normal extern function by
mistake.
OK with those changes,
thanks!
Honza
