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   <guoji...@linux.ibm.com>

        * tree-ssa-loop.h (clean_up_loop_closed_phi): New declaration.
* tree-ssa-loop.c (tree_ssa_loop_done): Call clean_up_loop_closed_phi.
        * tree-ssa-propagate.c (propagate_rhs_into_lhs): New function.

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

        * gcc.dg/tree-ssa/loopclosedphi.c: New test.
---
 gcc/testsuite/gcc.dg/tree-ssa/loopclosedphi.c |  21 +++
 gcc/tree-ssa-loop.c                           |   1 +
 gcc/tree-ssa-loop.h                           |   1 +
gcc/tree-ssa-propagate.c | 120 ++++++++++++++++++
 4 files changed, 143 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/loopclosedphi.c

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/loopclosedphi.c b/gcc/testsuite/gcc.dg/tree-ssa/loopclosedphi.c
new file mode 100644
index 00000000000..d71b757fbca
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/loopclosedphi.c
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -fno-tree-ch -w -fdump-tree-loopdone-details" } */
+
+void
+t6 (int qz, int wh)
+{
+  int jl = wh;
+
+  while (1.0 * qz / wh < 1)
+    {
+      qz = wh * (wh + 2);
+
+      while (wh < 1)
+        jl = 0;
+    }
+
+  while (qz < 1)
+    qz = jl * wh;
+}
+
+/* { dg-final { scan-tree-dump-times "Replacing" 2 "loopdone"} } */
diff --git a/gcc/tree-ssa-loop.c b/gcc/tree-ssa-loop.c
index 5e8365d4e83..7d680b2f5d2 100644
--- a/gcc/tree-ssa-loop.c
+++ b/gcc/tree-ssa-loop.c
@@ -530,6 +530,7 @@ tree_ssa_loop_done (void)
   free_numbers_of_iterations_estimates (cfun);
   scev_finalize ();
   loop_optimizer_finalize ();
+  clean_up_loop_closed_phi (cfun);
   return 0;
 }

diff --git a/gcc/tree-ssa-loop.h b/gcc/tree-ssa-loop.h
index 9e35125e6e8..baa940b9d1e 100644
--- a/gcc/tree-ssa-loop.h
+++ b/gcc/tree-ssa-loop.h
@@ -67,6 +67,7 @@ public:
extern bool for_each_index (tree *, bool (*) (tree, tree *, void *), void *); extern char *get_lsm_tmp_name (tree ref, unsigned n, const char *suffix = NULL); extern unsigned tree_num_loop_insns (class loop *, struct eni_weights *);
+extern unsigned clean_up_loop_closed_phi (function *);

 /* Returns the loop of the statement STMT.  */

diff --git a/gcc/tree-ssa-propagate.c b/gcc/tree-ssa-propagate.c
index 87dbf55fab9..813143852b9 100644
--- a/gcc/tree-ssa-propagate.c
+++ b/gcc/tree-ssa-propagate.c
@@ -1549,4 +1549,123 @@ propagate_tree_value_into_stmt (gimple_stmt_iterator *gsi, tree val)
   else
     gcc_unreachable ();
 }
+
+/* Propagate RHS into all uses of LHS (when possible).
+
+   RHS and LHS are derived from STMT, which is passed in solely so
+   that we can remove it if propagation is successful.  */
+
+static bool
+propagate_rhs_into_lhs (gphi *stmt, tree lhs, tree rhs)
+{
+  use_operand_p use_p;
+  imm_use_iterator iter;
+  gimple_stmt_iterator gsi;
+  gimple *use_stmt;
+  bool changed = false;
+  bool all = true;
+
+  /* Dump details.  */
+  if (dump_file && (dump_flags & TDF_DETAILS))
+    {
+      fprintf (dump_file, "  Replacing '");
+      print_generic_expr (dump_file, lhs, dump_flags);
+      fprintf (dump_file, "' with '");
+      print_generic_expr (dump_file, rhs, dump_flags);
+      fprintf (dump_file, "'\n");
+    }
+
+ /* Walk over every use of LHS and try to replace the use with RHS. */
+  FOR_EACH_IMM_USE_STMT (use_stmt, iter, lhs)
+    {
+      /* It is not safe to propagate into below stmts.  */
+      if (gimple_debug_bind_p (use_stmt)
+         || (gimple_code (use_stmt) == GIMPLE_ASM
+             && !may_propagate_copy_into_asm (lhs))
+         || (TREE_CODE (rhs) == SSA_NAME
+             && SSA_NAME_DEF_STMT (rhs) == use_stmt))
+       {
+         all = false;
+         continue;
+       }
+
+      /* Dump details.  */
+      if (dump_file && (dump_flags & TDF_DETAILS))
+       {
+         fprintf (dump_file, "    Original statement:");
+         print_gimple_stmt (dump_file, use_stmt, 0, dump_flags);
+       }
+
+      /* Propagate the RHS into this use of the LHS.  */
+      FOR_EACH_IMM_USE_ON_STMT (use_p, iter)
+       propagate_value (use_p, rhs);
+
+      /* Propagation may expose new operands to the renamer.  */
+      update_stmt (use_stmt);
+
+      /* If variable index is replaced with a constant, then
+        update the invariant flag for ADDR_EXPRs.  */
+      if (gimple_assign_single_p (use_stmt)
+         && TREE_CODE (gimple_assign_rhs1 (use_stmt)) == ADDR_EXPR)
+ recompute_tree_invariant_for_addr_expr (gimple_assign_rhs1 (use_stmt));
+
+      /* Dump details.  */
+      if (dump_file && (dump_flags & TDF_DETAILS))
+       {
+         fprintf (dump_file, "    Updated statement:");
+         print_gimple_stmt (dump_file, use_stmt, 0, dump_flags);
+       }
+
+      changed = true;
+    }
+
+  /* Remove the degenerate PHI node.  */
+  if (all)
+    {
+      gsi = gsi_for_stmt (stmt);
+      remove_phi_node (&gsi, true);
+    }
+
+  return changed;
+}
+
+/* Check exits of each loop in FUN, walk over loop closed PHIs in
+   each exit basic block and propagate degenerate PHIs.  */
+
+unsigned
+clean_up_loop_closed_phi (function *fun)
+{
+  unsigned i;
+  edge e;
+  gphi *phi;
+  tree rhs;
+  tree lhs;
+  gphi_iterator gsi;
+  struct loop *loop;
+  bool cfg_altered = false;
+
+  /* Walk over loop in function.  */
+  FOR_EACH_LOOP_FN (fun, loop, 0)
+    {
+      /* Check each exit edge of loop.  */
+      auto_vec<edge> exits = get_loop_exit_edges (loop);
+      FOR_EACH_VEC_ELT (exits, i, e)
+       if (single_pred_p (e->dest))
+         /* Walk over loop-closed PHIs.  */
+         for (gsi = gsi_start_phis (e->dest); !gsi_end_p (gsi);)
+           {
+             phi = gsi.phi ();
+             rhs = degenerate_phi_result (phi);
+             lhs = gimple_phi_result (phi);
+
+             /* Advance the iterator before stmt is removed.  */
+             gsi_next (&gsi);
+
+             if (rhs && !virtual_operand_p (lhs)
+                 && may_propagate_copy (lhs, rhs))
+               cfg_altered |= propagate_rhs_into_lhs (phi, lhs, rhs);
+           }
+    }
+
+  return cfg_altered;
+}
--
2.25.1

Reply via email to