[PATCH, SRA, PR 42154] Don't use access->expr to create references to original aggregates

[Richard Guenther]

On Mon, 23 Nov 2009, Martin Jambor wrote:

> Hi,
> 
> the patch below fixes PR 42154.  The problem was that I assumed that
> access->expr of the group representative could be re-used anywhere to
> create references to the associated part of the original aggregate.
> However, that is not necessarily true since get_ref_base_and_extent is
> capable of identifying array accesses with variable index into an
> array which has only one element as suitable for SRA replacement.  The
> original index might not necessarily be in the given array bounds at
> other places of the flow graph.  And there may be other similar cases.
> 
> This is fixed by replacing the two occurences where it was used in
> this way by appropriate calls to build_ref_for_offset which
> reconstruct all references from the constant offset and thus comes up
> with the correct constant index for array accesses.
> 
> Bootstrapped and tested on x86_64-linux without any problems.  OK for
> trunk?

Ok.

Thanks,
Richard.

> Thanks,
> 
> Martin
> 
> 
> 
> 2009-11-23  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR tree-optimization/42154
> 	* tree-sra.c (struct access): Added comments.
> 	(sra_modify_expr): Build references to the old aggregate with are
> 	instead of reusing access->expr.
> 	(load_assign_lhs_subreplacements): Likewise.
> 
> 	* testsuite/gcc.c-torture/execute/pr42154.c: New test.
> 
> Index: mine/gcc/tree-sra.c
> ===================================================================
> --- mine.orig/gcc/tree-sra.c
> +++ mine/gcc/tree-sra.c
> @@ -125,7 +125,9 @@ struct access
>    HOST_WIDE_INT size;
>    tree base;
>  
> -  /* Expression.  */
> +  /* Expression.  It is context dependant so do not use it to create new
> +     expressions to acces the original aggregate.  See PR 42154 for a
> +     testcase.  */
>    tree expr;
>    /* Type.  */
>    tree type;
> @@ -2113,10 +2115,17 @@ sra_modify_expr (tree *expr, gimple_stmt
>  	 gcc.c-torture/compile/20011217-1.c.  */
>        if (!is_gimple_reg_type (type))
>  	{
> -	  gimple stmt;
> +	  tree ref = access->base;
> +	  bool ok;
> +
> +	  ok = build_ref_for_offset (&ref, TREE_TYPE (ref),
> +				     access->offset, access->type, false);
> +	  gcc_assert (ok);
> +
>  	  if (write)
>  	    {
> -	      tree ref = unshare_expr (access->expr);
> +	      gimple stmt;
> +
>  	      if (access->grp_partial_lhs)
>  		ref = force_gimple_operand_gsi (gsi, ref, true, NULL_TREE,
>  						 false, GSI_NEW_STMT);
> @@ -2125,10 +2134,12 @@ sra_modify_expr (tree *expr, gimple_stmt
>  	    }
>  	  else
>  	    {
> +	      gimple stmt;
> +
>  	      if (access->grp_partial_lhs)
>  		repl = force_gimple_operand_gsi (gsi, repl, true, NULL_TREE,
>  						 true, GSI_SAME_STMT);
> -	      stmt = gimple_build_assign (unshare_expr (access->expr), repl);
> +	      stmt = gimple_build_assign (ref, repl);
>  	      gsi_insert_before (gsi, stmt, GSI_SAME_STMT);
>  	    }
>  	}
> @@ -2227,8 +2238,6 @@ load_assign_lhs_subreplacements (struct
>  	    }
>  	  else
>  	    {
> -	      bool repl_found;
> -
>  	      /* No suitable access on the right hand side, need to load from
>  		 the aggregate.  See if we have to update it first... */
>  	      if (*refreshed == SRA_UDH_NONE)
> @@ -2236,9 +2245,19 @@ load_assign_lhs_subreplacements (struct
>  								  lhs, old_gsi);
>  
>  	      if (*refreshed == SRA_UDH_LEFT)
> -		rhs = unshare_expr (lacc->expr);
> +		{
> +		  bool repl_found;
> +
> +		  rhs = lacc->base;
> +		  repl_found = build_ref_for_offset (&rhs, TREE_TYPE (rhs),
> +						     lacc->offset, lacc->type,
> +						     false);
> +		  gcc_assert (repl_found);
> +		}
>  	      else
>  		{
> +		  bool repl_found;
> +
>  		  rhs = top_racc->base;
>  		  repl_found = build_ref_for_offset (&rhs,
>  						     TREE_TYPE (top_racc->base),
> Index: mine/gcc/testsuite/gcc.c-torture/execute/pr42154.c
> ===================================================================
> --- /dev/null
> +++ mine/gcc/testsuite/gcc.c-torture/execute/pr42154.c
> @@ -0,0 +1,18 @@
> +struct A { char x[1]; };
> +extern void abort (void);
> +void __attribute__((noinline,noclone))
> +foo (struct A a)
> +{
> +  if (a.x[0] != 'a')
> +    abort ();
> +}
> +int main ()
> +{
> +  struct A a;
> +  int i;
> +  for (i = 0; i < 1; ++i)
> +    a.x[i] = 'a';
> +  foo (a);
> +  return 0;
> +}
> +
> 
> 

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