Woops I forgot the patch.
Thanks,
Andrew Pinski
On Fri, Mar 9, 2012 at 11:45 AM, Andrew Pinski
<andrew.pin...@caviumnetworks.com> wrote:
> On Tue, Jan 24, 2012 at 2:50 AM, Richard Guenther
> <richard.guent...@gmail.com> wrote:
>> On Tue, Jan 24, 2012 at 7:34 AM, Andrew Pinski
>> <andrew.pin...@caviumnetworks.com> wrote:
>>> Hi,
>>> Right now PHI-OPT does try to handle the case where we have multiple
>>> PHIs but the other PHIs have the same value for the edges we care
>>> about.
>>> This fixes the issue and allows PHI-OPT to handle a few more cases and
>>> it removes the TODO in the comments.
>>>
>>> OK For 4.8? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
>>>
>>> Thanks,
>>> Andrew Pinski
>>>
>>> ChangeLog:
>>> * tree-ssa-phiopt.c (gimple_phi_singleton_for_edges): New function.
>>
>> The name is confusing I think, because it returns the single non-singleton
>> PHI for the edge pair ... you can avoid choosing a better name by
>> inlining it to its single call site. I'd maybe name it
>> single_non_singleton_phi_for_edges.
>>
>> Otherwise ok.
>
> Here is the updated patch with one small change, value_replacement
> uses single_non_singleton_phi_for_edges now too.
>
> OK still? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
>
>
> Thanks,
> Andrew Pinski
>
>
> ChangeLog:
> * tree-ssa-phiopt.c (single_non_singleton_phi_for_edges): New function.
> (tree_ssa_phiopt_worker): Use single_non_singleton_phi_for_edges.
> (value_replacement): Likewise.
> (empty_block_p): Check also if the PHIs for the block are empty.
>
> testsuite/ChangeLog:
> * gcc.dg/tree-ssa/phi-opt-7.c: New testcase.
>
>
>>
>> Thanks,
>> Richard.
>>
>>> (tree_ssa_phiopt_worker): Use gimple_phi_singleton_for_edges.
>>> (empty_block_p): Check also if the PHIs for the block are empty.
>>>
>>> testsuite/ChangeLog:
>>> * gcc.dg/tree-ssa/phi-opt-7.c: New testcase.
Index: testsuite/gcc.dg/tree-ssa/phi-opt-7.c
===================================================================
--- testsuite/gcc.dg/tree-ssa/phi-opt-7.c (revision 0)
+++ testsuite/gcc.dg/tree-ssa/phi-opt-7.c (revision 0)
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-O1 -fdump-tree-optimized" } */
+
+int g(int,int);
+int f(int t, int c)
+{
+ int d = 0;
+ int e = 0;
+ if (t)
+ {
+ d = t;
+ if (c) e = 1;
+ }
+ else d = 0, e = 0;
+ return g(d,e);
+}
+
+/* There should be one ifs as one of them should be changed into
+ a conditional and the other should be there still. */
+/* { dg-final { scan-tree-dump-times "if" 1 "optimized" } }*/
+/* { dg-final { scan-tree-dump-times "D.\[0-9\]*_\[0-9\]* = c_\[0-9\]*.D. !=
0" 1 "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
+
Index: testsuite/gcc.dg/tree-ssa/phi-opt-8.c
===================================================================
--- testsuite/gcc.dg/tree-ssa/phi-opt-8.c (revision 185132)
+++ testsuite/gcc.dg/tree-ssa/phi-opt-8.c (working copy)
@@ -19,7 +19,7 @@ int f(int t, int c)
but currently is not as PHI-OPT does not reduce the t PHI as we have
two phis. Note this is fixed with
http://gcc.gnu.org/ml/gcc-patches/2012-01/msg01195.html . */
-/* { dg-final { scan-tree-dump-not "if" "phiopt1" { xfail *-*-* } } } */
+/* { dg-final { scan-tree-dump-not "if" "phiopt1" } } */
/* { dg-final { scan-tree-dump "g .t_\[0-9\]*.D.," "optimized" } } */
/* { dg-final { scan-tree-dump-not "PHI" "optimized" } } */
/* { dg-final { cleanup-tree-dump "phiopt1" } } */
Index: tree-ssa-phiopt.c
===================================================================
--- tree-ssa-phiopt.c (revision 185132)
+++ tree-ssa-phiopt.c (working copy)
@@ -193,6 +193,33 @@ tree_ssa_cs_elim (void)
return tree_ssa_phiopt_worker (true);
}
+/* Return the singleton PHI in the SEQ of PHIs for edges E0 and E1. */
+
+static gimple
+single_non_singleton_phi_for_edges (gimple_seq seq, edge e0, edge e1)
+{
+ gimple_stmt_iterator i;
+ gimple phi = NULL;
+ if (gimple_seq_singleton_p (seq))
+ return gsi_stmt (gsi_start (seq));
+ for (i = gsi_start (seq); !gsi_end_p (i); gsi_next (&i))
+ {
+ gimple p = gsi_stmt (i);
+ /* If the PHI arguments are equal then we can skip this PHI. */
+ if (operand_equal_for_phi_arg_p (gimple_phi_arg_def (p, e0->dest_idx),
+ gimple_phi_arg_def (p, e1->dest_idx)))
+ continue;
+
+ /* If we already have a PHI that has the two edge arguments are
+ different, then return it is not a singleton for these PHIs. */
+ if (phi)
+ return NULL;
+
+ phi = p;
+ }
+ return phi;
+}
+
/* For conditional store replacement we need a temporary to
put the old contents of the memory in. */
static tree condstoretemp;
@@ -316,6 +343,7 @@ tree_ssa_phiopt_worker (bool do_store_el
gimple_seq phis = phi_nodes (bb2);
gimple_stmt_iterator gsi;
bool candorest = true;
+
/* Value replacement can work with more than one PHI
so try that first. */
for (gsi = gsi_start (phis); !gsi_end_p (gsi); gsi_next (&gsi))
@@ -333,21 +361,8 @@ tree_ssa_phiopt_worker (bool do_store_el
if (!candorest)
continue;
- /* Check to make sure that there is only one non-virtual PHI node.
- TODO: we could do it with more than one iff the other PHI nodes
- have the same elements for these two edges. */
- phi = NULL;
- for (gsi = gsi_start (phis); !gsi_end_p (gsi); gsi_next (&gsi))
- {
- if (!is_gimple_reg (gimple_phi_result (gsi_stmt (gsi))))
- continue;
- if (phi)
- {
- phi = NULL;
- break;
- }
- phi = gsi_stmt (gsi);
- }
+
+ phi = single_non_singleton_phi_for_edges (phis, e1, e2);
if (!phi)
continue;
@@ -447,6 +462,8 @@ empty_block_p (basic_block bb)
{
/* BB must have no executable statements. */
gimple_stmt_iterator gsi = gsi_after_labels (bb);
+ if (phi_nodes (bb))
+ return false;
if (gsi_end_p (gsi))
return true;
if (is_gimple_debug (gsi_stmt (gsi)))
@@ -736,10 +753,11 @@ value_replacement (basic_block cond_bb,
arg = arg1;
/* If the middle basic block was empty or is defining the
- PHI arguments and this is a singleton phi then we can remove
- the middle basic block. */
+ PHI arguments and this is a single phi where the args are different
+ for the edges e0 and e1 then we can remove the middle basic block. */
if (emtpy_or_with_defined_p
- && gimple_seq_singleton_p (phi_nodes (gimple_bb (phi))))
+ && single_non_singleton_phi_for_edges (phi_nodes (gimple_bb (phi)),
+ e0, e1))
{
replace_phi_edge_with_variable (cond_bb, e1, phi, arg);
/* Note that we optimized this PHI. */
@@ -754,7 +772,8 @@ value_replacement (basic_block cond_bb,
{
fprintf (dump_file, "PHI ");
print_generic_expr (dump_file, gimple_phi_result (phi), 0);
- fprintf (dump_file, " reduced for COND_EXPR in block %d to ",
cond_bb->index);
+ fprintf (dump_file, " reduced for COND_EXPR in block %d to ",
+ cond_bb->index);
print_generic_expr (dump_file, arg, 0);
fprintf (dump_file, ".\n");
}
