Jiufu Guo <guoji...@linux.ibm.com> writes:
> On 2020-11-16 17:35, Richard Biener wrote:
>> On Mon, Nov 16, 2020 at 10:26 AM Jiufu Guo <guoji...@linux.ibm.com>
>> wrote:
>>>
>>> Jiufu Guo <guoji...@linux.ibm.com> writes:
>>>
>>> > Richard Biener <rguent...@suse.de> writes:
>>> >
>>> >> On Wed, 11 Nov 2020, Jiufu Guo wrote:
>>> >>
......
>>> +
>>> + /* Check dominator info before get loop-close PHIs from loop
>>> exits. */
>>> + if (dom_info_state (CDI_DOMINATORS) != DOM_OK)
>>
>> Please change this to
>>
>> /* Avoid possibly quadratic work when scanning for loop exits
>> across
>> all loops of a nest. */
>> if (!loop_state_satisfies_p (LOOPS_HAVE_RECORDED_EXITS))
>> return 0;
>>
>
> Great suggestion, thanks!
>
> And, the patch for loop-init.c, is also updated a little as below: call
> clean_up_loop_closed_phi before release_recorded_exits, to avoid flag
> LOOPS_HAVE_RECORDED_EXITS is cleared before checked.
>
> -----------------
> diff --git a/gcc/loop-init.c b/gcc/loop-init.c
> index 401e5282907..ac87dafef6e 100644
> --- a/gcc/loop-init.c
> +++ b/gcc/loop-init.c
> @@ -33,6 +33,7 @@ along with GCC; see the file COPYING3. If not see
> #include "tree-ssa-loop-niter.h"
> #include "loop-unroll.h"
> #include "tree-scalar-evolution.h"
> +#include "tree-cfgcleanup.h"
>
> ^L
> /* Apply FLAGS to the loop state. */
> @@ -133,13 +134,19 @@ loop_optimizer_init (unsigned flags)
> /* Finalize loop structures. */
>
> void
> -loop_optimizer_finalize (struct function *fn)
> +loop_optimizer_finalize (struct function *fn, bool
> clean_loop_closed_phi)
> {
> class loop *loop;
> basic_block bb;
>
> timevar_push (TV_LOOP_FINI);
>
> + if (clean_loop_closed_phi && loops_state_satisfies_p (fn,
> LOOP_CLOSED_SSA))
> + {
> + clean_up_loop_closed_phi (fn);
> + loops_state_clear (fn, LOOP_CLOSED_SSA);
> + }
> +
> if (loops_state_satisfies_p (fn, LOOPS_HAVE_RECORDED_EXITS))
> release_recorded_exits (fn);
> ----------------
>>> + return 0;
>>> +
......
>>> + {
>>> + phi = gsi.phi ();
>>> + rhs = degenerate_phi_result (phi);
>>
>> rhs = gimple_phi_arg_def (phi, 0);
> Thanks, sorry for missing this, you mentioned in previous mail.
>
....
>>> > ......
>>> >>> +
>>> >>> + replace_uses_by (lhs, rhs);
>>> >>> + remove_phi_node (&psi, true);
>>> >>> + cfg_altered = true;
>>> >>
>>> >> in the end the return value is unused but I think we should avoid
>>> >> altering the CFG since doing so requires it to be cleaned up for
>>> >> unreachable blocks. That means to open-code replace_uses_by as
>>> >>
>>> >> imm_use_iterator imm_iter;
>>> >> use_operand_p use;
>>> >> gimple *stmt;
>>> >> FOR_EACH_IMM_USE_STMT (stmt, imm_iter, name)
>>> >> {
>>> >> FOR_EACH_IMM_USE_ON_STMT (use, imm_iter)
>>> >> replace_exp (use, val);
>>> >> update_stmt (stmt);
>>> >> }
>>> >
>>> > Thansk! This could also save some code in replace_uses_by.
With more checking on `replace_uses_by` and tests, when a const is propagated
into an assignment stmt that contains ADDR_EXPR, invariant flag of the stmt
would be updated.
------------
/* Update the invariant flag for ADDR_EXPR if replacing
a variable index with a constant. */
if (gimple_assign_single_p (use_stmt)
&& TREE_CODE (gimple_assign_rhs1 (use_stmt))
== ADDR_EXPR)
recompute_tree_invariant_for_addr_expr (
gimple_assign_rhs1 (use_stmt));
------------
And then the updated patch looks like:
This updated patch propagates loop-closed PHIs them out at
loop_optimizer_finalize. For some cases, to clean up loop-closed PHIs
would save efforts of optimization passes after loopdone.
This patch passes bootstrap and regtest on ppc64le.
Thanks for any comments.
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;
+}
