Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

[Jan Hubicka <hubicka at ucw dot cz>]

Hi,
> diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c
> index f368cab..3576f7d 100644
> --- a/gcc/c-family/c-opts.c
> +++ b/gcc/c-family/c-opts.c
> @@ -899,6 +899,10 @@ c_common_post_options (const char **pfilename)
>    if (warn_implicit_function_declaration == -1)
>      warn_implicit_function_declaration = flag_isoc99;
>  
> +  /* Declone C++ 'structors if -Os.  */
> +  if (flag_declone_ctor_dtor == -1)
> +    flag_declone_ctor_dtor = optimize_size;

So only reason why this is optimize_size only is the fact that we can't rely on inliner
to fix up the wrappers?
Perhaps we can declone at least the non-comdat ones since there should be no negative
effects?

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.
It is wasteful to do so early and it needs tree-inline to handle unlowered gimple/generic.

> diff --git a/gcc/ipa-inline-analysis.c b/gcc/ipa-inline-analysis.c
> index 9e9087f..21e52a1 100644
> --- a/gcc/ipa-inline-analysis.c
> +++ b/gcc/ipa-inline-analysis.c
> @@ -2796,6 +2796,11 @@ compute_inline_parameters (struct cgraph_node *node, bool early)
>      }
>    estimate_function_body_sizes (node, early);
>  
> +  for (e = node->callees; e; e = e->next_callee)
> +    if (symtab_comdat_local_p (e->callee))
> +      break;
> +  node->calls_comdat_local = (e != NULL);
> +

You need to also add LTO streaming bits (or perhaps just arrange the
flag to be detected at stream-in time)
> diff --git a/gcc/ipa-inline-transform.c b/gcc/ipa-inline-transform.c
> index 7fb4ab9..70fc73d 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 (symtab_comdat_local_p (se->callee))

At this level inline decisions are represented as calls with ->inline_failed
flag false.  You want to test se->inline_failed && symtab_comdat_local_p (se->callee)
> +  else if (callee->calls_comdat_local
> +	   && !symtab_in_same_comdat_p (e->caller, callee))
> +    {
> +      e->inline_failed = CIF_FUNCTION_NOT_INLINABLE;

This is bit of a lie - the function is inlinable under some conditoins.
Lets add CIF_FUNCTION_CALLS_COMDAT_LOCAL for this so it will be easier
in future to diagnose possible problems once comdat locals are more
commonly used.
> @@ -969,14 +972,14 @@ function_and_variable_visibility (bool whole_program)
>  	  node->unique_name = ((node->resolution == LDPR_PREVAILING_DEF_IRONLY
>  				      || node->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)
>  				      && TREE_PUBLIC (node->decl));
> -	  symtab_make_decl_local (node->decl);
>  	  node->resolution = LDPR_PREVAILING_DEF_IRONLY;
> -	  if (node->same_comdat_group)
> +	  if (node->same_comdat_group && TREE_PUBLIC (node->decl))
>  	    /* cgraph_externally_visible_p has already checked all other nodes
>  	       in the group and they will all be made local.  We need to
>  	       dissolve the group at once so that the predicate does not
>  	       segfault though. */
>  	    symtab_dissolve_same_comdat_group_list (node);
> +	  symtab_make_decl_local (node->decl);

OK, so you change symtab_make_decl_local to not clear COMDAT_GROUP_FLAG, so I do not
see where the flag is cleared when the COMDAT symbol is promoted to static (in LTO).
Perhaps symtab_dissolve_same_comdat_group_list should also care clearing the COMDAT_GROUP
flags?

Patch looks OK with those changes,
thanks!
Honza