[PATCH] ipa: Avoid excessive removing of SSAs (PR 113757)

[Jan Hubicka]

> On Thu, Feb 08 2024, Martin Jambor wrote:
> > Hi,
> >
> > PR 113757 shows that the code which was meant to debug-reset and
> > remove SSAs defined by LHSs of calls redirected to
> > __builtin_unreachable can trigger also when speculative
> > devirtualization creates a call to a noreturn function (and since it
> > is noreturn, it does not bother dealing with its return value).
> >
> > What is more, it seems that the code handling this case is not really
> > necessary.  I feel slightly idiotic about this because I have a
> > feeling that I added it because of a failing test-case but I can
> > neither find the testcase nor a reason why the code in
> > cgraph_edge::redirect_call_stmt_to_callee would not be sufficient (it
> > turns the SSA name into a default-def, a bit like IPA-SRA, but any
> > code dominated by a call to a noreturn is not dangerous when it comes
> > to its side-effects).  So this patch just removes the handling.
> >
> > Bootstrapped and tested on x86_64-linux and ppc64le-linux.  I have also
> > LTO-bootstrapped and LTO-profilebootstrapped the patch on x86_64-linux.
> >
> > OK for master?
> >
> > Thanks,
> >
> > Martin
> >
> >
> > gcc/ChangeLog:
> >
> > 2024-02-07  Martin Jambor  <mjambor@suse.cz>
> >
> > 	PR ipa/113757
> > 	* tree-inline.cc (redirect_all_calls): Remove code adding SSAs to
> > 	id->killed_new_ssa_names.
> >
> > gcc/testsuite/ChangeLog:
> >
> > 2024-02-07  Martin Jambor  <mjambor@suse.cz>
> >
> > 	PR ipa/113757
> > 	* g++.dg/ipa/pr113757.C: New test.
OK,
thanks!
Honza
> > ---
> >  gcc/testsuite/g++.dg/ipa/pr113757.C | 14 ++++++++++++++
> >  gcc/tree-inline.cc                  | 14 ++------------
> >  2 files changed, 16 insertions(+), 12 deletions(-)
> >  create mode 100644 gcc/testsuite/g++.dg/ipa/pr113757.C
> >
> > diff --git a/gcc/testsuite/g++.dg/ipa/pr113757.C b/gcc/testsuite/g++.dg/ipa/pr113757.C
> > new file mode 100644
> > index 00000000000..885d4010a10
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/ipa/pr113757.C
> > @@ -0,0 +1,14 @@
> > +// { dg-do compile }
> > +// { dg-options "-O2 -fPIC" }
> > +// { dg-require-effective-target fpic }
> > +
> > +long size();
> > +struct ll {  virtual int hh();  };
> > +ll  *slice_owner;
> > +int ll::hh() { __builtin_exit(0); }
> > +int nn() {
> > +  if (size())
> > +    return 0;
> > +  return slice_owner->hh();
> > +}
> > +int (*a)() = nn;
> > diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc
> > index 75c10eb7dfc..cac41b4f031 100644
> > --- a/gcc/tree-inline.cc
> > +++ b/gcc/tree-inline.cc
> > @@ -2984,23 +2984,13 @@ redirect_all_calls (copy_body_data * id, basic_block bb)
> >        gimple *stmt = gsi_stmt (si);
> >        if (is_gimple_call (stmt))
> >  	{
> > -	  tree old_lhs = gimple_call_lhs (stmt);
> >  	  struct cgraph_edge *edge = id->dst_node->get_edge (stmt);
> >  	  if (edge)
> >  	    {
> >  	      if (!id->killed_new_ssa_names)
> >  		id->killed_new_ssa_names = new hash_set<tree> (16);
> > -	      gimple *new_stmt
> > -		= cgraph_edge::redirect_call_stmt_to_callee (edge,
> > -		    id->killed_new_ssa_names);
> > -	      if (old_lhs
> > -		  && TREE_CODE (old_lhs) == SSA_NAME
> > -		  && !gimple_call_lhs (new_stmt))
> > -		/* In case of IPA-SRA removing the LHS, the name should have
> > -		   been already added to the hash.  But in case of redirecting
> > -		   to builtin_unreachable it was not and the name still should
> > -		   be pruned from debug statements.  */
> > -		id->killed_new_ssa_names->add (old_lhs);
> > +	      cgraph_edge::redirect_call_stmt_to_callee (edge,
> > +		id->killed_new_ssa_names);
> >  
> >  	      if (stmt == last && id->call_stmt && maybe_clean_eh_stmt (stmt))
> >  		gimple_purge_dead_eh_edges (bb);
> > -- 
> > 2.43.0