This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix for PR51879 - Missed tail merging with non-const/pure calls
- From: Tom de Vries <Tom_deVries at mentor dot com>
- To: Richard Guenther <richard dot guenther at gmail dot com>
- Cc: Jakub Jelinek <jakub at redhat dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 2 May 2012 16:06:23 +0200
- Subject: Re: [PATCH] Fix for PR51879 - Missed tail merging with non-const/pure calls
- References: <4F1DD0CF.8010808@mentor.com> <CAFiYyc28OogUwtJyouGTLv9VcZ+Z=BsupY2HbJXFq4WZdNfjtg@mail.gmail.com> <4F230B0F.7080107@mentor.com> <4F89268B.6080007@mentor.com> <CAFiYyc1rThWX-0MRpAuZuOhXNpJK-tk_zpMX3oQignM0oDcp=Q@mail.gmail.com> <4F9718E9.5080903@mentor.com> <CAFiYyc1vYsr_aj6wETaj=KeJUnQABNEU5w+NUCpdxrMaf3BurQ@mail.gmail.com> <4F987310.5000603@mentor.com> <CAFiYyc3LD-hs0xPQ5qszbcmyo-NWPN840PeCtCcd5HenDm+EVw@mail.gmail.com> <4F9A3A90.4050608@mentor.com> <CAFiYyc2CHdj6n8LtJbwR5T7E1c5Nv6_M+f7LsYhPYxHPtzKa0A@mail.gmail.com>
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 <tom@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" } } */