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 DCCF03857BBD; Mon, 11 Dec 2023 03:07:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DCCF03857BBD Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=linux.ibm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linux.ibm.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org DCCF03857BBD Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=148.163.158.5 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702264054; cv=none; b=mCdIkzHPwqcLXEKCeCfFHLLuTxwTLfyBwSCekFMLHyAfuQv+mKEXcVuuPlsu++IpYNn38CLlsJJkR4m1PuxgztL/53kIvuDr6ZHR0vUiJdV4H2RS/DcRW7lxHt3gvfmiIrRhSHBTMhVX+fibPEpW0B6BQ3Em/aOXhVso7wAvTGE= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702264054; c=relaxed/simple; bh=QDW6FHZQlHnuJKMcLnfh4Fp5hfBmPVp5isNYySEax2E=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=lniTFQy9qRjteZFcMtx9QTtVnlDr0QkmJIv8KNBKywWBT86rNqYOj2nC3rbdCBv1Op3btdrQ5WkncrmTYeg6/7MLLimNGZlMDFuUxy9VUiQi3GGIZeatnfwFjfJRCkPcVICJVdNWoWqUT1krk7EE8AElhLb0ZpvhcFzZ/e+NPPY= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from pps.filterd (m0353725.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 3BANKLbh024666; Mon, 11 Dec 2023 03:07:30 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=from : to : cc : subject : references : date : in-reply-to : message-id : content-type : mime-version; s=pp1; bh=ua5kkTTDcLQ8+AFkTd2X9GMYooPswJkeiaFBXS8sw+k=; b=e3ZFkHWJIm43amvldspN56GpxEB5uMbmxHXNtcAstK9HKRvRvIHN5M4Qb8+rI1uCYYiu OsLRDjWq8wr3jjPrX4jHxL8LpvSqUZQcN+whCFO1Em6A+4ezsjdFjVA8YL4mSHMHFS+I W8Hl2yZQwwf97ImolJoEu7le8u5p5HPG7jLFb95mgijOMPkjnLTBGNOdNobJAqmHGwYR 4BwN2GBHL7iCFmkzvjRHE5Q+JJS5xQRSEQPzSuoAm65WKtCUJq0J23FGjg9NuxiflKqW AfBWRNtRa8+ZPItUWZ9y2nxAFX1XNq+sBmeK1aR/KsiJsS3sJZ/tss3jM6Mq0OcKTJDz 6g== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3uwfura5wd-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 11 Dec 2023 03:07:30 +0000 Received: from m0353725.ppops.net (m0353725.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 3BB309F9003455; Mon, 11 Dec 2023 03:07:29 GMT Received: from ppma22.wdc07v.mail.ibm.com (5c.69.3da9.ip4.static.sl-reverse.com [169.61.105.92]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3uwfura5w6-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 11 Dec 2023 03:07:29 +0000 Received: from pps.filterd (ppma22.wdc07v.mail.ibm.com [127.0.0.1]) by ppma22.wdc07v.mail.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 3BB0eOP7028244; Mon, 11 Dec 2023 03:07:28 GMT Received: from smtprelay04.dal12v.mail.ibm.com ([172.16.1.6]) by ppma22.wdc07v.mail.ibm.com (PPS) with ESMTPS id 3uw2xy6sm2-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 11 Dec 2023 03:07:28 +0000 Received: from smtpav04.dal12v.mail.ibm.com (smtpav04.dal12v.mail.ibm.com [10.241.53.103]) by smtprelay04.dal12v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 3BB37Sqa22938204 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 11 Dec 2023 03:07:28 GMT Received: from smtpav04.dal12v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 1123658056; Mon, 11 Dec 2023 03:07:28 +0000 (GMT) Received: from smtpav04.dal12v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id CD65B58052; Mon, 11 Dec 2023 03:07:27 +0000 (GMT) Received: from genoa (unknown [9.40.192.157]) by smtpav04.dal12v.mail.ibm.com (Postfix) with ESMTPS; Mon, 11 Dec 2023 03:07:27 +0000 (GMT) From: Jiufu Guo To: Jeff Law Cc: gcc-patches@gcc.gnu.org, rguenther@suse.de, richard.sandiford@arm.com, zadeck@naturalbridge.com, pinskia@gcc.gnu.org, segher@kernel.crashing.org, dje.gcc@gmail.com, linkw@gcc.gnu.org, bergner@linux.ibm.com Subject: Re: [PATCH] treat argp-based mem as frame related in dse References: <20231206092758.1000447-1-guojiufu@linux.ibm.com> <0aee8eb3-0dce-4d0b-ab1c-891feeba8950@gmail.com> <21b33e79-aaf8-4d5a-a002-9c677d1349c3@gmail.com> Date: Mon, 11 Dec 2023 11:07:23 +0800 In-Reply-To: <21b33e79-aaf8-4d5a-a002-9c677d1349c3@gmail.com> (Jeff Law's message of "Sun, 10 Dec 2023 11:19:26 -0700") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) Content-Type: text/plain X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: 8sedCQCv5c1cKTsaoNZR1KX4jHgD6wBB X-Proofpoint-GUID: wy5ryVebMiWOcOx6mSQcNW2viqrUhhyx X-Proofpoint-UnRewURL: 0 URL was un-rewritten MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-12-10_16,2023-12-07_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 phishscore=0 priorityscore=1501 impostorscore=0 bulkscore=0 mlxlogscore=999 lowpriorityscore=0 spamscore=0 adultscore=0 mlxscore=0 malwarescore=0 clxscore=1015 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311290000 definitions=main-2312110025 X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,KAM_SHORT,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi, Thanks again for your kind review! Jeff Law writes: > On 12/8/23 00:18, Jiufu Guo wrote: >> >> Hi, >> >> Jeff Law writes: >> >>> On 12/6/23 02:27, Jiufu Guo wrote: >>>> Hi, >>>> >>>> The issue mentioned in PR112525 would be able to be handled by >>>> updating dse.cc to treat arg_pointer_rtx similarly with frame_pointer_rtx. >>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=30271#c10 also mentioned >>>> this idea. >>>> One thing, arpg area may be used to pass argument to callee. So, it would >>>> be needed to check if call insns are using that mem. >>>> >>>> Bootstrap ®test pass on ppc64{,le} and x86_64. >>>> Is this ok for trunk? >>>> >>>> BR, >>>> Jeff (Jiufu Guo) >>>> >>>> >>>> PR rtl-optimization/112525 >>>> >>>> gcc/ChangeLog: >>>> >>>> * dse.cc (get_group_info): Add arg_pointer_rtx as frame_related. >>>> (check_mem_read_rtx): Add parameter to indicate if it is checking mem >>>> for call insn. >>>> (scan_insn): Add mem checking on call usage. >>>> >>>> gcc/testsuite/ChangeLog: >>>> >>>> * gcc.target/powerpc/pr112525.c: New test. >>> So conceptually the first chunk makes sense. Though I do worry about >>> Andrew's comment about it causing a bootstrap failure. Even thought >>> it was 15 years ago, it remains worrisome. >>> >> Yes, I understand your point. >> At that time, it is a comparesion failure. It may be related to debug >> info. But I did not figure out possible failures. >> >>> >>>> @@ -2368,7 +2370,8 @@ check_mem_read_rtx (rtx *loc, bb_info_t bb_info) >>>> /* If this read is just reading back something that we just >>>> stored, rewrite the read. */ >>>> - if (store_info->rhs >>>> + if (!used_in_call >>>> + && store_info->rhs >>>> && store_info->group_id == -1 >>>> && store_info->cse_base == base >>>> && known_subrange_p (offset, width, store_info->offset, >>>> @@ -2650,6 +2653,12 @@ scan_insn (bb_info_t bb_info, rtx_insn *insn, int max_active_local_stores) >>>> that is not relative to the frame. */ >>>> add_non_frame_wild_read (bb_info); >>>> + for (rtx link = CALL_INSN_FUNCTION_USAGE (insn); >>>> + link != NULL_RTX; >>>> + link = XEXP (link, 1)) >>>> + if (GET_CODE (XEXP (link, 0)) == USE && MEM_P (XEXP (XEXP (link, 0),0))) >>>> + check_mem_read_rtx (&XEXP (XEXP (link, 0),0), bb_info, true); >>> I'm having a bit of a hard time convincing myself this is correct >>> though. I can't see how rewriting the load to read the source of the >>> prior store is unsafe. If that fixes a problem, then it would seem >>> like we've gone wrong before here -- perhaps failing to use the fusage >>> loads to "kill" any available stores to the same or aliased memory >>> locations. >> As you said the later one, call's fusage would killing the previous >> store. It is a kind of case like: >> >> 134: [argp:SI+0x8]=r134:SI >> 135: [argp:SI+0x4]=0x1 >> 136: [argp:SI]=r132:SI >> 137: ax:SI=call [`memset'] argc:0xc >> REG_CALL_DECL `memset' >> REG_EH_REGION 0 >> >> This call insn is: >> (call_insn/j 137 136 147 27 (set (reg:SI 0 ax) >> (call (mem:QI (symbol_ref:SI ("memset") [flags 0x41] ) [0 __builtin_memset S1 A8]) >> (const_int 12 [0xc]))) "pr102798.c":23:22 1086 {*sibcall_value} >> (expr_list:REG_UNUSED (reg:SI 0 ax) >> (expr_list:REG_CALL_DECL (symbol_ref:SI ("memset") [flags 0x41] ) >> (expr_list:REG_EH_REGION (const_int 0 [0]) >> (nil)))) >> (expr_list:SI (use (mem/f:SI (reg/f:SI 16 argp) [0 S4 A32])) >> (expr_list:SI (use (mem:SI (plus:SI (reg/f:SI 16 argp) (const_int 4 [0x4])) [0 S4 A32])) >> (expr_list:SI (use (mem:SI (plus:SI (reg/f:SI 16 argp) (const_int 8 [0x8])) [0 S4 A32])) >> (nil))))) >> >> The stores in "insns 134-136" are used by the call. "check_mem_read_rtx" >> would prevent them to eliminated. > Right. But unless I read something wrong, the patch wasn't changing > store removal, it was changing whether or not we forwarded the source > of the store into the destination of a subsequent load from the same > address. "check_mem_read_rtx" has another behavior which checks the mem and adds read_info to insn_info->read_rec. "read_rec" could prevent the "store" from being eliminated during the dse's global alg. This patch leverages this behavior. And to avoid the "mem on fusage" to be replaced by leading store's rhs "replace_read" was disabled if the mem is on the call's fusage. BR, Jeff (Jiufu Guo) > > So I'm still a bit confused how this patch impacted store removal. > > jeff