[PATCH] Clean up loop-closed PHIs at loopdone pass

Richard Biener richard.guenther@gmail.com
Thu Nov 5 13:43:15 GMT 2020


On Thu, Nov 5, 2020 at 2:19 PM guojiufu via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> In PR87473, there are discussions about loop-closed PHIs which
> are generated for loop optimization passes.  It would be helpful
> to clean them up after loop optimization is done, then this may
> simplify some jobs of following passes.
> This patch introduces a cheaper way to propagate them out in
> pass_tree_loop_done.
>
> This patch passes bootstrap and regtest on ppc64le.  Is this ok for trunk?

Huh, I think this is somewhat useless work, the PHIs won't survive for long
and you certainly cannot expect degenerate PHIs to not occur anyway.
You probably can replace propagate_rhs_into_lhs by the
existing replace_uses_by function.  You're walking loop exits
after loop_optimizer_finalize () - that's wasting work.  If you want to
avoid inconsistent state and we really want to go with this I suggest
to instead add a flag to loop_optimizer_finalize () as to whether to
propagate out LC PHI nodes or not and do this from within there.

Thanks,
Richard.

> gcc/ChangeLog
> 2020-10-05  Jiufu Guo   <guojiufu@linux.ibm.com>
>
>         * tree-ssa-loop.h (clean_up_loop_closed_phi): New declaration.
>         * tree-ssa-loop.c (tree_ssa_loop_done): Call clean_up_loop_closed_phi.
>         * tree-ssa-propagate.c (propagate_rhs_into_lhs): New function.
>
> gcc/testsuite/ChangeLog
> 2020-10-05  Jiufu Guo   <guojiufu@linux.ibm.com>
>
>         * gcc.dg/tree-ssa/loopclosedphi.c: New test.
> ---
>  gcc/testsuite/gcc.dg/tree-ssa/loopclosedphi.c |  21 +++
>  gcc/tree-ssa-loop.c                           |   1 +
>  gcc/tree-ssa-loop.h                           |   1 +
>  gcc/tree-ssa-propagate.c                      | 120 ++++++++++++++++++
>  4 files changed, 143 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/loopclosedphi.c
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/loopclosedphi.c b/gcc/testsuite/gcc.dg/tree-ssa/loopclosedphi.c
> new file mode 100644
> index 00000000000..d71b757fbca
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/loopclosedphi.c
> @@ -0,0 +1,21 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3 -fno-tree-ch -w -fdump-tree-loopdone-details" } */
> +
> +void
> +t6 (int qz, int wh)
> +{
> +  int jl = wh;
> +
> +  while (1.0 * qz / wh < 1)
> +    {
> +      qz = wh * (wh + 2);
> +
> +      while (wh < 1)
> +        jl = 0;
> +    }
> +
> +  while (qz < 1)
> +    qz = jl * wh;
> +}
> +
> +/* { dg-final { scan-tree-dump-times "Replacing" 2 "loopdone"} } */
> diff --git a/gcc/tree-ssa-loop.c b/gcc/tree-ssa-loop.c
> index 5e8365d4e83..7d680b2f5d2 100644
> --- a/gcc/tree-ssa-loop.c
> +++ b/gcc/tree-ssa-loop.c
> @@ -530,6 +530,7 @@ tree_ssa_loop_done (void)
>    free_numbers_of_iterations_estimates (cfun);
>    scev_finalize ();
>    loop_optimizer_finalize ();
> +  clean_up_loop_closed_phi (cfun);
>    return 0;
>  }
>
> diff --git a/gcc/tree-ssa-loop.h b/gcc/tree-ssa-loop.h
> index 9e35125e6e8..baa940b9d1e 100644
> --- a/gcc/tree-ssa-loop.h
> +++ b/gcc/tree-ssa-loop.h
> @@ -67,6 +67,7 @@ public:
>  extern bool for_each_index (tree *, bool (*) (tree, tree *, void *), void *);
>  extern char *get_lsm_tmp_name (tree ref, unsigned n, const char *suffix = NULL);
>  extern unsigned tree_num_loop_insns (class loop *, struct eni_weights *);
> +extern unsigned clean_up_loop_closed_phi (function *);
>
>  /* Returns the loop of the statement STMT.  */
>
> diff --git a/gcc/tree-ssa-propagate.c b/gcc/tree-ssa-propagate.c
> index 87dbf55fab9..813143852b9 100644
> --- a/gcc/tree-ssa-propagate.c
> +++ b/gcc/tree-ssa-propagate.c
> @@ -1549,4 +1549,123 @@ propagate_tree_value_into_stmt (gimple_stmt_iterator *gsi, tree val)
>    else
>      gcc_unreachable ();
>  }
> +
> +/* Propagate RHS into all uses of LHS (when possible).
> +
> +   RHS and LHS are derived from STMT, which is passed in solely so
> +   that we can remove it if propagation is successful.  */
> +
> +static bool
> +propagate_rhs_into_lhs (gphi *stmt, tree lhs, tree rhs)
> +{
> +  use_operand_p use_p;
> +  imm_use_iterator iter;
> +  gimple_stmt_iterator gsi;
> +  gimple *use_stmt;
> +  bool changed = false;
> +  bool all = true;
> +
> +  /* Dump details.  */
> +  if (dump_file && (dump_flags & TDF_DETAILS))
> +    {
> +      fprintf (dump_file, "  Replacing '");
> +      print_generic_expr (dump_file, lhs, dump_flags);
> +      fprintf (dump_file, "' with '");
> +      print_generic_expr (dump_file, rhs, dump_flags);
> +      fprintf (dump_file, "'\n");
> +    }
> +
> +  /* Walk over every use of LHS and try to replace the use with RHS. */
> +  FOR_EACH_IMM_USE_STMT (use_stmt, iter, lhs)
> +    {
> +      /* It is not safe to propagate into below stmts.  */
> +      if (gimple_debug_bind_p (use_stmt)
> +         || (gimple_code (use_stmt) == GIMPLE_ASM
> +             && !may_propagate_copy_into_asm (lhs))
> +         || (TREE_CODE (rhs) == SSA_NAME
> +             && SSA_NAME_DEF_STMT (rhs) == use_stmt))
> +       {
> +         all = false;
> +         continue;
> +       }
> +
> +      /* Dump details.  */
> +      if (dump_file && (dump_flags & TDF_DETAILS))
> +       {
> +         fprintf (dump_file, "    Original statement:");
> +         print_gimple_stmt (dump_file, use_stmt, 0, dump_flags);
> +       }
> +
> +      /* Propagate the RHS into this use of the LHS.  */
> +      FOR_EACH_IMM_USE_ON_STMT (use_p, iter)
> +       propagate_value (use_p, rhs);
> +
> +      /* Propagation may expose new operands to the renamer.  */
> +      update_stmt (use_stmt);
> +
> +      /* If variable index is replaced with a constant, then
> +        update the invariant flag for ADDR_EXPRs.  */
> +      if (gimple_assign_single_p (use_stmt)
> +         && TREE_CODE (gimple_assign_rhs1 (use_stmt)) == ADDR_EXPR)
> +       recompute_tree_invariant_for_addr_expr (gimple_assign_rhs1 (use_stmt));
> +
> +      /* Dump details.  */
> +      if (dump_file && (dump_flags & TDF_DETAILS))
> +       {
> +         fprintf (dump_file, "    Updated statement:");
> +         print_gimple_stmt (dump_file, use_stmt, 0, dump_flags);
> +       }
> +
> +      changed = true;
> +    }
> +
> +  /* Remove the degenerate PHI node.  */
> +  if (all)
> +    {
> +      gsi = gsi_for_stmt (stmt);
> +      remove_phi_node (&gsi, true);
> +    }
> +
> +  return changed;
> +}
> +
> +/* Check exits of each loop in FUN, walk over loop closed PHIs in
> +   each exit basic block and propagate degenerate PHIs.  */
> +
> +unsigned
> +clean_up_loop_closed_phi (function *fun)
> +{
> +  unsigned i;
> +  edge e;
> +  gphi *phi;
> +  tree rhs;
> +  tree lhs;
> +  gphi_iterator gsi;
> +  struct loop *loop;
> +  bool cfg_altered = false;
> +
> +  /* Walk over loop in function.  */
> +  FOR_EACH_LOOP_FN (fun, loop, 0)
> +    {
> +      /* Check each exit edge of loop.  */
> +      auto_vec<edge> exits = get_loop_exit_edges (loop);
> +      FOR_EACH_VEC_ELT (exits, i, e)
> +       if (single_pred_p (e->dest))
> +         /* Walk over loop-closed PHIs.  */
> +         for (gsi = gsi_start_phis (e->dest); !gsi_end_p (gsi);)
> +           {
> +             phi = gsi.phi ();
> +             rhs = degenerate_phi_result (phi);
> +             lhs = gimple_phi_result (phi);
> +
> +             /* Advance the iterator before stmt is removed.  */
> +             gsi_next (&gsi);
> +
> +             if (rhs && !virtual_operand_p (lhs)
> +                 && may_propagate_copy (lhs, rhs))
> +               cfg_altered |= propagate_rhs_into_lhs (phi, lhs, rhs);
> +           }
> +    }
> +
> +  return cfg_altered;
> +}
> --
> 2.25.1
>


More information about the Gcc-patches mailing list