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
>

Reply via email to