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 <mjam...@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
