SRA: don't drop clobbers

[Marc Glisse <marc dot glisse at inria dot fr>]

Hello,

with this patch on top of
https://gcc.gnu.org/ml/gcc-patches/2014-06/msg02315.html
we finally warn for the testcase of PR 60517.

The new function is copied from init_subtree_with_zero right above. I 
guess it might be possible to merge them into a single function, if 
desired. I don't understand the debug stuff, but hopefully by keeping the 
functions similar enough it shouldn't be too broken.

When we see a clobber during scalarization, instead of dropping it, we add 
a clobber to the new variable, which the previous patch turns into an 
SSA_NAME with a default def. Then either we reach uninit and warn, or the 
variable appears in a PHI and CCP optimizes.

Bootstrap+testsuite (all,obj-c++,ada,go) on x86_64-linux-gnu.


2014-07-01  Marc Glisse  <marc.glisse@inria.fr>

        PR c++/60517
        PR tree-optimization/60770
gcc/
        * tree-sra.c (clobber_subtree): New function.
	(sra_modify_constructor_assign): Call it.
gcc/testsuite/
        * g++.dg/tree-ssa/pr60517.C: New file.

--
Marc Glisse

Index: gcc/tree-sra.c
===================================================================
--- gcc/tree-sra.c	(revision 212126)
+++ gcc/tree-sra.c	(working copy)
@@ -2727,20 +2727,58 @@ init_subtree_with_zero (struct access *a
       if (insert_after)
 	gsi_insert_after (gsi, ds, GSI_NEW_STMT);
       else
 	gsi_insert_before (gsi, ds, GSI_SAME_STMT);
     }
 
   for (child = access->first_child; child; child = child->next_sibling)
     init_subtree_with_zero (child, gsi, insert_after, loc);
 }
 
+static void
+clobber_subtree (struct access *access, gimple_stmt_iterator *gsi,
+		bool insert_after, location_t loc)
+
+{
+  struct access *child;
+
+  if (access->grp_to_be_replaced)
+    {
+      tree rep = get_access_replacement (access);
+      tree clobber = build_constructor (access->type, NULL);
+      TREE_THIS_VOLATILE (clobber) = 1;
+      gimple stmt = gimple_build_assign (rep, clobber);
+
+      if (insert_after)
+	gsi_insert_after (gsi, stmt, GSI_NEW_STMT);
+      else
+	gsi_insert_before (gsi, stmt, GSI_SAME_STMT);
+      update_stmt (stmt);
+      gimple_set_location (stmt, loc);
+    }
+  else if (access->grp_to_be_debug_replaced)
+    {
+      tree rep = get_access_replacement (access);
+      tree clobber = build_constructor (access->type, NULL);
+      TREE_THIS_VOLATILE (clobber) = 1;
+      gimple ds = gimple_build_debug_bind (rep, clobber, gsi_stmt (*gsi));
+
+      if (insert_after)
+	gsi_insert_after (gsi, ds, GSI_NEW_STMT);
+      else
+	gsi_insert_before (gsi, ds, GSI_SAME_STMT);
+    }
+
+  for (child = access->first_child; child; child = child->next_sibling)
+    clobber_subtree (child, gsi, insert_after, loc);
+}
+
 /* Search for an access representative for the given expression EXPR and
    return it or NULL if it cannot be found.  */
 
 static struct access *
 get_access_for_expr (tree expr)
 {
   HOST_WIDE_INT offset, size, max_size;
   tree base;
 
   /* FIXME: This should not be necessary but Ada produces V_C_Es with a type of
@@ -3039,43 +3077,41 @@ enum assignment_mod_result { SRA_AM_NONE
 			     SRA_AM_REMOVED };  /* stmt eliminated */
 
 /* Modify assignments with a CONSTRUCTOR on their RHS.  STMT contains a pointer
    to the assignment and GSI is the statement iterator pointing at it.  Returns
    the same values as sra_modify_assign.  */
 
 static enum assignment_mod_result
 sra_modify_constructor_assign (gimple *stmt, gimple_stmt_iterator *gsi)
 {
   tree lhs = gimple_assign_lhs (*stmt);
-  struct access *acc;
-  location_t loc;
-
-  acc = get_access_for_expr (lhs);
+  struct access *acc = get_access_for_expr (lhs);
   if (!acc)
     return SRA_AM_NONE;
+  location_t loc = gimple_location (*stmt);
 
   if (gimple_clobber_p (*stmt))
     {
-      /* Remove clobbers of fully scalarized variables, otherwise
-	 do nothing.  */
+      /* Clobber the replacement variable.  */
+      clobber_subtree (acc, gsi, !acc->grp_covered, loc);
+      /* Remove clobbers of fully scalarized variables, they are dead.  */
       if (acc->grp_covered)
 	{
 	  unlink_stmt_vdef (*stmt);
 	  gsi_remove (gsi, true);
 	  release_defs (*stmt);
 	  return SRA_AM_REMOVED;
 	}
       else
-	return SRA_AM_NONE;
+	return SRA_AM_MODIFIED;
     }
 
-  loc = gimple_location (*stmt);
   if (vec_safe_length (CONSTRUCTOR_ELTS (gimple_assign_rhs1 (*stmt))) > 0)
     {
       /* I have never seen this code path trigger but if it can happen the
 	 following should handle it gracefully.  */
       if (access_has_children_p (acc))
 	generate_subtree_copies (acc->first_child, lhs, acc->offset, 0, 0, gsi,
 				 true, true, loc);
       return SRA_AM_MODIFIED;
     }
 
Index: gcc/testsuite/g++.dg/tree-ssa/pr60517.C
===================================================================
--- gcc/testsuite/g++.dg/tree-ssa/pr60517.C	(revision 0)
+++ gcc/testsuite/g++.dg/tree-ssa/pr60517.C	(working copy)
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-O -Wall" } */
+
+struct B {
+    double x[2];
+};
+struct A {
+    B b;
+    B getB() { return b; }
+};
+
+double foo(A a) {
+    double * x = &(a.getB().x[0]);
+    return x[0]; /* { dg-warning "" } */
+}