Clean up loop-closed PHIs at loopdone pass

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?

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

2.25.1

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
>

On 2020-11-05 21:43, Richard Biener wrote:

Hi Richard,

Thanks for your comments and suggestions!

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

After `loopdone` pass, those loop-closed-PHIs will still live ~10 passes
(veclower, switchlower, slsr...) till the next `copyprop` pass.
It would be helpful to those passes if we can eliminate those 
degenerated PHIs
in a cheaper way.  As you mentioned in
https://gcc.gnu.org/legacy-ml/gcc-patches/2018-10/msg00834.html

We know vrp/dom may generate some degenerated PHIS, and then we have 
`copyprop`
was added after each vrp/dom pair to propagate out those PHIs.  Likely, 
I
think for loop-closed PHIs, we may also eliminate them once they are not 
needed.


> You probably can replace propagate_rhs_into_lhs by the
> existing replace_uses_by function.  You're walking loop exits

Yes, replace_uses_by + remove_phi_node would be a good implementation
propagate_rhs_into_lhs.


Thanks!

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

Thank you for the suggestion!
You mean adding a flag and in loop_optimizer_finalize, and add code 
like:
```
if (flag_propagate_loop_closed_phi_when_loop_done)
{
   loops_state_clear (fn, LOOP_CLOSED_SSA)
   clean_up_loop_closed_phis(fn);
}
```

Is this align with your suggestions?
One concern: function loop_optimizer_finalize is called a lot of places,
while we just need to clean up loop-closed PHIs at GIMPLE loopdone pass.

Thanks again,

Jiufu Guo.

> 
> 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
>>

On Fri, 6 Nov 2020, Jiufu Guo wrote:

> On 2020-11-05 21:43, Richard Biener wrote:
> 
> Hi Richard,
> 
> Thanks for your comments and suggestions!
> 
> > 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.
> 
> After `loopdone` pass, those loop-closed-PHIs will still live ~10 passes
> (veclower, switchlower, slsr...) till the next `copyprop` pass.
> It would be helpful to those passes if we can eliminate those degenerated PHIs
> in a cheaper way.  As you mentioned in
> https://gcc.gnu.org/legacy-ml/gcc-patches/2018-10/msg00834.html
> 
> We know vrp/dom may generate some degenerated PHIS, and then we have
> `copyprop`
> was added after each vrp/dom pair to propagate out those PHIs.  Likely, I
> think for loop-closed PHIs, we may also eliminate them once they are not
> needed.
> 
> 
> > You probably can replace propagate_rhs_into_lhs by the
> > existing replace_uses_by function.  You're walking loop exits
> 
> Yes, replace_uses_by + remove_phi_node would be a good implementation
> propagate_rhs_into_lhs.
> 
> 
> Thanks!
> 
> > 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.
> 
> Thank you for the suggestion!
> You mean adding a flag and in loop_optimizer_finalize, and add code like:
> ```
> if (flag_propagate_loop_closed_phi_when_loop_done)
> {
>   loops_state_clear (fn, LOOP_CLOSED_SSA)
>   clean_up_loop_closed_phis(fn);
> }
> ```
> 
> Is this align with your suggestions?

Yeah.

> One concern: function loop_optimizer_finalize is called a lot of places,
> while we just need to clean up loop-closed PHIs at GIMPLE loopdone pass.

There are quite some other passes rewriting into LC SSA outside of
the loop pipeline.  [E]VRP for example but also invariant motion.

To avoid touching too many places you can default the new argument
to false for example.

Richard.

> Thanks again,
> 
> Jiufu Guo.
> 
> > 
> > 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
> >> 
> 
>

Thanks a lot for the sugguestion from previous mails.
The patch was updated accordingly.

This updated patch propagates loop-closed PHIs them out after
loop_optimizer_finalize under a new introduced flag.  At some cases,
to clean up loop-closed PHIs would save efforts of optimization passes
after loopdone.

This patch passes bootstrap and regtest on ppc64le.  Is this ok for trunk?

gcc/ChangeLog
2020-10-11  Jiufu Guo   <guojiufu@linux.ibm.com>

	* common.opt (flag_clean_up_loop_closed_phi): New flag.
	* loop-init.c (loop_optimizer_finalize): Check
	flag_clean_up_loop_closed_phi and call clean_up_loop_closed_phi.
	* tree-cfgcleanup.h (clean_up_loop_closed_phi): New declare.
	* tree-ssa-propagate.c (clean_up_loop_closed_phi): New function.

gcc/testsuite/ChangeLog
2020-10-11  Jiufu Guo   <guojiufu@linux.ibm.com>

	* gcc.dg/tree-ssa/loopclosedphi.c: New test.

---
 gcc/common.opt                                |  4 ++
 gcc/loop-init.c                               |  8 +++
 gcc/testsuite/gcc.dg/tree-ssa/loopclosedphi.c | 21 +++++++
 gcc/tree-cfgcleanup.h                         |  1 +
 gcc/tree-ssa-propagate.c                      | 61 +++++++++++++++++++
 5 files changed, 95 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/loopclosedphi.c

diff --git a/gcc/common.opt b/gcc/common.opt
index 7e789d1c47f..f0d7b74d7ad 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1141,6 +1141,10 @@ fchecking=
 Common Joined RejectNegative UInteger Var(flag_checking)
 Perform internal consistency checkings.
 
+fclean-up-loop-closed-phi
+Common Report Var(flag_clean_up_loop_closed_phi) Optimization Init(0)
+Clean up loop-closed PHIs after loop optimization done.
+
 fcode-hoisting
 Common Report Var(flag_code_hoisting) Optimization
 Enable code hoisting.
diff --git a/gcc/loop-init.c b/gcc/loop-init.c
index 401e5282907..05804759ac9 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.  */
@@ -145,6 +146,13 @@ loop_optimizer_finalize (struct function *fn)
 
   free_numbers_of_iterations_estimates (fn);
 
+  if (flag_clean_up_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 we should preserve loop structure, do not free it but clear
      flags that advanced properties are there as we are not preserving
      that in full.  */
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..ab22a991935
--- /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 -fclean-up-loop-closed-phi" } */
+
+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-propagate.c b/gcc/tree-ssa-propagate.c
index 87dbf55fab9..a3bfe36c733 100644
--- a/gcc/tree-ssa-propagate.c
+++ b/gcc/tree-ssa-propagate.c
@@ -1549,3 +1549,64 @@ 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;
+  bool cfg_altered = false;
+
+  /* Check dominator info before get loop-close PHIs from loop exits.  */
+  if (dom_info_state (CDI_DOMINATORS) != DOM_OK)
+    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 = degenerate_phi_result (phi);
+	      lhs = gimple_phi_result (phi);
+
+	      if (rhs && may_propagate_copy (lhs, rhs))
+		{
+		  gimple_stmt_iterator psi = gsi;
+		  /* Advance the iterator before stmt is removed.  */
+		  gsi_next (&gsi);
+
+		  /* 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");
+		    }
+
+		  replace_uses_by (lhs, rhs);
+		  remove_phi_node (&psi, true);
+		  cfg_altered = true;
+		}
+	      else
+		gsi_next (&gsi);
+	    }
+    }
+
+  return cfg_altered;
+}

On Wed, 11 Nov 2020, Jiufu Guo wrote:

> 
> Thanks a lot for the sugguestion from previous mails.
> The patch was updated accordingly.
> 
> This updated patch propagates loop-closed PHIs them out after
> loop_optimizer_finalize under a new introduced flag.  At some cases,
> to clean up loop-closed PHIs would save efforts of optimization passes
> after loopdone.
> 
> This patch passes bootstrap and regtest on ppc64le.  Is this ok for trunk?

Comments below

> gcc/ChangeLog
> 2020-10-11  Jiufu Guo   <guojiufu@linux.ibm.com>
> 
> 	* common.opt (flag_clean_up_loop_closed_phi): New flag.
> 	* loop-init.c (loop_optimizer_finalize): Check
> 	flag_clean_up_loop_closed_phi and call clean_up_loop_closed_phi.
> 	* tree-cfgcleanup.h (clean_up_loop_closed_phi): New declare.
> 	* tree-ssa-propagate.c (clean_up_loop_closed_phi): New function.
> 
> gcc/testsuite/ChangeLog
> 2020-10-11  Jiufu Guo   <guojiufu@linux.ibm.com>
> 
> 	* gcc.dg/tree-ssa/loopclosedphi.c: New test.
> 
> ---
>  gcc/common.opt                                |  4 ++
>  gcc/loop-init.c                               |  8 +++
>  gcc/testsuite/gcc.dg/tree-ssa/loopclosedphi.c | 21 +++++++
>  gcc/tree-cfgcleanup.h                         |  1 +
>  gcc/tree-ssa-propagate.c                      | 61 +++++++++++++++++++
>  5 files changed, 95 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/loopclosedphi.c
> 
> diff --git a/gcc/common.opt b/gcc/common.opt
> index 7e789d1c47f..f0d7b74d7ad 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -1141,6 +1141,10 @@ fchecking=
>  Common Joined RejectNegative UInteger Var(flag_checking)
>  Perform internal consistency checkings.
>  
> +fclean-up-loop-closed-phi
> +Common Report Var(flag_clean_up_loop_closed_phi) Optimization Init(0)
> +Clean up loop-closed PHIs after loop optimization done.
> +
>  fcode-hoisting
>  Common Report Var(flag_code_hoisting) Optimization
>  Enable code hoisting.
> diff --git a/gcc/loop-init.c b/gcc/loop-init.c
> index 401e5282907..05804759ac9 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.  */
> @@ -145,6 +146,13 @@ loop_optimizer_finalize (struct function *fn)
>  
>    free_numbers_of_iterations_estimates (fn);
>  
> +  if (flag_clean_up_loop_closed_phi

Sorry if there was miscommunication but I've not meant to add a
new user-visible flag but instead a flag argument to loop_optimizer_finalize
(as said, you can default it to false to only need to change the
one in fini_loops)

> +      && loops_state_satisfies_p (fn, LOOP_CLOSED_SSA))
> +    {
> +      clean_up_loop_closed_phi (fn);
> +      loops_state_clear (fn, LOOP_CLOSED_SSA);
> +    }
> +
>    /* If we should preserve loop structure, do not free it but clear
>       flags that advanced properties are there as we are not preserving
>       that in full.  */
> 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..ab22a991935
> --- /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 -fclean-up-loop-closed-phi" } */
> +
> +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-propagate.c b/gcc/tree-ssa-propagate.c
> index 87dbf55fab9..a3bfe36c733 100644
> --- a/gcc/tree-ssa-propagate.c
> +++ b/gcc/tree-ssa-propagate.c
> @@ -1549,3 +1549,64 @@ 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;
> +  bool cfg_altered = false;
> +
> +  /* Check dominator info before get loop-close PHIs from loop exits.  */
> +  if (dom_info_state (CDI_DOMINATORS) != DOM_OK)

Why?

> +    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 = degenerate_phi_result (phi);

You have a single predecessor, thus the result is always degenerate.

> +	      lhs = gimple_phi_result (phi);
> +
> +	      if (rhs && may_propagate_copy (lhs, rhs))
> +		{
> +		  gimple_stmt_iterator psi = gsi;
> +		  /* Advance the iterator before stmt is removed.  */
> +		  gsi_next (&gsi);

remove_phi_node should take care of this, you shouldn't need this
(just do not advance the iterator when you remove the PHI node).

> +
> +		  /* 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");
> +		    }
> +
> +		  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);
    }

Thanks,
Richard.

> +		}
> +	      else
> +		gsi_next (&gsi);
> +	    }
> +    }
> +
> +  return cfg_altered;
> +}
>

Richard Biener <rguenther@suse.de> writes:

> On Wed, 11 Nov 2020, Jiufu Guo wrote:
>
>> 
>> Thanks a lot for the sugguestion from previous mails.
>> The patch was updated accordingly.
>> 
>> This updated patch propagates loop-closed PHIs them out after
>> loop_optimizer_finalize under a new introduced flag.  At some cases,
>> to clean up loop-closed PHIs would save efforts of optimization passes
>> after loopdone.
>> 
>> This patch passes bootstrap and regtest on ppc64le.  Is this ok for trunk?
>
> Comments below
>
>>    free_numbers_of_iterations_estimates (fn);
>>  
>> +  if (flag_clean_up_loop_closed_phi
>
> Sorry if there was miscommunication but I've not meant to add a
> new user-visible flag but instead a flag argument to loop_optimizer_finalize
> (as said, you can default it to false to only need to change the
> one in fini_loops)
Sorry for misunderstand.
Updated the patch.
>
>> +      && loops_state_satisfies_p (fn, LOOP_CLOSED_SSA))
>> +    {
>> +      clean_up_loop_closed_phi (fn);
>> +      loops_state_clear (fn, LOOP_CLOSED_SSA);
>> +    }
>> +
......
>> +  gphi *phi;
>> +  tree rhs;
>> +  tree lhs;
>> +  gphi_iterator gsi;
>> +  struct loop *loop;
>> +  bool cfg_altered = false;
>> +
>> +  /* Check dominator info before get loop-close PHIs from loop exits.  */
>> +  if (dom_info_state (CDI_DOMINATORS) != DOM_OK)
>
> Why?
>
As you said, loop_optimizer_finalize is also called from where dominator
info is not ready, e.g. called from: vrp(execute_vrp).
At there, loop exits info is not ready, and then
get_loop_exit_edges/get_loop_body_with_size function does not work.

>> +  /* 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 = degenerate_phi_result (phi);
>
> You have a single predecessor, thus the result is always degenerate.
>
>> +	      lhs = gimple_phi_result (phi);
>> +
>> +	      if (rhs && may_propagate_copy (lhs, rhs))
>> +		{
>> +		  gimple_stmt_iterator psi = gsi;
>> +		  /* Advance the iterator before stmt is removed.  */
>> +		  gsi_next (&gsi);
>
> remove_phi_node should take care of this, you shouldn't need this
> (just do not advance the iterator when you remove the PHI node).
>
Yeap, get it! Thanks, will update the patch accordingly.
>> +
......
>> +
>> +		  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.

BR. 
Jiufu Guo
>
> Thanks,
> Richard.
>
>> +		}
>> +	      else
>> +		gsi_next (&gsi);
>> +	    }
>> +    }
>> +
>> +  return cfg_altered;
>> +}
>>

Jiufu Guo <guojiufu@linux.ibm.com> writes:

> Richard Biener <rguenther@suse.de> writes:
>
>> On Wed, 11 Nov 2020, Jiufu Guo wrote:
>>
>>> 
>>> Thanks a lot for the sugguestion from previous mails.
>>> The patch was updated accordingly.
>>> 
>>> This updated patch propagates loop-closed PHIs them out after
>>> loop_optimizer_finalize under a new introduced flag.  At some cases,
>>> to clean up loop-closed PHIs would save efforts of optimization passes
>>> after loopdone.
>>> 
>>> This patch passes bootstrap and regtest on ppc64le.  Is this ok for trunk?
>>
>> Comments below
>>
>>>    free_numbers_of_iterations_estimates (fn);
>>>  
>>> +  if (flag_clean_up_loop_closed_phi
>>
>> Sorry if there was miscommunication but I've not meant to add a
>> new user-visible flag but instead a flag argument to loop_optimizer_finalize
>> (as said, you can default it to false to only need to change the
>> one in fini_loops)
> Sorry for misunderstand.
> Updated the patch.

Thanks for the comments!  The patch was updated accordingly.

This updated patch clean up loop closed PHIs in loop_optimizer_finalize,
which is called when some loop optimizations are done. For some cases,
this would save efforts of optimization passes after loopdone.

gcc/ChangeLog:
2020-10-16  Jiufu Guo   <guojiufu@linux.ibm.com>

	* cfgloop.h (loop_optimizer_finalize): Add flag argument.
	* loop-init.c (loop_optimizer_finalize): Call clean_up_loop_closed_phi.
	* tree-cfgcleanup.h (clean_up_loop_closed_phi): New declare.
	* tree-ssa-loop.c (tree_ssa_loop_done): Call loop_optimizer_finalize
	with flag argument.
	* tree-ssa-propagate.c (clean_up_loop_closed_phi): New function.

gcc/testsuite/ChangeLog:
2020-10-16  Jiufu Guo   <guojiufu@linux.ibm.com>

	* gcc.dg/tree-ssa/loopclosedphi.c: New test.

---
 gcc/cfgloop.h                                 |  2 +-
 gcc/loop-init.c                               |  9 ++-
 gcc/testsuite/gcc.dg/tree-ssa/loopclosedphi.c | 21 +++++++
 gcc/tree-cfgcleanup.h                         |  1 +
 gcc/tree-ssa-loop.c                           |  2 +-
 gcc/tree-ssa-propagate.c                      | 63 +++++++++++++++++++
 6 files changed, 95 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/loopclosedphi.c

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..0cb6f509b89 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,7 +134,7 @@ 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;
@@ -145,6 +146,12 @@ loop_optimizer_finalize (struct function *fn)
 
   free_numbers_of_iterations_estimates (fn);
 
+  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 we should preserve loop structure, do not free it but clear
      flags that advanced properties are there as we are not preserving
      that in full.  */
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..daa1db82afc 100644
--- a/gcc/tree-ssa-propagate.c
+++ b/gcc/tree-ssa-propagate.c
@@ -1549,3 +1549,66 @@ 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;
+
+  /* Check dominator info before get loop-close PHIs from loop exits.  */
+  if (dom_info_state (CDI_DOMINATORS) != DOM_OK)
+    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 = degenerate_phi_result (phi);
+	      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);
+		    }
+		  remove_phi_node (&gsi, true);
+		}
+	      else
+		gsi_next (&gsi);
+	    }
+    }
+
+  return 0;
+}

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:
> >>
> >>>
> >>> Thanks a lot for the sugguestion from previous mails.
> >>> The patch was updated accordingly.
> >>>
> >>> This updated patch propagates loop-closed PHIs them out after
> >>> loop_optimizer_finalize under a new introduced flag.  At some cases,
> >>> to clean up loop-closed PHIs would save efforts of optimization passes
> >>> after loopdone.
> >>>
> >>> This patch passes bootstrap and regtest on ppc64le.  Is this ok for trunk?
> >>
> >> Comments below
> >>
> >>>    free_numbers_of_iterations_estimates (fn);
> >>>
> >>> +  if (flag_clean_up_loop_closed_phi
> >>
> >> Sorry if there was miscommunication but I've not meant to add a
> >> new user-visible flag but instead a flag argument to loop_optimizer_finalize
> >> (as said, you can default it to false to only need to change the
> >> one in fini_loops)
> > Sorry for misunderstand.
> > Updated the patch.
>
> Thanks for the comments!  The patch was updated accordingly.
>
> This updated patch clean up loop closed PHIs in loop_optimizer_finalize,
> which is called when some loop optimizations are done. For some cases,
> this would save efforts of optimization passes after loopdone.
>
> gcc/ChangeLog:
> 2020-10-16  Jiufu Guo   <guojiufu@linux.ibm.com>
>
>         * cfgloop.h (loop_optimizer_finalize): Add flag argument.
>         * loop-init.c (loop_optimizer_finalize): Call clean_up_loop_closed_phi.
>         * tree-cfgcleanup.h (clean_up_loop_closed_phi): New declare.
>         * tree-ssa-loop.c (tree_ssa_loop_done): Call loop_optimizer_finalize
>         with flag argument.
>         * tree-ssa-propagate.c (clean_up_loop_closed_phi): New function.
>
> gcc/testsuite/ChangeLog:
> 2020-10-16  Jiufu Guo   <guojiufu@linux.ibm.com>
>
>         * gcc.dg/tree-ssa/loopclosedphi.c: New test.
>
> ---
>  gcc/cfgloop.h                                 |  2 +-
>  gcc/loop-init.c                               |  9 ++-
>  gcc/testsuite/gcc.dg/tree-ssa/loopclosedphi.c | 21 +++++++
>  gcc/tree-cfgcleanup.h                         |  1 +
>  gcc/tree-ssa-loop.c                           |  2 +-
>  gcc/tree-ssa-propagate.c                      | 63 +++++++++++++++++++
>  6 files changed, 95 insertions(+), 3 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/loopclosedphi.c
>
> 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..0cb6f509b89 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,7 +134,7 @@ 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;
> @@ -145,6 +146,12 @@ loop_optimizer_finalize (struct function *fn)
>
>    free_numbers_of_iterations_estimates (fn);
>
> +  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 we should preserve loop structure, do not free it but clear
>       flags that advanced properties are there as we are not preserving
>       that in full.  */
> 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..daa1db82afc 100644
> --- a/gcc/tree-ssa-propagate.c
> +++ b/gcc/tree-ssa-propagate.c
> @@ -1549,3 +1549,66 @@ 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;
> +
> +  /* 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;

> +    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 = degenerate_phi_result (phi);

  rhs = gimple_phi_arg_def (phi, 0);

OK with that changes.

Thanks,
Richard.

> +             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);
> +                   }
> +                 remove_phi_node (&gsi, true);
> +               }
> +             else
> +               gsi_next (&gsi);
> +           }
> +    }
> +
> +  return 0;
> +}
> --
> 2.25.1
>
>
>
>
> >>
> >>> +      && loops_state_satisfies_p (fn, LOOP_CLOSED_SSA))
> >>> +    {
> >>> +      clean_up_loop_closed_phi (fn);
> >>> +      loops_state_clear (fn, LOOP_CLOSED_SSA);
> >>> +    }
> >>> +
> > ......
> >>> +  gphi *phi;
> >>> +  tree rhs;
> >>> +  tree lhs;
> >>> +  gphi_iterator gsi;
> >>> +  struct loop *loop;
> >>> +  bool cfg_altered = false;
> >>> +
> >>> +  /* Check dominator info before get loop-close PHIs from loop exits.  */
> >>> +  if (dom_info_state (CDI_DOMINATORS) != DOM_OK)
> >>
> >> Why?
> >>
> > As you said, loop_optimizer_finalize is also called from where dominator
> > info is not ready, e.g. called from: vrp(execute_vrp).
> > At there, loop exits info is not ready, and then
> > get_loop_exit_edges/get_loop_body_with_size function does not work.
> >
> >>> +  /* 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 = degenerate_phi_result (phi);
> >>
> >> You have a single predecessor, thus the result is always degenerate.
> >>
> >>> +         lhs = gimple_phi_result (phi);
> >>> +
> >>> +         if (rhs && may_propagate_copy (lhs, rhs))
> >>> +           {
> >>> +             gimple_stmt_iterator psi = gsi;
> >>> +             /* Advance the iterator before stmt is removed.  */
> >>> +             gsi_next (&gsi);
> >>
> >> remove_phi_node should take care of this, you shouldn't need this
> >> (just do not advance the iterator when you remove the PHI node).
> >>
> > Yeap, get it! Thanks, will update the patch accordingly.
> >>> +
> > ......
> >>> +
> >>> +             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.
> >
> > BR.
> > Jiufu Guo
> >>
> >> Thanks,
> >> Richard.
> >>
> >>> +           }
> >>> +         else
> >>> +           gsi_next (&gsi);
> >>> +       }
> >>> +    }
> >>> +
> >>> +  return cfg_altered;
> >>> +}
> >>>

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:
>> >>
>> >>>
>> >>> Thanks a lot for the suggestion from previous mails.
>> >>> The patch was updated accordingly.
>> >>>
>> >>> This updated patch propagates loop-closed PHIs them out after
>> >>> loop_optimizer_finalize under a new introduced flag.  At some cases,
>> >>> to clean up loop-closed PHIs would save efforts of optimization passes
>> >>> after loopdone.
>> >>>
>> >>> This patch passes bootstrap and regtest on ppc64le.  Is this ok for trunk?
>> >>
>> >> Comments below
>> >>
>> >>>    free_numbers_of_iterations_estimates (fn);
>> >>>
>> >>> +  if (flag_clean_up_loop_closed_phi
>> >>
>> >> Sorry if there was miscommunication but I've not meant to add a
>> >> new user-visible flag but instead a flag argument to loop_optimizer_finalize
>> >> (as said, you can default it to false to only need to change the
>> >> one in fini_loops)
>> > Sorry for misunderstand.
>> > Updated the patch.
>> 
>> Thanks for the comments!  The patch was updated accordingly.
>> 
>> This updated patch clean up loop closed PHIs in 
>> loop_optimizer_finalize,
>> which is called when some loop optimizations are done. For some cases,
>> this would save efforts of optimization passes after loopdone.
>> 
>> gcc/ChangeLog:
>> 2020-10-16  Jiufu Guo   <guojiufu@linux.ibm.com>
>> 
>>         * cfgloop.h (loop_optimizer_finalize): Add flag argument.
>>         * loop-init.c (loop_optimizer_finalize): Call 
>> clean_up_loop_closed_phi.
>>         * tree-cfgcleanup.h (clean_up_loop_closed_phi): New declare.
>>         * tree-ssa-loop.c (tree_ssa_loop_done): Call 
>> loop_optimizer_finalize
>>         with flag argument.
>>         * tree-ssa-propagate.c (clean_up_loop_closed_phi): New 
>> function.
>> 
>> gcc/testsuite/ChangeLog:
>> 2020-10-16  Jiufu Guo   <guojiufu@linux.ibm.com>
>> 
>>         * gcc.dg/tree-ssa/loopclosedphi.c: New test.
>> 
>> ---
>>  gcc/cfgloop.h                                 |  2 +-
>>  gcc/loop-init.c                               |  9 ++-
>>  gcc/testsuite/gcc.dg/tree-ssa/loopclosedphi.c | 21 +++++++
>>  gcc/tree-cfgcleanup.h                         |  1 +
>>  gcc/tree-ssa-loop.c                           |  2 +-
>>  gcc/tree-ssa-propagate.c                      | 63 
>> +++++++++++++++++++
>>  6 files changed, 95 insertions(+), 3 deletions(-)
>>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/loopclosedphi.c
>> 
>> 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..0cb6f509b89 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,7 +134,7 @@ 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;
>> @@ -145,6 +146,12 @@ loop_optimizer_finalize (struct function *fn)
>> 
>>    free_numbers_of_iterations_estimates (fn);
>> 
>> +  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 we should preserve loop structure, do not free it but clear
>>       flags that advanced properties are there as we are not 
>> preserving
>>       that in full.  */
>> 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..daa1db82afc 100644
>> --- a/gcc/tree-ssa-propagate.c
>> +++ b/gcc/tree-ssa-propagate.c
>> @@ -1549,3 +1549,66 @@ 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;
>> +
>> +  /* 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;
>> +
>> +  /* 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 = degenerate_phi_result (phi);
> 
>   rhs = gimple_phi_arg_def (phi, 0);
Thanks, sorry for missing this, you mentioned in previous mail.

I will commit the updated patch.

Thanks!
Jiufu Guo.


> 
> OK with that changes.
> 
> Thanks,
> Richard.
> 
>> +             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);
>> +                   }
>> +                 remove_phi_node (&gsi, true);
>> +               }
>> +             else
>> +               gsi_next (&gsi);
>> +           }
>> +    }
>> +
>> +  return 0;
>> +}
>> --
>> 2.25.1
>> 
>> 
>> 
>> 
>> >>
>> >>> +      && loops_state_satisfies_p (fn, LOOP_CLOSED_SSA))
>> >>> +    {
>> >>> +      clean_up_loop_closed_phi (fn);
>> >>> +      loops_state_clear (fn, LOOP_CLOSED_SSA);
>> >>> +    }
>> >>> +
>> > ......
>> >>> +  gphi *phi;
>> >>> +  tree rhs;
>> >>> +  tree lhs;
>> >>> +  gphi_iterator gsi;
>> >>> +  struct loop *loop;
>> >>> +  bool cfg_altered = false;
>> >>> +
>> >>> +  /* Check dominator info before get loop-close PHIs from loop exits.  */
>> >>> +  if (dom_info_state (CDI_DOMINATORS) != DOM_OK)
>> >>
>> >> Why?
>> >>
>> > As you said, loop_optimizer_finalize is also called from where dominator
>> > info is not ready, e.g. called from: vrp(execute_vrp).
>> > At there, loop exits info is not ready, and then
>> > get_loop_exit_edges/get_loop_body_with_size function does not work.
>> >
>> >>> +  /* 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 = degenerate_phi_result (phi);
>> >>
>> >> You have a single predecessor, thus the result is always degenerate.
>> >>
>> >>> +         lhs = gimple_phi_result (phi);
>> >>> +
>> >>> +         if (rhs && may_propagate_copy (lhs, rhs))
>> >>> +           {
>> >>> +             gimple_stmt_iterator psi = gsi;
>> >>> +             /* Advance the iterator before stmt is removed.  */
>> >>> +             gsi_next (&gsi);
>> >>
>> >> remove_phi_node should take care of this, you shouldn't need this
>> >> (just do not advance the iterator when you remove the PHI node).
>> >>
>> > Yeap, get it! Thanks, will update the patch accordingly.
>> >>> +
>> > ......
>> >>> +
>> >>> +             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.
>> >
>> > BR.
>> > Jiufu Guo
>> >>
>> >> Thanks,
>> >> Richard.
>> >>
>> >>> +           }
>> >>> +         else
>> >>> +           gsi_next (&gsi);
>> >>> +       }
>> >>> +    }
>> >>> +
>> >>> +  return cfg_altered;
>> >>> +}
>> >>>

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.

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;
+}

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;
> +}
>