From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0b-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id DF2D3389202B for ; Mon, 16 Nov 2020 07:59:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org DF2D3389202B Received: from pps.filterd (m0127361.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 0AG7X5TK054396; Mon, 16 Nov 2020 02:59:07 -0500 Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com with ESMTP id 34um2q1usb-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 16 Nov 2020 02:59:07 -0500 Received: from m0127361.ppops.net (m0127361.ppops.net [127.0.0.1]) by pps.reinject (8.16.0.36/8.16.0.36) with SMTP id 0AG7XrXt059152; Mon, 16 Nov 2020 02:59:06 -0500 Received: from ppma04wdc.us.ibm.com (1a.90.2fa9.ip4.static.sl-reverse.com [169.47.144.26]) by mx0a-001b2d01.pphosted.com with ESMTP id 34um2q1ury-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 16 Nov 2020 02:59:06 -0500 Received: from pps.filterd (ppma04wdc.us.ibm.com [127.0.0.1]) by ppma04wdc.us.ibm.com (8.16.0.42/8.16.0.42) with SMTP id 0AG7qLV9027352; Mon, 16 Nov 2020 07:59:06 GMT Received: from b01cxnp22035.gho.pok.ibm.com (b01cxnp22035.gho.pok.ibm.com [9.57.198.25]) by ppma04wdc.us.ibm.com with ESMTP id 34t6v8na2y-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 16 Nov 2020 07:59:06 +0000 Received: from b01ledav005.gho.pok.ibm.com (b01ledav005.gho.pok.ibm.com [9.57.199.110]) by b01cxnp22035.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 0AG7x5eH9896618 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 16 Nov 2020 07:59:05 GMT Received: from b01ledav005.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 6ACADAE060; Mon, 16 Nov 2020 07:59:05 +0000 (GMT) Received: from b01ledav005.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id F0765AE05F; Mon, 16 Nov 2020 07:59:04 +0000 (GMT) Received: from genoa (unknown [9.40.192.157]) by b01ledav005.gho.pok.ibm.com (Postfix) with ESMTPS; Mon, 16 Nov 2020 07:59:04 +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> Date: Mon, 16 Nov 2020 15:59:02 +0800 In-Reply-To: (Richard Biener's message of "Fri, 13 Nov 2020 09:18:46 +0100 (CET)") 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-16_02:2020-11-13, 2020-11-16 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 adultscore=0 malwarescore=0 suspectscore=2 lowpriorityscore=0 mlxlogscore=650 clxscore=1015 spamscore=0 phishscore=0 mlxscore=0 bulkscore=0 priorityscore=1501 impostorscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2011160043 X-Spam-Status: No, score=-5.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, 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: Mon, 16 Nov 2020 07:59:10 -0000 Richard Biener 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 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; >> +} >>