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

[Martin Jambor <mjambor at suse dot cz>]

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?

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;
+}
+