[PATCH V2] Clean up loop-closed PHIs after loop finalize

Richard Biener rguenther@suse.de
Tue Nov 17 10:21:48 GMT 2020


On Tue, 17 Nov 2020, Jiufu Guo wrote:

> Jiufu Guo <guojiufu@linux.ibm.com> writes:
> 
> > On 2020-11-16 17:35, Richard Biener wrote:
> >> On Mon, Nov 16, 2020 at 10:26 AM Jiufu Guo <guojiufu@linux.ibm.com>
> >> wrote:
> >>>
> >>> Jiufu Guo <guojiufu@linux.ibm.com> writes:
> >>>
> >>> > Richard Biener <rguenther@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 <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend


More information about the Gcc-patches mailing list