From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by sourceware.org (Postfix) with ESMTPS id 479A93857C73 for ; Tue, 17 Nov 2020 08:09:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 479A93857C73 Received: from pps.filterd (m0187473.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 0AH81xMC041035; Tue, 17 Nov 2020 03:09:44 -0500 Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com with ESMTP id 34v7dsmhk8-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 17 Nov 2020 03:09:44 -0500 Received: from m0187473.ppops.net (m0187473.ppops.net [127.0.0.1]) by pps.reinject (8.16.0.36/8.16.0.36) with SMTP id 0AH82UtY042582; Tue, 17 Nov 2020 03:09:43 -0500 Received: from ppma03wdc.us.ibm.com (ba.79.3fa9.ip4.static.sl-reverse.com [169.63.121.186]) by mx0a-001b2d01.pphosted.com with ESMTP id 34v7dsmhjg-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 17 Nov 2020 03:09:43 -0500 Received: from pps.filterd (ppma03wdc.us.ibm.com [127.0.0.1]) by ppma03wdc.us.ibm.com (8.16.0.42/8.16.0.42) with SMTP id 0AH87slM019442; Tue, 17 Nov 2020 08:09:42 GMT Received: from b03cxnp08025.gho.boulder.ibm.com (b03cxnp08025.gho.boulder.ibm.com [9.17.130.17]) by ppma03wdc.us.ibm.com with ESMTP id 34t6v8vrgr-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 17 Nov 2020 08:09:42 +0000 Received: from b03ledav002.gho.boulder.ibm.com (b03ledav002.gho.boulder.ibm.com [9.17.130.233]) by b03cxnp08025.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 0AH89ZTM5178034 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 17 Nov 2020 08:09:35 GMT Received: from b03ledav002.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 4047F136065; Tue, 17 Nov 2020 08:09:41 +0000 (GMT) Received: from b03ledav002.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id D0F53136059; Tue, 17 Nov 2020 08:09:40 +0000 (GMT) Received: from genoa (unknown [9.40.192.157]) by b03ledav002.gho.boulder.ibm.com (Postfix) with ESMTPS; Tue, 17 Nov 2020 08:09:40 +0000 (GMT) From: Jiufu Guo To: Richard Biener Cc: Richard Biener , GCC Patches , Segher Boessenkool , Bill Schmidt , David Edelsohn Subject: Re: [PATCH V2] Clean up loop-closed PHIs after loop finalize References: <20201105131836.1043501-1-guojiufu@linux.ibm.com> <643c465403d11f033d072ab66224b21e@linux.vnet.ibm.com> <42c93146f7c9ac873edf3d31293b17ed@linux.vnet.ibm.com> Date: Tue, 17 Nov 2020 16:09:38 +0800 In-Reply-To: <42c93146f7c9ac873edf3d31293b17ed@linux.vnet.ibm.com> (Jiufu Guo's message of "Tue, 17 Nov 2020 13:58:53 +0800") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-TM-AS-GCONF: 00 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.312, 18.0.737 definitions=2020-11-17_02:2020-11-13, 2020-11-17 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 mlxscore=0 spamscore=0 bulkscore=0 clxscore=1015 phishscore=0 malwarescore=0 priorityscore=1501 suspectscore=2 mlxlogscore=886 impostorscore=0 lowpriorityscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2011170054 X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 17 Nov 2020 08:09:47 -0000 Jiufu Guo writes: > On 2020-11-16 17:35, Richard Biener wrote: >> On Mon, Nov 16, 2020 at 10:26 AM Jiufu Guo >> wrote: >>> >>> Jiufu Guo writes: >>> >>> > Richard Biener 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 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; +}