On 27/04/12 11:01, Richard Guenther wrote: <SNIP> >>>>> I see you do not handle <SNIP> >>>>> struct S { int i; }; >>>>> struct S foo (void); >>>>> struct S bar (void) >>>>> { >>>>> struct S s1, s2; >>>>> if (...) >>>>> s = foo (); >>>>> else >>>>> s = foo (); >>>>> >>>>> because the calls have a LHS that is not an SSA name. >>>> >>>> Indeed, the gvn patch handles this example conservatively, and >>>> tree-tail-merge >>>> fails to optimize this test-case: >>>> ... >>>> struct S { int i; }; >>>> extern struct S foo (void); >>>> extern int foo2 (void); >>>> struct S s; >>>> int bar (int c) { >>>> int r; >>>> if (c) >>>> { >>>> s = foo (); >>>> r = foo2 (); >>>> } >>>> else >>>> { >>>> s = foo (); >>>> r = foo2 (); >>>> } >>>> return r; >>>> } >>>> ... >>>> >>>> A todo. >>>> <SNIP> >>>> bootstrapped and reg-tested on x86_64 (ada inclusive). >>>> >>>> Is this patch ok, or is the todo required? >>> >>> No, you can followup with that. >>>
Richard, here is the follow-up patch, which adds value numbering of a call for which the lhs is not an SSA_NAME. The only thing I ended up using from the patch in http://gcc.gnu.org/ml/gcc-patches/2012-01/msg01731.html was the idea of using MODIFY_EXPR. I don't include any handling of MODIFY_EXPR in create_component_ref_by_pieces_1 because I don't think it will trigger with PRE. bootstrapped and reg-tested on x86_64. Ok for trunk? Thanks, - Tom 2012-05-02 Tom de Vries <t...@codesourcery.com> * tree-ssa-sccvn.c (copy_reference_ops_from_call) (visit_reference_op_call): Handle case that lhs is not an SSA_NAME. (visit_use): Call visit_reference_op_call for calls with lhs that is not an SSA_NAME. * gcc.dg/pr51879-16.c: New test. * gcc.dg/pr51879-17.c: Same.
Index: gcc/tree-ssa-sccvn.c =================================================================== --- gcc/tree-ssa-sccvn.c (revision 186907) +++ gcc/tree-ssa-sccvn.c (working copy) @@ -942,6 +947,17 @@ copy_reference_ops_from_call (gimple cal { vn_reference_op_s temp; unsigned i; + tree lhs = gimple_call_lhs (call); + + if (lhs && TREE_CODE (lhs) != SSA_NAME) + { + memset (&temp, 0, sizeof (temp)); + temp.opcode = MODIFY_EXPR; + temp.type = TREE_TYPE (lhs); + temp.op0 = lhs; + temp.off = -1; + VEC_safe_push (vn_reference_op_s, heap, *result, &temp); + } /* Copy the type, opcode, function being called and static chain. */ memset (&temp, 0, sizeof (temp)); @@ -2624,6 +2641,9 @@ visit_reference_op_call (tree lhs, gimpl tree vuse = gimple_vuse (stmt); tree vdef = gimple_vdef (stmt); + if (lhs && TREE_CODE (lhs) != SSA_NAME) + lhs = NULL_TREE; + vr1.vuse = vuse ? SSA_VAL (vuse) : NULL_TREE; vr1.operands = valueize_shared_reference_ops_from_call (stmt); vr1.type = gimple_expr_type (stmt); @@ -3410,9 +3432,7 @@ visit_use (tree use) /* If the call has side effects, subsequent calls won't have the same incoming vuse, so it's save to assume equality. */ - || gimple_has_side_effects (stmt)) - && ((lhs && TREE_CODE (lhs) == SSA_NAME) - || (!lhs && gimple_vdef (stmt)))) + || gimple_has_side_effects (stmt))) { changed = visit_reference_op_call (lhs, stmt); } Index: gcc/testsuite/gcc.dg/pr51879-16.c =================================================================== --- /dev/null (new file) +++ gcc/testsuite/gcc.dg/pr51879-16.c (revision 0) @@ -0,0 +1,32 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-pre" } */ + +struct S { + int i; +}; + +extern struct S foo (void); +extern int foo2 (void); + +struct S s; + +int bar (int c) { + int r; + + if (c) + { + s = foo (); + r = foo2 (); + } + else + { + s = foo (); + r = foo2 (); + } + + return r; +} + +/* { dg-final { scan-tree-dump-times "foo \\(" 1 "pre"} } */ +/* { dg-final { scan-tree-dump-times "foo2 \\(" 1 "pre"} } */ +/* { dg-final { cleanup-tree-dump "pre" } } */ Index: gcc/testsuite/gcc.dg/pr51879-17.c =================================================================== --- /dev/null (new file) +++ gcc/testsuite/gcc.dg/pr51879-17.c (revision 0) @@ -0,0 +1,32 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-pre" } */ + +struct S { + int i; +}; + +extern struct S foo (void); +extern int foo2 (void); + +struct S s, s2; + +int bar (int c) { + int r; + + if (c) + { + s = foo (); + r = foo2 (); + } + else + { + s2 = foo (); + r = foo2 (); + } + + return r; +} + +/* { dg-final { scan-tree-dump-times "foo \\(" 2 "pre"} } */ +/* { dg-final { scan-tree-dump-times "foo2 \\(" 2 "pre"} } */ +/* { dg-final { cleanup-tree-dump "pre" } } */