diff mbox series

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

Message ID ri6h820p6tp.fsf@suse.cz
State New
Headers show
Series Prevent IPA-SRA from creating calls to local comdats (PR 92676) | expand

Commit Message

Martin Jambor Dec. 16, 2019, 4:34 p.m. UTC
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(-)

Comments

Martin Jambor Jan. 9, 2020, 12:17 p.m. UTC | #1
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
Martin Jambor Feb. 3, 2020, 12:24 p.m. UTC | #2
Hello Honza,

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
Martin Jambor March 11, 2020, 3:53 p.m. UTC | #3
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
Jan Hubicka March 20, 2020, 3:20 p.m. UTC | #4
> 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.

OK except for ...
> @@ -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;

You need to walk all aliases here.
> +    }
>    new_node->calls_comdat_local = node->calls_comdat_local;
>  
>    if (dump_file)
> -- 
> 2.24.0
>
Martin Jambor April 2, 2020, 9:52 a.m. UTC | #5
Hi,

On Fri, Mar 20 2020, Jan Hubicka wrote:
>

[...]

>
> OK except for ...
>> @@ -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;
>
> You need to walk all aliases here.

Oh, right.  I assume thunks too?  (I acknowledge I swapped most of this
out since I fixed this bug and now cannot really say if we can have a
thunk there in the call chain.)  So like this?

Passed bootstrap and testing on x86_64-linux, running LTO bootstrap now.

Thanks,

Martin


2020-04-01  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.
	(mark_callers_calls_cdlcl): New function.
	(process_isra_node_results): Set calls_comdat_local of callers if
	appropriate.
---
 gcc/ChangeLog | 13 +++++++++++++
 gcc/ipa-sra.c | 38 ++++++++++++++++++++++++++++++++++++--
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 654356c8dc8..e9183f4921a 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,16 @@
+2020-04-01  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.
+	(mark_callers_calls_cdlcl): New function.
+	(process_isra_node_results): Set calls_comdat_local of callers if
+	appropriate.
+
 2020-04-01  Jakub Jelinek  <jakub@redhat.com>
 
 	PR middle-end/94423
diff --git a/gcc/ipa-sra.c b/gcc/ipa-sra.c
index 31de527d111..45ea62522ab 100644
--- a/gcc/ipa-sra.c
+++ b/gcc/ipa-sra.c
@@ -2897,10 +2897,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;
 };
@@ -2922,6 +2926,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)
@@ -2944,6 +2955,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)
@@ -2962,6 +2974,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)
     {
@@ -3679,6 +3698,17 @@ push_param_adjustments_for_index (isra_func_summary *ifs, unsigned base_index,
     }
 }
 
+/* Worker for all call_for_symbol_thunks_and_aliases.  Set calls_comdat_local
+   flag of all callers of NODE.  */
+
+static bool
+mark_callers_calls_cdlcl (struct cgraph_node *node, void *)
+{
+  for (cgraph_edge *cs = node->callers; cs; cs = cs->next_caller)
+    cs->caller->calls_comdat_local = true;
+  return false;
+}
+
 
 /* Do final processing of results of IPA propagation regarding NODE, clone it
    if appropriate.  */
@@ -3763,8 +3793,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);
+      new_node->call_for_symbol_thunks_and_aliases (mark_callers_calls_cdlcl,
+						    NULL, true);
+    }
   new_node->calls_comdat_local = node->calls_comdat_local;
 
   if (dump_file)
diff mbox series

Patch

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)