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 <guoji...@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 <guoji...@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;
+}
--
2.25.1
Richard Biener <rguent...@suse.de> writes:
> 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.
>> >
