[PATCH] Prevent IPA-SRA from creating calls to local comdats (PR 92676)

[Martin Jambor]

Hello,

ping.

Thanks,

Martin


On Mon, Dec 16 2019, Martin Jambor wrote:
> Hi,
>
> since r278669 (fix for PR ipa/91956), IPA-SRA makes sure that the clone
> it creates is put into the same same_comdat as the original cgraph_node,
> so that it can call private comdats (such as the ipa-split bits of a
> comdat that is private).
>
> However, that means that if there is non-comdat caller of a public
> comdat that is modified by IPA-SRA, it now finds itself calling a
> private comdat, which call graph verifier does not like (and for a
> reason, in theory it can disappear and since it is private it would not
> be available from other CUs).
>
> The patch fixes this by performing the fix for PR 91956 only when the
> node in question actually calls a local comdat and when it does, also
> making sure that no callers come from a different same_comdat (disabling
> IPA-SRA if both conditions are true), so that it plays by the rules in
> both modes, does not violate the private comdat calling rule and at the
> same time does not disable the transformation unnecessarily.
>
> The patch also fixes up the calls_comdat_local of callers of the
> modified node, despite that not triggering any known issues.  It has
> passed LTO-bootstrap and testing.  What do you think?
>
> Thanks,
>
> Martin
>
>
> 2019-12-16  Martin Jambor  <mjambor@suse.cz>
>
> 	PR ipa/92676
> 	* ipa-sra.c (struct caller_issues): New fields candidate and
> 	call_from_outside_comdat.
> 	(check_for_caller_issues): Check for calls from outsied of
> 	candidate's same_comdat_group.
> 	(check_all_callers_for_issues): Set up issues.candidate, check result
> 	of the new check.
> 	(process_isra_node_results): Set calls_comdat_local of callers if
> 	appropriate.
> ---
>  gcc/ipa-sra.c | 27 +++++++++++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/ipa-sra.c b/gcc/ipa-sra.c
> index 421c0899e11..8612c8fc920 100644
> --- a/gcc/ipa-sra.c
> +++ b/gcc/ipa-sra.c
> @@ -2895,10 +2895,14 @@ ipa_sra_ipa_function_checks (cgraph_node *node)
>  
>  struct caller_issues
>  {
> +  /* The candidate being considered.  */
> +  cgraph_node *candidate;
>    /* There is a thunk among callers.  */
>    bool thunk;
>    /* Call site with no available information.  */
>    bool unknown_callsite;
> +  /* Call from outside the the candidate's comdat group.  */
> +  bool call_from_outside_comdat;
>    /* There is a bit-aligned load into one of non-gimple-typed arguments. */
>    bool bit_aligned_aggregate_argument;
>  };
> @@ -2920,6 +2924,13 @@ check_for_caller_issues (struct cgraph_node *node, void *data)
>  	     thunks.  */
>  	  return true;
>  	}
> +      if (issues->candidate->calls_comdat_local
> +	  && issues->candidate->same_comdat_group
> +	  && !issues->candidate->in_same_comdat_group_p (cs->caller))
> +	{
> +	  issues->call_from_outside_comdat = true;
> +	  return true;
> +	}
>  
>        isra_call_summary *csum = call_sums->get (cs);
>        if (!csum)
> @@ -2942,6 +2953,7 @@ check_all_callers_for_issues (cgraph_node *node)
>  {
>    struct caller_issues issues;
>    memset (&issues, 0, sizeof (issues));
> +  issues.candidate = node;
>  
>    node->call_for_symbol_and_aliases (check_for_caller_issues, &issues, true);
>    if (issues.unknown_callsite)
> @@ -2960,6 +2972,13 @@ check_all_callers_for_issues (cgraph_node *node)
>  		 node->dump_name ());
>        return true;
>      }
> +  if (issues.call_from_outside_comdat)
> +    {
> +      if (dump_file)
> +	fprintf (dump_file, "Function would become private comdat called "
> +		 "outside of its comdat group.\n");
> +      return true;
> +    }
>  
>    if (issues.bit_aligned_aggregate_argument)
>      {
> @@ -3759,8 +3778,12 @@ process_isra_node_results (cgraph_node *node,
>      = node->create_virtual_clone (callers, NULL, new_adjustments, "isra",
>  				  suffix_counter);
>    suffix_counter++;
> -  if (node->same_comdat_group)
> -    new_node->add_to_same_comdat_group (node);
> +  if (node->calls_comdat_local && node->same_comdat_group)
> +    {
> +      new_node->add_to_same_comdat_group (node);
> +      for (cgraph_edge *cs = new_node->callers; cs; cs = cs->next_caller)
> +	cs->caller->calls_comdat_local = true;
> +    }
>    new_node->calls_comdat_local = node->calls_comdat_local;
>  
>    if (dump_file)
> -- 
> 2.24.0