On Thu, May 23, 2024 at 10:55 PM Andrew Pinski <quic_apin...@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_apin...@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
>
