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 <guoji...@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 <guoji...@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
>
