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 5937B3882AEC; Wed, 19 Jun 2024 07:29:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5937B3882AEC 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 5937B3882AEC Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=148.163.156.1 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718782179; cv=none; b=KA66cNZrR2BKaDBJnHvy0H2NhaDlXNRtxxU+fjHNrBRNO/wrokvs5VxqEbzg4MzKv667A4eVFz5Xaf+bNynXtAlqSCCKooOHg0CbDbC7dH2CiAKjE8X7JZHgz2+Tlmt1VnAkwMM63f1ZKh/rFUNxnZjM9sjU6eeJWD3+7IqTG5g= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718782179; c=relaxed/simple; bh=3DHc/C7JKqjTuO3l14fLqq2ZaF7eYTxAGD8zCL6EnWM=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=qHoaEXk3h3f26lcVMU0Fvokt29x+AhJsWXIZIMg975g2Ct72rMSQ2hB7DIZV4C7xvODvTsUY38ph21xNbRYdKomTvVIu1S/V53iFXkjwTjD313inJSt5sSpjuNwVh0GIf5oBEjaIOBeUU6ac4Oz1Hj/z9gxWfg+UmigzdjACvjU= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from pps.filterd (m0353728.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.18.1.2/8.18.1.2) with ESMTP id 45J7RBVU015628; Wed, 19 Jun 2024 07:29:36 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h= message-id:date:mime-version:subject:to:cc:references:from :in-reply-to:content-type:content-transfer-encoding; s=pp1; bh=+ tIAQhCWM6gJyKff4fDxMryDdq2kk8xNdUgeeWkzp3o=; b=cI/2/9cb28n9azUUt tRa+635VCGZuYdLsXAlr7irL2xHGdz2JrewBJMvP0aSJ1ok7TBbP467XE/9h6Biz pdVSTgeDwW9WVF/YUlReWElD9lJB2jdVgVUZAEAu0kqKbm/oK2PI3D7sa6uiJTST GJDm5YaiBJLzrB9EeZDlKaPrupIbTPIzUiWaRojhB2YLZklnPI+ESIaXkGb0RZc4 QRB4KNg3uDLNVdjsmsqUHN9utkYRib0ntnwr54qmavJECywellutbvf3ZmAPmFMk 8dYUBgVqlFawb3BWEDyEPx+uaZL5hr8iblXB+e4mlWFpEx2hiXQCmG3/IwZGCeus pzIxQ== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3yuthe81xp-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 19 Jun 2024 07:29:36 +0000 (GMT) Received: from m0353728.ppops.net (m0353728.ppops.net [127.0.0.1]) by pps.reinject (8.18.0.8/8.18.0.8) with ESMTP id 45J7TZGA018717; Wed, 19 Jun 2024 07:29:35 GMT Received: from ppma12.dal12v.mail.ibm.com (dc.9e.1632.ip4.static.sl-reverse.com [50.22.158.220]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3yuthe81xm-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 19 Jun 2024 07:29:35 +0000 (GMT) Received: from pps.filterd (ppma12.dal12v.mail.ibm.com [127.0.0.1]) by ppma12.dal12v.mail.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 45J75vFR006285; Wed, 19 Jun 2024 07:29:34 GMT Received: from smtprelay02.fra02v.mail.ibm.com ([9.218.2.226]) by ppma12.dal12v.mail.ibm.com (PPS) with ESMTPS id 3ysn9uteuw-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 19 Jun 2024 07:29:34 +0000 Received: from smtpav02.fra02v.mail.ibm.com (smtpav02.fra02v.mail.ibm.com [10.20.54.101]) by smtprelay02.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 45J7TTCC55378384 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 19 Jun 2024 07:29:31 GMT Received: from smtpav02.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 308A520040; Wed, 19 Jun 2024 07:29:29 +0000 (GMT) Received: from smtpav02.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 261DA2004B; Wed, 19 Jun 2024 07:29:27 +0000 (GMT) Received: from [9.200.158.244] (unknown [9.200.158.244]) by smtpav02.fra02v.mail.ibm.com (Postfix) with ESMTP; Wed, 19 Jun 2024 07:29:26 +0000 (GMT) Message-ID: <5ec38640-2948-2503-80d8-b07145bd79a3@linux.ibm.com> Date: Wed, 19 Jun 2024 15:29:25 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 Subject: Re: [PATCH v4] rs6000: Fix incorrect RTL for Power LE when removing the UNSPECS [PR106069] Content-Language: en-US To: Segher Boessenkool Cc: gcc-patches@gcc.gnu.org, dje.gcc@gmail.com, guojiufu@linux.ibm.com, linkw@gcc.gnu.org, Peter Bergner , Xionghu Luo References: <20230210025952.1887696-1-xionghuluo@tencent.com> <20240618203137.GU19790@gate.crashing.org> From: "Kewen.Lin" In-Reply-To: <20240618203137.GU19790@gate.crashing.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: wyd1EQajRWkudBxW9yJd2DcKpmL20Cly X-Proofpoint-GUID: 8Z8U6vj1n9BJs3dMte7ZYbZHS15OEsX3 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.293,Aquarius:18.0.1039,Hydra:6.0.680,FMLib:17.12.28.16 definitions=2024-06-19_02,2024-06-17_01,2024-05-17_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 suspectscore=0 spamscore=0 phishscore=0 adultscore=0 clxscore=1015 priorityscore=1501 mlxscore=0 mlxlogscore=999 lowpriorityscore=0 bulkscore=0 impostorscore=0 malwarescore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.19.0-2405170001 definitions=main-2406190053 X-Spam-Status: No, score=-7.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_MSPIKE_H3,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 Segher, on 2024/6/19 04:31, Segher Boessenkool wrote: > On Fri, Feb 10, 2023 at 10:59:52AM +0800, Xionghu Luo via Gcc-patches wrote: > So, nothing here is obvious at all still. Could you please split it up > a bit more, so that every step is either small or simple? I just chatted with Xionghu off-list, he is being busy on some other tasks and preferred me to follow up this. > > So maybe first just split patterns to BE and LE versions, and nothing > else? > > And one patch per insn, if at all possible. OK, I'll try to separate them as element type word, half-word and byte. > > This matters so that a regression search will immediately show the > culprit pattern, if anything went wrong. > > Most patches will not change anything consequential, but some will, and > it should be very clear which do! > > And change (or add) comments in the patch so that I don't have to ask > the same questions as before again! :-) > > Most of this seems clean and good, but there is just too much > independent stuff going on at the same time. If your patch series is > split up correctly writing a changelog for it is very easy (this is a > good canary to use!), and if we get regressions from this it should be > trivial to fond the problem, too. Good point. > >> @@ -3699,13 +3799,13 @@ (define_expand "vec_widen_umult_hi_v16qi" >> { >> emit_insn (gen_altivec_vmuleub (ve, operands[1], operands[2])); >> emit_insn (gen_altivec_vmuloub (vo, operands[1], operands[2])); >> - emit_insn (gen_altivec_vmrghh_direct (operands[0], ve, vo)); >> + emit_insn (gen_altivec_vmrghh_direct_be (operands[0], ve, vo)); >> } >> else >> { >> emit_insn (gen_altivec_vmuloub (ve, operands[1], operands[2])); >> emit_insn (gen_altivec_vmuleub (vo, operands[1], operands[2])); >> - emit_insn (gen_altivec_vmrghh_direct (operands[0], vo, ve)); >> + emit_insn (gen_altivec_vmrghh_direct_le (operands[0], vo, ve)); >> } >> DONE; >> }) > > Please don't. Call the generic gen_vmrg* patterns from the widen > things, don't try to do the compilers job of specialising stuff, it > only makes things much less readable, and causes more mistakes. Just do > like what was there before, essentially. Before r12-4496 (the culprit commit), this part looks like: @@ -3795,182 +3708,182 @@ (define_expand "vec_widen_smult_hi_v16qi" emit_insn (gen_altivec_vmrghh_direct (operands[0], ve, vo)); } else { emit_insn (gen_altivec_vmulosb (ve, operands[1], operands[2])); emit_insn (gen_altivec_vmulesb (vo, operands[1], operands[2])); emit_insn (gen_altivec_vmrghh_direct (operands[0], vo, ve)); } DONE; }) , its associated gen_altivec_vmrghh_direct looks like: -(define_insn "altivec_vmrghh_direct" - [(set (match_operand:V8HI 0 "register_operand" "=v") - (unspec:V8HI [(match_operand:V8HI 1 "register_operand" "v") - (match_operand:V8HI 2 "register_operand" "v")] - UNSPEC_VMRGH_DIRECT))] - "TARGET_ALTIVEC" "vmrghh %0,%1,%2" [(set_attr "type" "vecperm")]) , the intention is to emit exactly the insn "vmrghh". It's doable to call gen_vmrg* here instead, but I'm not sure if it's more readable, as this vec_widen_smult_hi_v16qi expander already has the different arms for BE and LE, for calling with the generic gen_vmrg*, it would be gen_altivec_vmrghb for BE and gen_altivec_vmrglb for LE, for LE readers need to be more careful that we actually generate vmrghh. From this perspective, gen_altivec_vmrghh_direct_{be,le} seems more straight. BR, Kewen