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.gli...@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 GlisseIndex: 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 "" } */
+}
