Message ID | 17e8b572-e9c1-44f6-aff8-b14cda8321f9@linux.vnet.ibm.com |
---|---|
State | Accepted |
Headers |
Return-Path: <gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7300:a81b:b0:108:e6aa:91d0 with SMTP id bq27csp2319982dyb; Mon, 26 Feb 2024 12:35:51 -0800 (PST) X-Forwarded-Encrypted: i=3; AJvYcCUjLetRXQiB3MqjnIY63wBsLQEPune9VzGarE/xkqdeoWpc3fhMtCV9q+155Jy4ObBh/RRWUxpz+QXEwXF8Spituq6fbg== X-Google-Smtp-Source: AGHT+IHx5bxUlIgiD71N6CjNexy6YDUEYv8r8DYOe4ZQtlq9VNf28dUu0YvMmfig6Bc/TYVziTyq X-Received: by 2002:a05:6358:5e04:b0:179:f2:daa4 with SMTP id q4-20020a0563585e0400b0017900f2daa4mr7625496rwn.24.1708979751675; Mon, 26 Feb 2024 12:35:51 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1708979751; cv=pass; d=google.com; s=arc-20160816; b=1Dum8q1X7b5BYp7fcG107wYPFXORJaF4flMYWLNrtCAAHtSIegZoN4J7dfjk3H6U5G BrhoxiYpF9vlbm8qUDwgnVfOOFPbrUJY4MwPIbWl8XQ3G/x6t+asE+f/NNPb3BhPShLX tL1WT/UqM+5giuvhqIcIh7OVfQYp98q3bb+ZPJxPj4un2K3LrBlrVrvaTc+w3pNqsLoy 9/bgMgyW2OGpCwhkyPe6JIKn5XW18uVJY0wpHBMJPwCz7q4dolaDoDOAUhtPtvYn86+5 z9Ky/tSmVHCEOoogRoZkrWtTB1WcsqEmnbV3SyzXRXRAb1PAK/bpj3NAzkQKoG2Ez17d qvLg== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=errors-to:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:content-transfer-encoding :in-reply-to:cc:to:from:references:content-language:subject :user-agent:mime-version:date:message-id:dkim-signature:arc-filter :dmarc-filter:delivered-to; bh=zEOZ9vGY02rvqMeJi4uZYWydrosfO5Jfphzb8D8Lt+E=; fh=/MyOeInRJ/A2BzmHKho94GwX3DlheemeapqhhKcbqsc=; b=STK03U9IsMIpzMK0jJfqGLjCaqMl0BlF6TY5+WUSxbAi1HrqiK2I2fzhdHmvfsvyNi I4YYSlP7pegPBk9Tr0QgF+x6o6EBcjY6xIx94psDIEn6BnlrI46kLjuMLDiEgAiPYuKe KVGYGDl01+/XtPO8Vwu41UUGIPy43IA0c+Qz554XJw0NZasPpEi7DOXH/TZVfDbQUWx1 XMitW2e1RNY+3QTR7sYPaZTxXHj5TK+1+Z7e5ff3+d9Z3JZXuatL5hWckxf7qSJl0+Ls ukf5zTfsJCh0Mc43/JWS8w1Li3ACDp7iV52dhnh9T2fJXXjQvrsdCPl6Q2yHsWgnyuUS +E/Q==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@ibm.com header.s=pp1 header.b=axt90QtW; arc=pass (i=1); spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=REJECT sp=NONE dis=NONE) header.from=ibm.com Received: from server2.sourceware.org (server2.sourceware.org. [8.43.85.97]) by mx.google.com with ESMTPS id s7-20020ac87587000000b0042e85219bb3si2803349qtq.328.2024.02.26.12.35.51 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 26 Feb 2024 12:35:51 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) client-ip=8.43.85.97; Authentication-Results: mx.google.com; dkim=pass header.i=@ibm.com header.s=pp1 header.b=axt90QtW; arc=pass (i=1); spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=REJECT sp=NONE dis=NONE) header.from=ibm.com Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 5D6113858410 for <ouuuleilei@gmail.com>; Mon, 26 Feb 2024 20:35:51 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mx0b-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id 3FD473858C98 for <gcc-patches@gcc.gnu.org>; Mon, 26 Feb 2024 20:35:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3FD473858C98 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=linux.vnet.ibm.com Authentication-Results: sourceware.org; spf=none smtp.mailfrom=linux.vnet.ibm.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 3FD473858C98 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=1708979713; cv=none; b=imB6BO+SI9HX21ouDCU9z1qaaoe+CIyu0m5lDrfcvoZljiTs/XKR2YzSVaTp4iu+EGMz2G67q5rysUZSFsjk8gQeVY3RYmvdhHrHvGjoj5LSZnQZJNNossOz29xXGmmlTXOQ5fQjiRWIaCoDFQJEqij1/X+TfmHAsW6VktjfZH8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708979713; c=relaxed/simple; bh=AQnHIz8GMb3ItcvWKu/Dvt8Q3TO4/qJ3A2HyPj010zU=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:From:To; b=U8XMvpJ5o1nx7GzH3M3Qur39Mwk2vbsV7lBFEMmuMu5w9IUKbhpc5yv+iEwD4HdMWleIVjU60ZyDPSSykg6XPfP499NvFqr0jwQP+cqx0/kJxPVUA5IOGykUWAouzU7WYzDacBGp6ejVQnX3ruNNk4wqxwKoJVbKvAfeRDKUZpo= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from pps.filterd (m0353723.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 41QKS1c3018749; Mon, 26 Feb 2024 20:35:10 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : date : mime-version : subject : references : from : to : cc : in-reply-to : content-type : content-transfer-encoding; s=pp1; bh=zEOZ9vGY02rvqMeJi4uZYWydrosfO5Jfphzb8D8Lt+E=; b=axt90QtWw7MjqGCQy5jopFAFhv3aYYAKKYFipVxZDU9oxMoPee4CiGLZ+PEtUTLo2t8x RcriwOIzxL/Hry20rhMlSXr5D462zEKL1cvV/QBsdVTHFlJ0mEYAZ95E57xRM6vALf4g AAK6ScWTFBWiIMDh4McfKo05NEBB4Its6mCB9DHMuMXhyt0QvO1FCjsCQjGYj20zhoyM xgXPRUoGJp7m7lDH13mMIgLjcokTdFNI481ENaNXgPZrrYn82cxZfUyOf6+yAUspjpsZ 1Ktwql9+eKF7HL87zZ5WM2al8OvTTbQ8Ei+5a1BIBcOkR5iGdGYXD5LNN6j0K9HYlZ5+ /g== 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 3wh1q0r3p6-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 26 Feb 2024 20:35:09 +0000 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 41QIpDwu008792; Mon, 26 Feb 2024 20:32:48 GMT Received: from smtprelay02.wdc07v.mail.ibm.com ([172.16.1.69]) by ppma12.dal12v.mail.ibm.com (PPS) with ESMTPS id 3wftstbud3-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 26 Feb 2024 20:32:48 +0000 Received: from smtpav06.dal12v.mail.ibm.com (smtpav06.dal12v.mail.ibm.com [10.241.53.105]) by smtprelay02.wdc07v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 41QKWi5X7930550 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 26 Feb 2024 20:32:46 GMT Received: from smtpav06.dal12v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id D338B58072; Mon, 26 Feb 2024 20:32:42 +0000 (GMT) Received: from smtpav06.dal12v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 96DAA5806A; Mon, 26 Feb 2024 20:32:40 +0000 (GMT) Received: from [9.195.32.75] (unknown [9.195.32.75]) by smtpav06.dal12v.mail.ibm.com (Postfix) with ESMTP; Mon, 26 Feb 2024 20:32:40 +0000 (GMT) Message-ID: <17e8b572-e9c1-44f6-aff8-b14cda8321f9@linux.vnet.ibm.com> Date: Tue, 27 Feb 2024 02:02:38 +0530 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: [PATCH V2] rs6000: Don't allow immediate value in the vsx_splat pattern [PR113950] Content-Language: en-US References: <7faaa8f2-d706-4f6a-811f-103b74f2de8a@linux.vnet.ibm.com> From: jeevitha <jeevitha@linux.vnet.ibm.com> To: GCC Patches <gcc-patches@gcc.gnu.org>, "Kewen.Lin" <linkw@linux.ibm.com>, Segher Boessenkool <segher@kernel.crashing.org>, Michael Meissner <meissner@linux.ibm.com> Cc: Peter Bergner <bergner@linux.ibm.com> In-Reply-To: <7faaa8f2-d706-4f6a-811f-103b74f2de8a@linux.vnet.ibm.com> X-Forwarded-Message-Id: <7faaa8f2-d706-4f6a-811f-103b74f2de8a@linux.vnet.ibm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: jSruJA_5BK9cMTeTHjdWw41Cx_eZ7EsQ X-Proofpoint-GUID: jSruJA_5BK9cMTeTHjdWw41Cx_eZ7EsQ X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2024-02-26_11,2024-02-26_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 lowpriorityscore=0 impostorscore=0 mlxlogscore=912 adultscore=0 phishscore=0 mlxscore=0 malwarescore=0 spamscore=0 clxscore=1015 priorityscore=1501 bulkscore=0 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311290000 definitions=main-2402260158 X-Spam-Status: No, score=-12.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, 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 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1791941276754957043 X-GMAIL-MSGID: 1791995151837305657 |
Series |
[V2] rs6000: Don't allow immediate value in the vsx_splat pattern [PR113950]
|
|
Checks
Context | Check | Description |
---|---|---|
snail/gcc-patch-check | success | Github commit url |
Commit Message
jeevitha
Feb. 26, 2024, 8:32 p.m. UTC
Hi All, The following patch has been bootstrapped and regtested on powerpc64le-linux. There is no immediate value splatting instruction in Power. Currently, those values need to be stored in a register or memory. To address this issue, I have updated the predicate for the second operand in vsx_splat to splat_input_operand and corrected the assignment of op1 to operands[1]. These changes ensure that operand1 is stored in a register. 2024-02-26 Jeevitha Palanisamy <jeevitha@linux.ibm.com> gcc/ PR target/113950 * config/rs6000/vsx.md (vsx_splat_<mode>): Updated the predicates for second operand and corrected the assignment. gcc/testsuite/ PR target/113950 * gcc.target/powerpc/pr113950.c: New testcase.
Comments
Hi! On Tue, Feb 27, 2024 at 02:02:38AM +0530, jeevitha wrote: > There is no immediate value splatting instruction in Power. Currently, those > values need to be stored in a register or memory. To address this issue, I > have updated the predicate for the second operand in vsx_splat to > splat_input_operand and corrected the assignment of op1 to operands[1]. > These changes ensure that operand1 is stored in a register. input_operand allows a lot of things that splat_input_operand does not, not just immediate operands. NAK. (For example, *all* memory is okay for input_operand, always). I'm not saying we do not want to restrict these things, but a commit that doesn't discuss this at all is not okay. Sorry. Segher
On 2/27/24 6:40 AM, Segher Boessenkool wrote: > On Tue, Feb 27, 2024 at 02:02:38AM +0530, jeevitha wrote: >> There is no immediate value splatting instruction in Power. Currently, those >> values need to be stored in a register or memory. To address this issue, I >> have updated the predicate for the second operand in vsx_splat to >> splat_input_operand and corrected the assignment of op1 to operands[1]. >> These changes ensure that operand1 is stored in a register. > > input_operand allows a lot of things that splat_input_operand does not, > not just immediate operands. NAK. > > (For example, *all* memory is okay for input_operand, always). > > I'm not saying we do not want to restrict these things, but a commit > that doesn't discuss this at all is not okay. Sorry. So it seems you're not NAKing the use of splat_input_operand, but just that it needs more explanation in the git log entry, correct? Yes, input_operand accepts a lot more things than splat_input_operand does, but the multiple define_insns this define_expand feeds, uses gpc_reg_operand, memory_operand and splat_input_operand for their operands[1] operand (splat_input_operand accepts reg and mem too), so it seems to match better what the patterns will be accepting and I always thought that using predicates that more accurately reflect what the define_insns expect/accept lead to better code gen. Mike, was it just an oversight to not use splat_input_operand for the vsx_splat_<mode> expander or was input_operand a conscious decision? If input_operand was used purposely, then we can just fall back to the s/op1/operands[1]/ change which we already know fixes the bug. Peter
On Tue, Feb 27, 2024 at 04:50:02PM -0600, Peter Bergner wrote: > On 2/27/24 6:40 AM, Segher Boessenkool wrote: > > On Tue, Feb 27, 2024 at 02:02:38AM +0530, jeevitha wrote: > > input_operand allows a lot of things that splat_input_operand does not, > > not just immediate operands. NAK. > > > > (For example, *all* memory is okay for input_operand, always). > > > > I'm not saying we do not want to restrict these things, but a commit > > that doesn't discuss this at all is not okay. Sorry. > > So it seems you're not NAKing the use of splat_input_operand, but > just that it needs more explanation in the git log entry, correct? I NAK the patch. _Of course_ there needs to be *something* done, there is a bug after all, it needs to be fixed. But no, there are big questions about if splat_input_operand is correct as well. This needs to be justified in the patch submission. > Yes, input_operand accepts a lot more things than splat_input_operand > does, but the multiple define_insns this define_expand feeds, uses > gpc_reg_operand, memory_operand and splat_input_operand for their > operands[1] operand (splat_input_operand accepts reg and mem too), > so it seems to match better what the patterns will be accepting and > I always thought that using predicates that more accurately reflect > what the define_insns expect/accept lead to better code gen. Still, it needs explanation why we allowed it before, but that was a mistake, or for some reason we do not need it. Sell your patch! :-) > Mike, was it just an oversight to not use splat_input_operand for the > vsx_splat_<mode> expander or was input_operand a conscious decision? > > If input_operand was used purposely, then we can just fall back to > the s/op1/operands[1]/ change which we already know fixes the bug. input_operand allows all inputs for mov insns. It isn't suitable for any other instructions. Segher
On 2/28/24 8:31 AM, Segher Boessenkool wrote: > On Tue, Feb 27, 2024 at 04:50:02PM -0600, Peter Bergner wrote: >> So it seems you're not NAKing the use of splat_input_operand, but >> just that it needs more explanation in the git log entry, correct? > > I NAK the patch. _Of course_ there needs to be *something* done, there > is a bug after all, it needs to be fixed. > > But no, there are big questions about if splat_input_operand is correct > as well. This needs to be justified in the patch submission. Ok, then Jeevitha, repost the patch with the s/op1/operands[1]/ only change. Jeevitha has already bootstrapped and regtested that change and it does fix the bug. Clearly, the splat_input_operand change needs more discussion and would be a follow-on patch...if we decide to do it at all. Peter
On Wed, Feb 28, 2024 at 11:58:15AM -0600, Peter Bergner wrote: > On 2/28/24 8:31 AM, Segher Boessenkool wrote: > > On Tue, Feb 27, 2024 at 04:50:02PM -0600, Peter Bergner wrote: > >> So it seems you're not NAKing the use of splat_input_operand, but > >> just that it needs more explanation in the git log entry, correct? > > > > I NAK the patch. _Of course_ there needs to be *something* done, there > > is a bug after all, it needs to be fixed. > > > > But no, there are big questions about if splat_input_operand is correct > > as well. This needs to be justified in the patch submission. > > Ok, then Jeevitha, repost the patch with the s/op1/operands[1]/ only change. > Jeevitha has already bootstrapped and regtested that change and it does > fix the bug. > > Clearly, the splat_input_operand change needs more discussion and would > be a follow-on patch...if we decide to do it at all. It is clear that input_operand is wrong. It isn't clear that splat_input_operand is correct though :-( Segher
diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md index 6111cc90eb7..3e2df247630 100644 --- a/gcc/config/rs6000/vsx.md +++ b/gcc/config/rs6000/vsx.md @@ -4660,14 +4660,14 @@ (define_expand "vsx_splat_<mode>" [(set (match_operand:VSX_D 0 "vsx_register_operand") (vec_duplicate:VSX_D - (match_operand:<VEC_base> 1 "input_operand")))] + (match_operand:<VEC_base> 1 "splat_input_operand")))] "VECTOR_MEM_VSX_P (<MODE>mode)" { rtx op1 = operands[1]; if (MEM_P (op1)) operands[1] = rs6000_force_indexed_or_indirect_mem (op1); else if (!REG_P (op1)) - op1 = force_reg (<VSX_D:VEC_base>mode, op1); + operands[1] = force_reg (<VSX_D:VEC_base>mode, op1); }) (define_insn "vsx_splat_<mode>_reg" diff --git a/gcc/testsuite/gcc.target/powerpc/pr113950.c b/gcc/testsuite/gcc.target/powerpc/pr113950.c new file mode 100644 index 00000000000..5c6865a8544 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/pr113950.c @@ -0,0 +1,25 @@ +/* PR target/113950 */ +/* { dg-do compile } */ +/* { dg-require-effective-target powerpc_vsx_ok } */ +/* { dg-options "-O1 -mdejagnu-cpu=power7" } */ + +/* Verify we do not ICE on the following. */ + +void abort (void); + +int main () +{ + int i; + vector signed long long vsll_result, vsll_expected_result; + signed long long sll_arg1; + + sll_arg1 = 300; + vsll_expected_result = (vector signed long long) {300, 300}; + vsll_result = __builtin_vsx_splat_2di (sll_arg1); + + for (i = 0; i < 2; i++) + if (vsll_result[i] != vsll_expected_result[i]) + abort(); + + return 0; +}