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 23AE93858D32; Tue, 13 Jun 2023 02:34:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 23AE93858D32 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 Received: from pps.filterd (m0356516.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 35D2HG6E002208; Tue, 13 Jun 2023 02:34:13 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=oyyjMbvak/7002s9AbmX/oTylebw12POXx/itlycbvU=; b=CZtsjPidBlOK07U3X1o7QYDwmCLmZ/mZUJOV9LVEfAet1XMPb+eSEIVJ11X/dRhYimly zcIr1bWZdacuRcv9Q5Rn/zCXIoLK3zdmSqTc63iosDd4mYuiC2krXBuu7Jxk1w7wsi2V c9rxHpAlU+vjiQy4WnqoHR6nMDYWr7eRDCTs2rywSzAJIohsVhrGxAgpqVi5M/C/Zq5b MJIU5E+MWEctop5/qMWf8PdLY4dOA/G9A2Fu5crwd/jHB+AxUGnr3hhigarl4kSn1gPr lfeCNQZuhraXTDOOQVzIW8QJVbuNx37ca2FuvHLm7j86BZ7qm3zarN/yJlr2R3HY43RA Xw== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3r6fhr090s-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 13 Jun 2023 02:34:13 +0000 Received: from m0356516.ppops.net (m0356516.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 35D2S499003345; Tue, 13 Jun 2023 02:34:12 GMT Received: from ppma05wdc.us.ibm.com (1b.90.2fa9.ip4.static.sl-reverse.com [169.47.144.27]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3r6fhr090f-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 13 Jun 2023 02:34:12 +0000 Received: from pps.filterd (ppma05wdc.us.ibm.com [127.0.0.1]) by ppma05wdc.us.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 35D0vU5c014149; Tue, 13 Jun 2023 02:34:12 GMT Received: from smtprelay06.wdc07v.mail.ibm.com ([9.208.129.118]) by ppma05wdc.us.ibm.com (PPS) with ESMTPS id 3r4gt5e60j-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 13 Jun 2023 02:34:12 +0000 Received: from smtpav01.dal12v.mail.ibm.com (smtpav01.dal12v.mail.ibm.com [10.241.53.100]) by smtprelay06.wdc07v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 35D2YA0652691418 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 13 Jun 2023 02:34:11 GMT Received: from smtpav01.dal12v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id B1C9D58069; Tue, 13 Jun 2023 02:34:10 +0000 (GMT) Received: from smtpav01.dal12v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 8946358064; Tue, 13 Jun 2023 02:34:10 +0000 (GMT) Received: from ltcden2-lp1.aus.stglabs.ibm.com (unknown [9.3.90.43]) by smtpav01.dal12v.mail.ibm.com (Postfix) with ESMTPS; Tue, 13 Jun 2023 02:34:10 +0000 (GMT) From: Jiufu Guo To: Jeff Law Cc: Richard Biener , Richard Sandiford , gcc-patches@gcc.gnu.org, segher@kernel.crashing.org, dje.gcc@gmail.com, linkw@gcc.gnu.org, bergner@linux.ibm.com Subject: Re: [PATCH] Make sure SCALAR_INT_MODE_P before invoke try_const_anchors References: <20230609052847.2128612-1-guojiufu@linux.ibm.com> <56dbba43adda001d1668c29e8024c85d@linux.ibm.com> <7nwn0dgfvj.fsf@ltcden2-lp1.aus.stglabs.ibm.com> <7no7lohklj.fsf@ltcden2-lp1.aus.stglabs.ibm.com> <7na5x5dyjx.fsf@ltcden2-lp1.aus.stglabs.ibm.com> Date: Tue, 13 Jun 2023 10:34:06 +0800 In-Reply-To: (Jeff Law's message of "Mon, 12 Jun 2023 14:54:30 -0600") Message-ID: <7npm60ccpt.fsf@ltcden2-lp1.aus.stglabs.ibm.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) Content-Type: text/plain X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: np6trPAdaV-6tJFc_a842Jaw_PqDdxgk X-Proofpoint-GUID: QDfZ6Jcik7PDnC9RRgMfgbcu0hjt9_Qg X-Proofpoint-UnRewURL: 0 URL was un-rewritten MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.573,FMLib:17.11.176.26 definitions=2023-06-12_18,2023-06-12_02,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 mlxscore=0 lowpriorityscore=0 suspectscore=0 priorityscore=1501 bulkscore=0 clxscore=1015 impostorscore=0 adultscore=0 spamscore=0 malwarescore=0 mlxlogscore=999 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2305260000 definitions=main-2306130020 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_H5,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, Jeff Law writes: > On 6/11/23 23:44, Jiufu Guo wrote: >> Richard Biener writes: >> >>> On Fri, 9 Jun 2023, Jiufu Guo wrote: >>> >>>> >>>> Hi, >>>> >>>> Richard Biener writes: >>>> >>>>> On Fri, 9 Jun 2023, Jiufu Guo wrote: >>>>> >>>>>> >>>>>> Hi, >>>>>> >>>>>> Richard Biener writes: >>>>>> >>>>>>> On Fri, 9 Jun 2023, Richard Sandiford wrote: >>>>>>> >>>>>>>> guojiufu writes: >>>>>>>>> Hi, >>>>>>>>> >>>>>>>>> On 2023-06-09 16:00, Richard Biener wrote: >>>>>>>>>> On Fri, 9 Jun 2023, Jiufu Guo wrote: >>>>>>>>>> >>>>>>>>>>> Hi, >>>>>>>>>>> >>>> ... >>>>>>>>>>> >>>>>>>>>>> This patch is raised when drafting below one. >>>>>>>>>>> https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603530.html. >>>>>>>>>>> With that patch, "{[%1:DI]=0;} stack_tie" with BLKmode runs into >>>>>>>>>>> try_const_anchors, and hits the assert/ice. >>>>>>>>>>> >>>>>>>>>>> Boostrap and regtest pass on ppc64{,le} and x86_64. >>>>>>>>>>> Is this ok for trunk? >>>>>>>>>> >>>>>>>>>> Iff the correct fix at all (how can a CONST_INT have BLKmode?) then >>>>>>>>>> I suggest to instead fix try_const_anchors to change >>>>>>>>>> >>>>>>>>>> /* CONST_INT is used for CC modes, but we should leave those alone. >>>>>>>>>> */ >>>>>>>>>> if (GET_MODE_CLASS (mode) == MODE_CC) >>>>>>>>>> return NULL_RTX; >>>>>>>>>> >>>>>>>>>> gcc_assert (SCALAR_INT_MODE_P (mode)); >>>>>>>>>> >>>>>>>>>> to >>>>>>>>>> >>>>>>>>>> /* CONST_INT is used for CC modes, leave any non-scalar-int mode >>>>>>>>>> alone. */ >>>>>>>>>> if (!SCALAR_INT_MODE_P (mode)) >>>>>>>>>> return NULL_RTX; >>>>>>>>>> >>>>>>>>> >>>>>>>>> This is also able to fix this issue. there is a "Punt on CC modes" >>>>>>>>> patch >>>>>>>>> to return NULL_RTX in try_const_anchors. >>>>>>>>> >>>>>>>>>> but as said I wonder how we arrive at a BLKmode CONST_INT and whether >>>>>>>>>> we should have fended this off earlier. Can you share more complete >>>>>>>>>> RTL of that stack_tie? >>>>>>>>> >>>>>>>>> >>>>>>>>> (insn 15 14 16 3 (parallel [ >>>>>>>>> (set (mem/c:BLK (reg/f:DI 1 1) [1 A8]) >>>>>>>>> (const_int 0 [0])) >>>>>>>>> ]) "/home/guojiufu/temp/gdb.c":13:3 922 {stack_tie} >>>>>>>>> (nil)) >>>>>>>>> >>>>>>>>> It is "set (mem/c:BLK (reg/f:DI 1 1) (const_int 0 [0])". >>>>>>>> >>>>>>>> I'm not convinced this is correct RTL. (unspec:BLK [(const_int 0)] ...) >>>>>>>> would be though. It's arguably more accurate too, since the effect >>>>>>>> on the stack locations is unspecified rather than predictable. >>>>>>> >>>>>>> powerpc seems to be the only port with a stack_tie that's not >>>>>>> using an UNSPEC RHS. >>>>>> In rs6000.md, it is >>>>>> >>>>>> ; This is to explain that changes to the stack pointer should >>>>>> ; not be moved over loads from or stores to stack memory. >>>>>> (define_insn "stack_tie" >>>>>> [(match_parallel 0 "tie_operand" >>>>>> [(set (mem:BLK (reg 1)) (const_int 0))])] >>>>>> "" >>>>>> "" >>>>>> [(set_attr "length" "0")]) >>>>>> >>>>>> This would be just an placeholder insn, and acts as the comments. >>>>>> UNSPEC_ would works like other targets. While, I'm wondering >>>>>> the concerns on "set (mem:BLK (reg 1)) (const_int 0)". >>>>>> MODEs between SET_DEST and SET_SRC? >>>>> >>>>> I don't think the issue is the mode but the issue is that >>>>> the patter as-is says some memory is zeroed while that's not >>>>> actually true (not specifying a size means we can't really do >>>>> anything with this MEM, but still). Using an UNSPEC avoids >>>>> implying anything for the stored value. >>>>> >>>>> Of course I think a MEM SET_DEST without a specified size is bougs >>>>> as well, but there's larger precedent for this... >>>> >>>> Thanks for your kindly comments! >>>> Using "(set (mem:BLK (reg 1)) (const_int 0))" here, may because this >>>> insn does not generate real thing (not a real store and no asm code), >>>> may like barrier. >>>> >>>> While I agree that, using UNSPEC may be more clear to avoid mis-reading. >>> >>> Btw, another way to avoid the issue in CSE is to make it not process >>> (aka record anything for optimization) for SET from MEMs with >>> !MEM_SIZE_KNOWN_P >> >> Thanks! Yes, this would make sense. >> Then, there are two ideas(patches) to handle this issue: >> Which one would be preferable? This one (from compiling time aspect)? >> >> And maybe, the changes in rs6000 stack_tie through using unspec >> can be a standalone enhancement besides cse patch. > I'd tend to lean more towards fixing the rs6000 backend. It's basically lying to the rest of the compiler and when it presents passes with something like > > (set (mem:BLK) (const_int 0)) > > It's largely inviting the generic bits to treat it like a memory store, when in fact it's something significantly different. > > I don't think the CSE patch is wrong or a bad idea, more that it's > just papering over a problem caused by an odd chunk of RTL created by > the PPC backend. Thanks a lot for your very kindly and helpful review! Agree with you comments! A patch in rs6000 was prepared to handle this. BR, Jeff (Jiufu Guo) > > jeff