On Tue, 17 Nov 2020, Jiufu Guo wrote:
> Jiufu Guo <guoji...@linux.ibm.com> writes:
>
> > On 2020-11-16 17:35, Richard Biener wrote:
> >> On Mon, Nov 16, 2020 at 10:26 AM Jiufu Guo <guoji...@linux.ibm.com>
> >> wrote:
> >>>
> >>> Jiufu Guo <guoji...@linux.ibm.com> writes:
> >>>
> >>> > Richard Biener <rguent...@suse.de> writes:
> >>> >
> >>> >> On Wed, 11 Nov 2020, Jiufu Guo wrote:
> >>> >>
> ......
> >>> +
> >>> + /* Check dominator info before get loop-close PHIs from loop
> >>> exits. */
> >>> + if (dom_info_state (CDI_DOMINATORS) != DOM_OK)
> >>
> >> Please change this to
> >>
> >> /* Avoid possibly quadratic work when scanning for loop exits
> >> across
> >> all loops of a nest. */
> >> if (!loop_state_satisfies_p (LOOPS_HAVE_RECORDED_EXITS))
> >> return 0;
> >>
> >
> > Great suggestion, thanks!
> >
> > And, the patch for loop-init.c, is also updated a little as below: call
> > clean_up_loop_closed_phi before release_recorded_exits, to avoid flag
> > LOOPS_HAVE_RECORDED_EXITS is cleared before checked.
> >
> > -----------------
> > diff --git a/gcc/loop-init.c b/gcc/loop-init.c
> > index 401e5282907..ac87dafef6e 100644
> > --- a/gcc/loop-init.c
> > +++ b/gcc/loop-init.c
> > @@ -33,6 +33,7 @@ along with GCC; see the file COPYING3. If not see
> > #include "tree-ssa-loop-niter.h"
> > #include "loop-unroll.h"
> > #include "tree-scalar-evolution.h"
> > +#include "tree-cfgcleanup.h"
> >
> > ^L
> > /* Apply FLAGS to the loop state. */
> > @@ -133,13 +134,19 @@ loop_optimizer_init (unsigned flags)
> > /* Finalize loop structures. */
> >
> > void
> > -loop_optimizer_finalize (struct function *fn)
> > +loop_optimizer_finalize (struct function *fn, bool
> > clean_loop_closed_phi)
> > {
> > class loop *loop;
> > basic_block bb;
> >
> > timevar_push (TV_LOOP_FINI);
> >
> > + if (clean_loop_closed_phi && loops_state_satisfies_p (fn,
> > LOOP_CLOSED_SSA))
> > + {
> > + clean_up_loop_closed_phi (fn);
> > + loops_state_clear (fn, LOOP_CLOSED_SSA);
> > + }
> > +
> > if (loops_state_satisfies_p (fn, LOOPS_HAVE_RECORDED_EXITS))
> > release_recorded_exits (fn);
> > ----------------
> >>> + return 0;
> >>> +
> ......
> >>> + {
> >>> + phi = gsi.phi ();
> >>> + rhs = degenerate_phi_result (phi);
> >>
> >> rhs = gimple_phi_arg_def (phi, 0);
> > Thanks, sorry for missing this, you mentioned in previous mail.
> >
> ....
> >>> > ......
> >>> >>> +
> >>> >>> + replace_uses_by (lhs, rhs);
> >>> >>> + remove_phi_node (&psi, true);
> >>> >>> + cfg_altered = true;
> >>> >>
> >>> >> in the end the return value is unused but I think we should avoid
> >>> >> altering the CFG since doing so requires it to be cleaned up for
> >>> >> unreachable blocks. That means to open-code replace_uses_by as
> >>> >>
> >>> >> imm_use_iterator imm_iter;
> >>> >> use_operand_p use;
> >>> >> gimple *stmt;
> >>> >> FOR_EACH_IMM_USE_STMT (stmt, imm_iter, name)
> >>> >> {
> >>> >> FOR_EACH_IMM_USE_ON_STMT (use, imm_iter)
> >>> >> replace_exp (use, val);
> >>> >> update_stmt (stmt);
> >>> >> }
> >>> >
> >>> > Thansk! This could also save some code in replace_uses_by.
> With more checking on `replace_uses_by` and tests, when a const is propagated
> into an assignment stmt that contains ADDR_EXPR, invariant flag of the stmt
> would be updated.
>
> ------------
> /* Update the invariant flag for ADDR_EXPR if replacing
>
> a variable index with a constant. */
> 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));
> ------------
>
> And then the updated patch looks like:
>
> This updated patch propagates loop-closed PHIs them out at
> loop_optimizer_finalize. For some cases, to clean up loop-closed PHIs
> would save efforts of optimization passes after loopdone.
>
> This patch passes bootstrap and regtest on ppc64le.
> Thanks for any comments.
OK.
Thanks,
Richard.
> Thanks,
> Jiufu Guo.
>
> diff --git a/gcc/cfgloop.h b/gcc/cfgloop.h
> index d14689dc31f..438b1f779bb 100644
> --- a/gcc/cfgloop.h
> +++ b/gcc/cfgloop.h
> @@ -824,7 +824,7 @@ extern void init_set_costs (void);
>
> /* Loop optimizer initialization. */
> extern void loop_optimizer_init (unsigned);
> -extern void loop_optimizer_finalize (function *);
> +extern void loop_optimizer_finalize (function *, bool = false);
> inline void
> loop_optimizer_finalize ()
> {
> diff --git a/gcc/loop-init.c b/gcc/loop-init.c
> index 401e5282907..ac87dafef6e 100644
> --- a/gcc/loop-init.c
> +++ b/gcc/loop-init.c
> @@ -33,6 +33,7 @@ along with GCC; see the file COPYING3. If not see
> #include "tree-ssa-loop-niter.h"
> #include "loop-unroll.h"
> #include "tree-scalar-evolution.h"
> +#include "tree-cfgcleanup.h"
>
>
> /* Apply FLAGS to the loop state. */
> @@ -133,13 +134,19 @@ loop_optimizer_init (unsigned flags)
> /* Finalize loop structures. */
>
> void
> -loop_optimizer_finalize (struct function *fn)
> +loop_optimizer_finalize (struct function *fn, bool clean_loop_closed_phi)
> {
> class loop *loop;
> basic_block bb;
>
> timevar_push (TV_LOOP_FINI);
>
> + if (clean_loop_closed_phi && loops_state_satisfies_p (fn, LOOP_CLOSED_SSA))
> + {
> + clean_up_loop_closed_phi (fn);
> + loops_state_clear (fn, LOOP_CLOSED_SSA);
> + }
> +
> if (loops_state_satisfies_p (fn, LOOPS_HAVE_RECORDED_EXITS))
> release_recorded_exits (fn);
>
> 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-cfgcleanup.h b/gcc/tree-cfgcleanup.h
> index 6ff6726bfe4..9e368d63709 100644
> --- a/gcc/tree-cfgcleanup.h
> +++ b/gcc/tree-cfgcleanup.h
> @@ -26,5 +26,6 @@ extern bool cleanup_tree_cfg (unsigned = 0);
> extern bool fixup_noreturn_call (gimple *stmt);
> extern bool delete_unreachable_blocks_update_callgraph (cgraph_node
> *dst_node,
> bool update_clones);
> +extern unsigned clean_up_loop_closed_phi (function *);
>
> #endif /* GCC_TREE_CFGCLEANUP_H */
> diff --git a/gcc/tree-ssa-loop.c b/gcc/tree-ssa-loop.c
> index 5e8365d4e83..339a0c50bc8 100644
> --- a/gcc/tree-ssa-loop.c
> +++ b/gcc/tree-ssa-loop.c
> @@ -529,7 +529,7 @@ tree_ssa_loop_done (void)
> {
> free_numbers_of_iterations_estimates (cfun);
> scev_finalize ();
> - loop_optimizer_finalize ();
> + loop_optimizer_finalize (cfun, true);
> return 0;
> }
>
> diff --git a/gcc/tree-ssa-propagate.c b/gcc/tree-ssa-propagate.c
> index 87dbf55fab9..354057b48bf 100644
> --- a/gcc/tree-ssa-propagate.c
> +++ b/gcc/tree-ssa-propagate.c
> @@ -1549,3 +1549,75 @@ propagate_tree_value_into_stmt (gimple_stmt_iterator
> *gsi, tree val)
> else
> gcc_unreachable ();
> }
> +
> +/* 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;
> +
> + /* Avoid possibly quadratic work when scanning for loop exits across
> + all loops of a nest. */
> + if (!loops_state_satisfies_p (LOOPS_HAVE_RECORDED_EXITS))
> + return 0;
> +
> + /* Walk over loop in function. */
> + FOR_EACH_LOOP_FN (fun, loop, 0)
> + {
> + /* Check each exit edege 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 = gimple_phi_arg_def (phi, 0);
> + lhs = gimple_phi_result (phi);
> +
> + if (rhs && may_propagate_copy (lhs, rhs))
> + {
> + /* 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");
> + }
> +
> + use_operand_p use_p;
> + imm_use_iterator iter;
> + gimple *use_stmt;
> + FOR_EACH_IMM_USE_STMT (use_stmt, iter, lhs)
> + {
> + FOR_EACH_IMM_USE_ON_STMT (use_p, iter)
> + replace_exp (use_p, rhs);
> + update_stmt (use_stmt);
> +
> + /* Update the invariant flag for ADDR_EXPR if replacing
> + a variable index with a constant. */
> + 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));
> + }
> + remove_phi_node (&gsi, true);
> + }
> + else
> + gsi_next (&gsi);
> + }
> + }
> +
> + return 0;
> +}
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend
