This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR40389
On Mon, 15 Jun 2009, Richard Guenther wrote:
> On Sun, 14 Jun 2009, Richard Guenther wrote:
>
> > On Sat, 13 Jun 2009, Richard Guenther wrote:
> >
> > > On Sat, 13 Jun 2009, Richard Guenther wrote:
> > >
> > > >
> > > > Thix fixes PR40389 - we have to assume that the return value has
> > > > its address taken when NRV is in effect (this may not be a 100%
> > > > complete patch, as we probably also have to assume that it escaped).
> > > >
> > > > At least it fixes the miscompile.
> > > >
> > > > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
> > >
> > > Actually not. When adding the PTA fix I see missed optimizations as
> > > the C++ FE unconditionally sets TREE_ADDRESSABLE on the LHS. If
> >
> > Simple oversight from me.
> >
> > > I understand correctly the issue only appears if a copy was elided.
> > >
> > > I am currently bootstrapping the following more constrained fix.
> >
> > I'm installing the following, bootstrapped and tested on
> > x86_64-unknown-linux-gnu.
>
> This is a variant for the 4.4 branch. If we can settle on
> testing TREE_ADDRESSABLE on the object, not the type (or if somebody
> can explain me what TREE_ADDRESSABLE on types semantically means
> for the middle-end and how this relates to the failure mode).
After discussion I adjusted the trunk as follows and backported the
patch to 4.4 and 4.3 after bootstrapping & testing on
x86_64-unknown-linux-gnu.
Richard.
2009-06-17 Richard Guenther <rguenther@suse.de>
PR tree-optimization/40389
* tree-ssa-structalias.c (handle_rhs_call): Restrict NRV case
to addressable types.
* gimple.c (walk_stmt_load_store_addr_ops): Likewise.
Index: gcc/tree-ssa-structalias.c
===================================================================
*** gcc/tree-ssa-structalias.c (revision 148534)
--- gcc/tree-ssa-structalias.c (working copy)
*************** handle_rhs_call (gimple stmt, VEC(ce_s,
*** 3362,3368 ****
/* And if we applied NRV the address of the return slot escapes as well. */
if (gimple_call_return_slot_opt_p (stmt)
&& gimple_call_lhs (stmt) != NULL_TREE
! && TREE_ADDRESSABLE (gimple_call_lhs (stmt)))
{
VEC(ce_s, heap) *tmpc = NULL;
struct constraint_expr lhsc, *c;
--- 3362,3368 ----
/* And if we applied NRV the address of the return slot escapes as well. */
if (gimple_call_return_slot_opt_p (stmt)
&& gimple_call_lhs (stmt) != NULL_TREE
! && TREE_ADDRESSABLE (TREE_TYPE (gimple_call_lhs (stmt))))
{
VEC(ce_s, heap) *tmpc = NULL;
struct constraint_expr lhsc, *c;
Index: gcc/gimple.c
===================================================================
*** gcc/gimple.c (revision 148533)
--- gcc/gimple.c (working copy)
*************** walk_stmt_load_store_addr_ops (gimple st
*** 3267,3273 ****
if (visit_addr
&& gimple_call_return_slot_opt_p (stmt)
&& gimple_call_lhs (stmt) != NULL_TREE
! && TREE_ADDRESSABLE (gimple_call_lhs (stmt)))
ret |= visit_addr (stmt, gimple_call_lhs (stmt), data);
}
else if (gimple_code (stmt) == GIMPLE_ASM)
--- 3267,3273 ----
if (visit_addr
&& gimple_call_return_slot_opt_p (stmt)
&& gimple_call_lhs (stmt) != NULL_TREE
! && TREE_ADDRESSABLE (TREE_TYPE (gimple_call_lhs (stmt))))
ret |= visit_addr (stmt, gimple_call_lhs (stmt), data);
}
else if (gimple_code (stmt) == GIMPLE_ASM)
2009-06-15 Richard Guenther <rguenther@suse.de>
PR middle-end/40389
* tree-ssa-operands.c (parse_ssa_operands): Add NRV results
to the addresses taken bitmap.
* g++.dg/torture/pr40389.C: New testcase.
Index: gcc/tree-ssa-operands.c
===================================================================
*** gcc/tree-ssa-operands.c (revision 148490)
--- gcc/tree-ssa-operands.c (working copy)
*************** parse_ssa_operands (gimple stmt)
*** 2099,2104 ****
--- 2099,2111 ----
/* Add call-clobbered operands, if needed. */
if (code == GIMPLE_CALL)
maybe_add_call_clobbered_vops (stmt);
+
+ /* Make sure the return value is addressable in case of NRV. */
+ if (code == GIMPLE_CALL
+ && gimple_call_lhs (stmt) != NULL_TREE
+ && gimple_call_return_slot_opt_p (stmt)
+ && TREE_ADDRESSABLE (TREE_TYPE (gimple_call_lhs (stmt))))
+ gimple_add_to_addresses_taken (stmt, gimple_call_lhs (stmt));
}
}
Index: gcc/testsuite/g++.dg/torture/pr40389.C
===================================================================
*** gcc/testsuite/g++.dg/torture/pr40389.C (revision 0)
--- gcc/testsuite/g++.dg/torture/pr40389.C (revision 0)
***************
*** 0 ****
--- 1,84 ----
+ /* { dg-do run } */
+
+ template <typename V> struct S
+ {
+ V *f, *l;
+ __attribute__ ((noinline)) S (void) { f = 0, l = 0; }
+ void foo (V *x)
+ {
+ if (x->p != 0)
+ x->p->n = x->n;
+ else
+ f = x->n;
+ if (x->n != 0)
+ x->n->p = x->p;
+ else
+ l = x->p;
+ }
+ __attribute__ ((noinline)) void bar (V *x)
+ {
+ x->n = 0;
+ x->p = l;
+ if (l != 0)
+ l->n = x;
+ else
+ f = x;
+ l = x;
+ }
+ };
+
+ struct H;
+
+ struct A
+ {
+ S <H> k;
+ };
+
+ struct H
+ {
+ A *a;
+ H *p, *n;
+ __attribute__ ((noinline)) H (void) { p = 0, n = 0, a = 0; }
+ __attribute__ ((noinline)) H (A *b) : a (b)
+ {
+ p = 0;
+ n = 0;
+ if (a != 0)
+ a->k.bar (this);
+ }
+ __attribute__ ((noinline)) H (const H &h) : a (h.a)
+ {
+ p = 0;
+ n = 0;
+ if (a != 0)
+ a->k.bar (this);
+ }
+ ~H (void) { if (a != 0) a->k.foo (this); }
+ H &operator= (const H &o)
+ {
+ if (a != 0 || &o == this)
+ __builtin_abort ();
+ a = o.a;
+ if (a != 0)
+ a->k.bar (this);
+ return *this;
+ }
+ };
+
+ __attribute__ ((noinline))
+ H baz (void)
+ {
+ return H (new A);
+ }
+
+ H g;
+
+ int
+ main (void)
+ {
+ g = baz ();
+ if (g.a->k.f != &g)
+ __builtin_abort ();
+ return 0;
+ }
+
2009-06-16 Richard Guenther <rguenther@suse.de>
PR middle-end/40389
* tree-ssa-operands.c (get_modify_stmt_operands): Add NRV
results to the addresses taken bitmap.
* g++.dg/torture/pr40389.C: New testcase.
Index: gcc/tree-ssa-operands.c
===================================================================
--- gcc/tree-ssa-operands.c (revision 147864)
+++ gcc/tree-ssa-operands.c (working copy)
@@ -2060,6 +2060,17 @@ get_modify_stmt_operands (tree stmt, tre
We used to distinguish between preserving and killing definitions.
We always emit preserving definitions now. */
get_expr_operands (stmt, &GIMPLE_STMT_OPERAND (expr, 0), opf_def);
+
+ /* Make sure the return value is addressable in case of NRV. */
+ if (TREE_CODE (GIMPLE_STMT_OPERAND (expr, 1)) == CALL_EXPR
+ && CALL_EXPR_RETURN_SLOT_OPT (GIMPLE_STMT_OPERAND (expr, 1))
+ && TREE_ADDRESSABLE (TREE_TYPE (GIMPLE_STMT_OPERAND (expr, 0))))
+ {
+ tree t = get_base_address (GIMPLE_STMT_OPERAND (expr, 0));
+ stmt_ann_t s_ann = stmt_ann (stmt);
+ if (t && DECL_P (t) && s_ann)
+ add_to_addressable_set (t, &s_ann->addresses_taken);
+ }
}
Index: gcc/testsuite/g++.dg/torture/pr40389.C
===================================================================
--- gcc/testsuite/g++.dg/torture/pr40389.C (revision 0)
+++ gcc/testsuite/g++.dg/torture/pr40389.C (revision 0)
@@ -0,0 +1,84 @@
+/* { dg-do run } */
+
+template <typename V> struct S
+{
+ V *f, *l;
+ __attribute__ ((noinline)) S (void) { f = 0, l = 0; }
+ void foo (V *x)
+ {
+ if (x->p != 0)
+ x->p->n = x->n;
+ else
+ f = x->n;
+ if (x->n != 0)
+ x->n->p = x->p;
+ else
+ l = x->p;
+ }
+ __attribute__ ((noinline)) void bar (V *x)
+ {
+ x->n = 0;
+ x->p = l;
+ if (l != 0)
+ l->n = x;
+ else
+ f = x;
+ l = x;
+ }
+};
+
+struct H;
+
+struct A
+{
+ S <H> k;
+};
+
+struct H
+{
+ A *a;
+ H *p, *n;
+ __attribute__ ((noinline)) H (void) { p = 0, n = 0, a = 0; }
+ __attribute__ ((noinline)) H (A *b) : a (b)
+ {
+ p = 0;
+ n = 0;
+ if (a != 0)
+ a->k.bar (this);
+ }
+ __attribute__ ((noinline)) H (const H &h) : a (h.a)
+ {
+ p = 0;
+ n = 0;
+ if (a != 0)
+ a->k.bar (this);
+ }
+ ~H (void) { if (a != 0) a->k.foo (this); }
+ H &operator= (const H &o)
+ {
+ if (a != 0 || &o == this)
+ __builtin_abort ();
+ a = o.a;
+ if (a != 0)
+ a->k.bar (this);
+ return *this;
+ }
+};
+
+__attribute__ ((noinline))
+H baz (void)
+{
+ return H (new A);
+}
+
+H g;
+
+int
+main (void)
+{
+ g = baz ();
+ if (g.a->k.f != &g)
+ __builtin_abort ();
+ return 0;
+}
+