| Message ID | CA+=Sn1n8oxxSyAALuth5-LkOvOp2CJFW6Ee9_AvDz3BgJiPcAQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Jan 24, 2012 at 7:34 AM, Andrew Pinski <andrew.pinski@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.
On Tue, Jan 24, 2012 at 2:50 AM, Richard Guenther <richard.guenther@gmail.com> wrote: > On Tue, Jan 24, 2012 at 7:34 AM, Andrew Pinski > <andrew.pinski@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: 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)))