Re: [PATCH] Improve PHI-OPT when there are multiple phis
On Sat, Mar 10, 2012 at 8:55 AM, Andrew Pinski andrew.pin...@caviumnetworks.com wrote: Woops I forgot the patch. Ok. Thanks, Richard. 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.
Re: [PATCH] Improve PHI-OPT when there are multiple phis
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.
Re: [PATCH] Improve PHI-OPT when there are multiple phis
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
Re: [PATCH] Improve PHI-OPT when there are multiple phis
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. 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.
[PATCH] Improve PHI-OPT when there are multiple phis
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.
(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: tree-ssa-phiopt.c
===
--- tree-ssa-phiopt.c (revision 183458)
+++ tree-ssa-phiopt.c (working copy)
@@ -192,6 +192,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
+gimple_phi_singleton_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;
@@ -313,23 +340,9 @@ tree_ssa_phiopt_worker (bool do_store_el
else
{
gimple_seq phis = phi_nodes (bb2);
- gimple_stmt_iterator gsi;
+ phi = gimple_phi_singleton_for_edges (phis, e1, e2);
- /* 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);
- }
+ /* Check to make sure that there is only one PHI node. */
if (!phi)
continue;
@@ -431,6 +444,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)))
