| Message ID | 20240523205259.3531001-1-quic_apinski@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Use simple_dce_from_worklist in phiprop | expand |
On Thu, May 23, 2024 at 10:55 PM Andrew Pinski <quic_apinski@quicinc.com> wrote: > > I noticed that phiprop leaves around phi nodes which > defines a ssa name which is unused. This just adds a > bitmap to mark those ssa names and then calls > simple_dce_from_worklist at the very end to remove > those phi nodes and all of the dependencies if there > was any. This might allow us to optimize something earlier > due to the removal of the phi which was taking the address > of the variables. > > Bootstrapped and tested on x86_64-linux-gnu with no regressions. OK > gcc/ChangeLog: > > * tree-ssa-phiprop.cc (phiprop_insert_phi): Add > dce_ssa_names argument. Add the phi's result to it. > (propagate_with_phi): Add dce_ssa_names argument. > Update call to phiprop_insert_phi. > (pass_phiprop::execute): Update call to propagate_with_phi. > Call simple_dce_from_worklist if there was a change. > > Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com> > --- > gcc/tree-ssa-phiprop.cc | 28 ++++++++++++++++++---------- > 1 file changed, 18 insertions(+), 10 deletions(-) > > diff --git a/gcc/tree-ssa-phiprop.cc b/gcc/tree-ssa-phiprop.cc > index 041521ef106..2a1cdae46d2 100644 > --- a/gcc/tree-ssa-phiprop.cc > +++ b/gcc/tree-ssa-phiprop.cc > @@ -34,6 +34,7 @@ along with GCC; see the file COPYING3. If not see > #include "stor-layout.h" > #include "tree-ssa-loop.h" > #include "tree-cfg.h" > +#include "tree-ssa-dce.h" > > /* This pass propagates indirect loads through the PHI node for its > address to make the load source possibly non-addressable and to > @@ -132,12 +133,15 @@ phivn_valid_p (struct phiprop_d *phivn, tree name, basic_block bb) > > static tree > phiprop_insert_phi (basic_block bb, gphi *phi, gimple *use_stmt, > - struct phiprop_d *phivn, size_t n) > + struct phiprop_d *phivn, size_t n, > + bitmap dce_ssa_names) > { > tree res; > gphi *new_phi = NULL; > edge_iterator ei; > edge e; > + tree phi_result = PHI_RESULT (phi); > + bitmap_set_bit (dce_ssa_names, SSA_NAME_VERSION (phi_result)); > > gcc_assert (is_gimple_assign (use_stmt) > && gimple_assign_rhs_code (use_stmt) == MEM_REF); > @@ -276,7 +280,7 @@ chk_uses (tree, tree *idx, void *data) > > static bool > propagate_with_phi (basic_block bb, gphi *phi, struct phiprop_d *phivn, > - size_t n) > + size_t n, bitmap dce_ssa_names) > { > tree ptr = PHI_RESULT (phi); > gimple *use_stmt; > @@ -420,9 +424,10 @@ propagate_with_phi (basic_block bb, gphi *phi, struct phiprop_d *phivn, > goto next; > } > > - phiprop_insert_phi (bb, phi, use_stmt, phivn, n); > + phiprop_insert_phi (bb, phi, use_stmt, phivn, n, dce_ssa_names); > > - /* Remove old stmt. The phi is taken care of by DCE. */ > + /* Remove old stmt. The phi and all of maybe its depedencies > + will be removed later via simple_dce_from_worklist. */ > gsi = gsi_for_stmt (use_stmt); > /* Unlinking the VDEF here is fine as we are sure that we process > stmts in execution order due to aggregate copies having VDEFs > @@ -442,16 +447,15 @@ propagate_with_phi (basic_block bb, gphi *phi, struct phiprop_d *phivn, > is the first load transformation. */ > else if (!phi_inserted) > { > - res = phiprop_insert_phi (bb, phi, use_stmt, phivn, n); > + res = phiprop_insert_phi (bb, phi, use_stmt, phivn, n, dce_ssa_names); > type = TREE_TYPE (res); > > /* Remember the value we created for *ptr. */ > phivn[SSA_NAME_VERSION (ptr)].value = res; > phivn[SSA_NAME_VERSION (ptr)].vuse = vuse; > > - /* Remove old stmt. The phi is taken care of by DCE, if we > - want to delete it here we also have to delete all intermediate > - copies. */ > + /* Remove old stmt. The phi and all of maybe its depedencies > + will be removed later via simple_dce_from_worklist. */ > gsi = gsi_for_stmt (use_stmt); > gsi_remove (&gsi, true); > > @@ -514,6 +518,7 @@ pass_phiprop::execute (function *fun) > gphi_iterator gsi; > unsigned i; > size_t n; > + auto_bitmap dce_ssa_names; > > calculate_dominance_info (CDI_DOMINATORS); > > @@ -531,11 +536,14 @@ pass_phiprop::execute (function *fun) > if (bb_has_abnormal_pred (bb)) > continue; > for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); gsi_next (&gsi)) > - did_something |= propagate_with_phi (bb, gsi.phi (), phivn, n); > + did_something |= propagate_with_phi (bb, gsi.phi (), phivn, n, dce_ssa_names); > } > > if (did_something) > - gsi_commit_edge_inserts (); > + { > + gsi_commit_edge_inserts (); > + simple_dce_from_worklist (dce_ssa_names); > + } > > free (phivn); > > -- > 2.43.0 >
diff --git a/gcc/tree-ssa-phiprop.cc b/gcc/tree-ssa-phiprop.cc index 041521ef106..2a1cdae46d2 100644 --- a/gcc/tree-ssa-phiprop.cc +++ b/gcc/tree-ssa-phiprop.cc @@ -34,6 +34,7 @@ along with GCC; see the file COPYING3. If not see #include "stor-layout.h" #include "tree-ssa-loop.h" #include "tree-cfg.h" +#include "tree-ssa-dce.h" /* This pass propagates indirect loads through the PHI node for its address to make the load source possibly non-addressable and to @@ -132,12 +133,15 @@ phivn_valid_p (struct phiprop_d *phivn, tree name, basic_block bb) static tree phiprop_insert_phi (basic_block bb, gphi *phi, gimple *use_stmt, - struct phiprop_d *phivn, size_t n) + struct phiprop_d *phivn, size_t n, + bitmap dce_ssa_names) { tree res; gphi *new_phi = NULL; edge_iterator ei; edge e; + tree phi_result = PHI_RESULT (phi); + bitmap_set_bit (dce_ssa_names, SSA_NAME_VERSION (phi_result)); gcc_assert (is_gimple_assign (use_stmt) && gimple_assign_rhs_code (use_stmt) == MEM_REF); @@ -276,7 +280,7 @@ chk_uses (tree, tree *idx, void *data) static bool propagate_with_phi (basic_block bb, gphi *phi, struct phiprop_d *phivn, - size_t n) + size_t n, bitmap dce_ssa_names) { tree ptr = PHI_RESULT (phi); gimple *use_stmt; @@ -420,9 +424,10 @@ propagate_with_phi (basic_block bb, gphi *phi, struct phiprop_d *phivn, goto next; } - phiprop_insert_phi (bb, phi, use_stmt, phivn, n); + phiprop_insert_phi (bb, phi, use_stmt, phivn, n, dce_ssa_names); - /* Remove old stmt. The phi is taken care of by DCE. */ + /* Remove old stmt. The phi and all of maybe its depedencies + will be removed later via simple_dce_from_worklist. */ gsi = gsi_for_stmt (use_stmt); /* Unlinking the VDEF here is fine as we are sure that we process stmts in execution order due to aggregate copies having VDEFs @@ -442,16 +447,15 @@ propagate_with_phi (basic_block bb, gphi *phi, struct phiprop_d *phivn, is the first load transformation. */ else if (!phi_inserted) { - res = phiprop_insert_phi (bb, phi, use_stmt, phivn, n); + res = phiprop_insert_phi (bb, phi, use_stmt, phivn, n, dce_ssa_names); type = TREE_TYPE (res); /* Remember the value we created for *ptr. */ phivn[SSA_NAME_VERSION (ptr)].value = res; phivn[SSA_NAME_VERSION (ptr)].vuse = vuse; - /* Remove old stmt. The phi is taken care of by DCE, if we - want to delete it here we also have to delete all intermediate - copies. */ + /* Remove old stmt. The phi and all of maybe its depedencies + will be removed later via simple_dce_from_worklist. */ gsi = gsi_for_stmt (use_stmt); gsi_remove (&gsi, true); @@ -514,6 +518,7 @@ pass_phiprop::execute (function *fun) gphi_iterator gsi; unsigned i; size_t n; + auto_bitmap dce_ssa_names; calculate_dominance_info (CDI_DOMINATORS); @@ -531,11 +536,14 @@ pass_phiprop::execute (function *fun) if (bb_has_abnormal_pred (bb)) continue; for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); gsi_next (&gsi)) - did_something |= propagate_with_phi (bb, gsi.phi (), phivn, n); + did_something |= propagate_with_phi (bb, gsi.phi (), phivn, n, dce_ssa_names); } if (did_something) - gsi_commit_edge_inserts (); + { + gsi_commit_edge_inserts (); + simple_dce_from_worklist (dce_ssa_names); + } free (phivn);
I noticed that phiprop leaves around phi nodes which defines a ssa name which is unused. This just adds a bitmap to mark those ssa names and then calls simple_dce_from_worklist at the very end to remove those phi nodes and all of the dependencies if there was any. This might allow us to optimize something earlier due to the removal of the phi which was taking the address of the variables. Bootstrapped and tested on x86_64-linux-gnu with no regressions. gcc/ChangeLog: * tree-ssa-phiprop.cc (phiprop_insert_phi): Add dce_ssa_names argument. Add the phi's result to it. (propagate_with_phi): Add dce_ssa_names argument. Update call to phiprop_insert_phi. (pass_phiprop::execute): Update call to propagate_with_phi. Call simple_dce_from_worklist if there was a change. Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com> --- gcc/tree-ssa-phiprop.cc | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-)