Re: [PATCH] Fix IPA-SRA with same_body aliases (PR tree-optimization/42508)

[Richard Guenther <rguenther at suse dot de>]

On Mon, 4 Jan 2010, Jakub Jelinek wrote:

> Hi!
> 
> As IPA-SRA modifies in place function prototypes (and callers), instead of
> creating clones, we need to also modify same body aliases.  The following
> patch does that.  The cgraphunit.c change is just something I've noticed
> while debugging - if an inline clone is created and e->callee is that inline
> clone of a node with same_body alias and we call the alias,
> cgraph_materialize_all_clones would unnecessarily change the caller from
> the alias to the other decl (from D1 to D2 in this case).
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk?

Huh.  Are you sure thunks will not get confused with this?  After
all the analysis doesn't see what the thunks do and the thunks are
not clones either (and they'll emit a call to the original
function in a way that probably doesn't work anymore).

So - why not simply refuse to early-IPA-SRA (that's the only one
that doesnt' work with clones?) functions that have same-body
(or thunk) aliases?

Thanks,
Richard.

> 2010-01-04  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/42508
> 	* tree-sra.c (convert_callers): Check for recursive call
> 	by comparing cgraph nodes instead of decls.
> 	(modify_function): Call ipa_modify_formal_parameters also
> 	on all same_body aliases.
> 
> 	* g++.dg/opt/pr42508.C: New test.
> 
> 	* cgraphunit.c (cgraph_materialize_all_clones): Compare
> 	cgraph nodes when checking for same_body aliases.
> 
> --- gcc/tree-sra.c.jj	2010-01-04 10:46:33.000000000 +0100
> +++ gcc/tree-sra.c	2010-01-04 14:56:24.000000000 +0100
> @@ -3814,8 +3814,11 @@ convert_callers (struct cgraph_node *nod
>        for (gsi = gsi_start_bb (this_block); !gsi_end_p (gsi); gsi_next (&gsi))
>          {
>  	  gimple stmt = gsi_stmt (gsi);
> -	  if (gimple_code (stmt) == GIMPLE_CALL
> -	      && gimple_call_fndecl (stmt) == node->decl)
> +	  tree call_fndecl;
> +	  if (gimple_code (stmt) != GIMPLE_CALL)
> +	    continue;
> +	  call_fndecl = gimple_call_fndecl (stmt);
> +	  if (call_fndecl && cgraph_get_node (call_fndecl) == node)
>  	    {
>  	      if (dump_file)
>  		fprintf (dump_file, "Adjusting recursive call");
> @@ -3833,6 +3836,11 @@ convert_callers (struct cgraph_node *nod
>  static void
>  modify_function (struct cgraph_node *node, ipa_parm_adjustment_vec adjustments)
>  {
> +  struct cgraph_node *alias;
> +  for (alias = node->same_body; alias; alias = alias->next)
> +    ipa_modify_formal_parameters (alias->decl, adjustments, "ISRA");
> +  /* current_function_decl must be handled last, after same_body aliases,
> +     as following functions will use what it computed.  */
>    ipa_modify_formal_parameters (current_function_decl, adjustments, "ISRA");
>    scan_function (sra_ipa_modify_expr, sra_ipa_modify_assign,
>  		 replace_removed_params_ssa_names, false, adjustments);
> --- gcc/cgraphunit.c.jj	2009-12-10 22:54:27.000000000 +0100
> +++ gcc/cgraphunit.c	2010-01-04 15:01:00.000000000 +0100
> @@ -2323,20 +2323,9 @@ cgraph_materialize_all_clones (void)
>  		gimple new_stmt;
>  		gimple_stmt_iterator gsi;
>  
> -		if (e->callee->same_body)
> -		  {
> -		    struct cgraph_node *alias;
> -
> -		    for (alias = e->callee->same_body;
> -			 alias;
> -			 alias = alias->next)
> -		      if (decl == alias->decl)
> -			break;
> -		    /* Don't update call from same body alias to the real
> -		       function.  */
> -		    if (alias)
> -		      continue;
> -		  }
> +		if (cgraph_get_node (decl) == cgraph_get_node (e->callee->decl))
> +		  /* Don't update call from same body alias to the real function.  */
> +		  continue;
>  
>  		if (cgraph_dump_file)
>  		  {
> --- gcc/testsuite/g++.dg/opt/pr42508.C.jj	2010-01-04 15:00:43.000000000 +0100
> +++ gcc/testsuite/g++.dg/opt/pr42508.C	2010-01-04 15:00:25.000000000 +0100
> @@ -0,0 +1,33 @@
> +// PR tree-optimization/42508
> +// { dg-do run }
> +// { dg-options "-O1 -fipa-sra" }
> +
> +extern "C" void abort ();
> +
> +int v[10], vidx;
> +
> +struct A
> +{
> +  A *prev;
> +  int i;
> +  ~A()
> +  {
> +    v[vidx++] = i;
> +    delete prev;
> +  }
> +};
> +
> +int
> +main ()
> +{
> +  A *a1 = new A ();
> +  A *a2 = new A ();
> +  a1->prev = 0;
> +  a1->i = 1;
> +  a2->prev = a1;
> +  a2->i = 2;
> +  delete a2;
> +  if (vidx != 2 || v[0] != 2 || v[1] != 1)
> +    abort ();
> +  return 0;
> +}
> 
> 	Jakub
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex