Hello,

now that the update_address_taken patch is in, let me re-post the SRA follow-up. With this patch, testcase pr60517.C (attached) has a use of an undefined variable at the time of the uninit pass. Sadly, while this warned with earlier versions of the other patch (when I was inserting default definitions of new variables), it does not anymore because we have TREE_NO_WARNING all over the place. Still, I believe this is a step in the right direction, and it passed bootstrap+testsuite (minus the new testcase). Would it be ok to commit the tree-sra.c change? Then I could close PR60770 (the warning itself is PR60517).

2014-11-03  Marc Glisse  <marc.gli...@inria.fr>

        PR tree-optimization/60770
        * tree-sra.c (clobber_subtree): New function.
        (sra_modify_constructor_assign): Call it.

--
Marc Glisse
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 "" } */
+}
Index: gcc/tree-sra.c
===================================================================
--- gcc/tree-sra.c      (revision 217035)
+++ gcc/tree-sra.c      (working copy)
@@ -2716,20 +2716,51 @@ 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);
 }
 
+/* Clobber all scalar replacements in an access subtree.  ACCESS is the the
+   root of the subtree to be processed.  GSI is the statement iterator used
+   for inserting statements which are added after the current statement if
+   INSERT_AFTER is true or before it otherwise.  */
+
+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);
+    }
+
+  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
@@ -3028,43 +3059,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;
     }
 

Reply via email to