Message ID | 20220113145645.4077141-7-christophe.lyon@foss.st.com |
---|---|
State | Superseded |
Commit | 0d0aaea105f6b5ddd9b4763e4cbd16ef65a74cb9 |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 7720F39518B5 for <patchwork@sourceware.org>; Thu, 13 Jan 2022 15:04:41 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7720F39518B5 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1642086281; bh=/YISRsGD3kNKNIjvVDekLofhnhWuHkw0pU6c7YFDvM0=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=iNVL33fcNQrA6i/WH0TRoVc+II9NPPZGhNKHyHZg01GQIHny0nBMizJ7+6ara5zej Ycl8JL9YNCepScPgrIPOz+PLnWV8r/v8gWK6cmyEUElf2ym68+N1lqDJF8C65ocRtE bX/x2gOyd3dBuqZ9wTVDKAW1uqZzWXmwFQIHcnig= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mx07-00178001.pphosted.com (mx07-00178001.pphosted.com [185.132.182.106]) by sourceware.org (Postfix) with ESMTPS id DC51F3951C37 for <gcc-patches@gcc.gnu.org>; Thu, 13 Jan 2022 14:59:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DC51F3951C37 Received: from pps.filterd (m0241204.ppops.net [127.0.0.1]) by mx07-00178001.pphosted.com (8.16.1.2/8.16.1.2) with ESMTP id 20D9w2tg031850 for <gcc-patches@gcc.gnu.org>; Thu, 13 Jan 2022 15:59:15 +0100 Received: from beta.dmz-eu.st.com (beta.dmz-eu.st.com [164.129.1.35]) by mx07-00178001.pphosted.com (PPS) with ESMTPS id 3dj25b5ajb-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for <gcc-patches@gcc.gnu.org>; Thu, 13 Jan 2022 15:59:15 +0100 Received: from euls16034.sgp.st.com (euls16034.sgp.st.com [10.75.44.20]) by beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 0886110002A for <gcc-patches@gcc.gnu.org>; Thu, 13 Jan 2022 15:59:15 +0100 (CET) Received: from Webmail-eu.st.com (sfhdag2node2.st.com [10.75.127.5]) by euls16034.sgp.st.com (STMicroelectronics) with ESMTP id F3EAE21423B for <gcc-patches@gcc.gnu.org>; Thu, 13 Jan 2022 15:59:14 +0100 (CET) Received: from gnx2104.gnb.st.com (10.75.127.47) by SFHDAG2NODE2.st.com (10.75.127.5) with Microsoft SMTP Server (TLS) id 15.0.1497.26; Thu, 13 Jan 2022 15:59:14 +0100 To: <gcc-patches@gcc.gnu.org> Subject: [PATCH v3 06/15] arm: Fix mve_vmvnq_n_<supf><mode> argument mode Date: Thu, 13 Jan 2022 15:56:16 +0100 Message-ID: <20220113145645.4077141-7-christophe.lyon@foss.st.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20220113145645.4077141-1-christophe.lyon@foss.st.com> References: <20220113145645.4077141-1-christophe.lyon@foss.st.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-Originating-IP: [10.75.127.47] X-ClientProxiedBy: SFHDAG2NODE2.st.com (10.75.127.5) To SFHDAG2NODE2.st.com (10.75.127.5) X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.816,Hydra:6.0.425,FMLib:17.11.62.513 definitions=2022-01-13_07,2022-01-13_01,2021-12-02_01 X-Spam-Status: No, score=-8.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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 <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> From: Christophe Lyon via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Christophe Lyon <christophe.lyon@foss.st.com> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
ARM/MVE use vectors of boolean for predicates
|
|
Commit Message
Christophe Lyon
Jan. 13, 2022, 2:56 p.m. UTC
The vmvnq_n* intrinsics and have [u]int[16|32]_t arguments, so use <V_elem> iterator instead of HI in mve_vmvnq_n_<supf><mode>. 2022-01-13 Christophe Lyon <christophe.lyon@foss.st.com> gcc/ * config/arm/mve.md (mve_vmvnq_n_<supf><mode>): Use V_elem mode for operand 1.
Comments
On 13/01/2022 14:56, Christophe Lyon via Gcc-patches wrote: > The vmvnq_n* intrinsics and have [u]int[16|32]_t arguments, so use > <V_elem> iterator instead of HI in mve_vmvnq_n_<supf><mode>. > > 2022-01-13 Christophe Lyon <christophe.lyon@foss.st.com> > > gcc/ > * config/arm/mve.md (mve_vmvnq_n_<supf><mode>): Use V_elem mode > for operand 1. > > diff --git a/gcc/config/arm/mve.md b/gcc/config/arm/mve.md > index 171dd384133..5c3b34dce3a 100644 > --- a/gcc/config/arm/mve.md > +++ b/gcc/config/arm/mve.md > @@ -617,7 +617,7 @@ (define_insn "mve_vcvtaq_<supf><mode>" > (define_insn "mve_vmvnq_n_<supf><mode>" > [ > (set (match_operand:MVE_5 0 "s_register_operand" "=w") > - (unspec:MVE_5 [(match_operand:HI 1 "immediate_operand" "i")] > + (unspec:MVE_5 [(match_operand:<V_elem> 1 "immediate_operand" "i")] > VMVNQ_N)) > ] > "TARGET_HAVE_MVE" While fixing this it might be good to fix the constraint and predicate inspired by "DL" and "neon_inv_logic_op2" respectively. This would avoid the compiler generating wrong assembly, and instead it would probably lead to the compiler using a load literal. I kind of think it would be better to have the intrinsic refuse the immediate altogether, but it seems for NEON we also use the load literal approach.
On Wed, Jan 19, 2022 at 8:03 PM Andre Vieira (lists) via Gcc-patches < gcc-patches@gcc.gnu.org> wrote: > > On 13/01/2022 14:56, Christophe Lyon via Gcc-patches wrote: > > The vmvnq_n* intrinsics and have [u]int[16|32]_t arguments, so use > > <V_elem> iterator instead of HI in mve_vmvnq_n_<supf><mode>. > > > > 2022-01-13 Christophe Lyon <christophe.lyon@foss.st.com> > > > > gcc/ > > * config/arm/mve.md (mve_vmvnq_n_<supf><mode>): Use V_elem mode > > for operand 1. > > > > diff --git a/gcc/config/arm/mve.md b/gcc/config/arm/mve.md > > index 171dd384133..5c3b34dce3a 100644 > > --- a/gcc/config/arm/mve.md > > +++ b/gcc/config/arm/mve.md > > @@ -617,7 +617,7 @@ (define_insn "mve_vcvtaq_<supf><mode>" > > (define_insn "mve_vmvnq_n_<supf><mode>" > > [ > > (set (match_operand:MVE_5 0 "s_register_operand" "=w") > > - (unspec:MVE_5 [(match_operand:HI 1 "immediate_operand" "i")] > > + (unspec:MVE_5 [(match_operand:<V_elem> 1 "immediate_operand" "i")] > > VMVNQ_N)) > > ] > > "TARGET_HAVE_MVE" > > While fixing this it might be good to fix the constraint and predicate > inspired by "DL" and "neon_inv_logic_op2" respectively. This would avoid > the compiler generating wrong assembly, and instead it would probably > lead to the compiler using a load literal. > > I kind of think it would be better to have the intrinsic refuse the > immediate altogether, but it seems for NEON we also use the load literal > approach. > > Ha, I thought that patch had been approved at v2 too: https://gcc.gnu.org/pipermail/gcc-patches/2021-October/581344.html
On 20/01/2022 09:23, Christophe Lyon wrote: > > > On Wed, Jan 19, 2022 at 8:03 PM Andre Vieira (lists) via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > On 13/01/2022 14:56, Christophe Lyon via Gcc-patches wrote: > > The vmvnq_n* intrinsics and have [u]int[16|32]_t arguments, so use > > <V_elem> iterator instead of HI in mve_vmvnq_n_<supf><mode>. > > > > 2022-01-13 Christophe Lyon <christophe.lyon@foss.st.com> > > > > gcc/ > > * config/arm/mve.md (mve_vmvnq_n_<supf><mode>): Use V_elem > mode > > for operand 1. > > > > diff --git a/gcc/config/arm/mve.md b/gcc/config/arm/mve.md > > index 171dd384133..5c3b34dce3a 100644 > > --- a/gcc/config/arm/mve.md > > +++ b/gcc/config/arm/mve.md > > @@ -617,7 +617,7 @@ (define_insn "mve_vcvtaq_<supf><mode>" > > (define_insn "mve_vmvnq_n_<supf><mode>" > > [ > > (set (match_operand:MVE_5 0 "s_register_operand" "=w") > > - (unspec:MVE_5 [(match_operand:HI 1 "immediate_operand" "i")] > > + (unspec:MVE_5 [(match_operand:<V_elem> 1 > "immediate_operand" "i")] > > VMVNQ_N)) > > ] > > "TARGET_HAVE_MVE" > > While fixing this it might be good to fix the constraint and > predicate > inspired by "DL" and "neon_inv_logic_op2" respectively. This would > avoid > the compiler generating wrong assembly, and instead it would probably > lead to the compiler using a load literal. > > I kind of think it would be better to have the intrinsic refuse the > immediate altogether, but it seems for NEON we also use the load > literal > approach. > > > Ha, I thought that patch had been approved at v2 too: > https://gcc.gnu.org/pipermail/gcc-patches/2021-October/581344.html > Yeah sorry I had not looked at the previous version of these series! I can put together a follow-up for this then.
On Thu, Jan 20, 2022 at 10:38 AM Andre Simoes Dias Vieira < andre.simoesdiasvieira@arm.com> wrote: > > On 20/01/2022 09:23, Christophe Lyon wrote: > > > > On Wed, Jan 19, 2022 at 8:03 PM Andre Vieira (lists) via Gcc-patches < > gcc-patches@gcc.gnu.org> wrote: > >> >> On 13/01/2022 14:56, Christophe Lyon via Gcc-patches wrote: >> > The vmvnq_n* intrinsics and have [u]int[16|32]_t arguments, so use >> > <V_elem> iterator instead of HI in mve_vmvnq_n_<supf><mode>. >> > >> > 2022-01-13 Christophe Lyon <christophe.lyon@foss.st.com> >> > >> > gcc/ >> > * config/arm/mve.md (mve_vmvnq_n_<supf><mode>): Use V_elem mode >> > for operand 1. >> > >> > diff --git a/gcc/config/arm/mve.md b/gcc/config/arm/mve.md >> > index 171dd384133..5c3b34dce3a 100644 >> > --- a/gcc/config/arm/mve.md >> > +++ b/gcc/config/arm/mve.md >> > @@ -617,7 +617,7 @@ (define_insn "mve_vcvtaq_<supf><mode>" >> > (define_insn "mve_vmvnq_n_<supf><mode>" >> > [ >> > (set (match_operand:MVE_5 0 "s_register_operand" "=w") >> > - (unspec:MVE_5 [(match_operand:HI 1 "immediate_operand" "i")] >> > + (unspec:MVE_5 [(match_operand:<V_elem> 1 "immediate_operand" "i")] >> > VMVNQ_N)) >> > ] >> > "TARGET_HAVE_MVE" >> >> While fixing this it might be good to fix the constraint and predicate >> inspired by "DL" and "neon_inv_logic_op2" respectively. This would avoid >> the compiler generating wrong assembly, and instead it would probably >> lead to the compiler using a load literal. >> >> I kind of think it would be better to have the intrinsic refuse the >> immediate altogether, but it seems for NEON we also use the load literal >> approach. >> >> > Ha, I thought that patch had been approved at v2 too: > https://gcc.gnu.org/pipermail/gcc-patches/2021-October/581344.html > > Yeah sorry I had not looked at the previous version of these series! > > I can put together a follow-up for this then. > No problem, thanks!
"Andre Vieira (lists)" <andre.simoesdiasvieira@arm.com> writes: > On 13/01/2022 14:56, Christophe Lyon via Gcc-patches wrote: >> The vmvnq_n* intrinsics and have [u]int[16|32]_t arguments, so use >> <V_elem> iterator instead of HI in mve_vmvnq_n_<supf><mode>. >> >> 2022-01-13 Christophe Lyon <christophe.lyon@foss.st.com> >> >> gcc/ >> * config/arm/mve.md (mve_vmvnq_n_<supf><mode>): Use V_elem mode >> for operand 1. >> >> diff --git a/gcc/config/arm/mve.md b/gcc/config/arm/mve.md >> index 171dd384133..5c3b34dce3a 100644 >> --- a/gcc/config/arm/mve.md >> +++ b/gcc/config/arm/mve.md >> @@ -617,7 +617,7 @@ (define_insn "mve_vcvtaq_<supf><mode>" >> (define_insn "mve_vmvnq_n_<supf><mode>" >> [ >> (set (match_operand:MVE_5 0 "s_register_operand" "=w") >> - (unspec:MVE_5 [(match_operand:HI 1 "immediate_operand" "i")] >> + (unspec:MVE_5 [(match_operand:<V_elem> 1 "immediate_operand" "i")] >> VMVNQ_N)) >> ] >> "TARGET_HAVE_MVE" > > While fixing this it might be good to fix the constraint and predicate > inspired by "DL" and "neon_inv_logic_op2" respectively. This would avoid > the compiler generating wrong assembly, and instead it would probably > lead to the compiler using a load literal. FWIW: for cases like this, I think it's better to define a predicate only (not a constraint). By design, the only time that constraints are used independently of predicates is during RA, and there's nothing that RA can/should do for immediate operands. Thanks, Richard
On 20/01/2022 10:45, Richard Sandiford wrote: > "Andre Vieira (lists)" <andre.simoesdiasvieira@arm.com> writes: >> On 13/01/2022 14:56, Christophe Lyon via Gcc-patches wrote: >>> The vmvnq_n* intrinsics and have [u]int[16|32]_t arguments, so use >>> <V_elem> iterator instead of HI in mve_vmvnq_n_<supf><mode>. >>> >>> 2022-01-13 Christophe Lyon <christophe.lyon@foss.st.com> >>> >>> gcc/ >>> * config/arm/mve.md (mve_vmvnq_n_<supf><mode>): Use V_elem mode >>> for operand 1. >>> >>> diff --git a/gcc/config/arm/mve.md b/gcc/config/arm/mve.md >>> index 171dd384133..5c3b34dce3a 100644 >>> --- a/gcc/config/arm/mve.md >>> +++ b/gcc/config/arm/mve.md >>> @@ -617,7 +617,7 @@ (define_insn "mve_vcvtaq_<supf><mode>" >>> (define_insn "mve_vmvnq_n_<supf><mode>" >>> [ >>> (set (match_operand:MVE_5 0 "s_register_operand" "=w") >>> - (unspec:MVE_5 [(match_operand:HI 1 "immediate_operand" "i")] >>> + (unspec:MVE_5 [(match_operand:<V_elem> 1 "immediate_operand" "i")] >>> VMVNQ_N)) >>> ] >>> "TARGET_HAVE_MVE" >> While fixing this it might be good to fix the constraint and predicate >> inspired by "DL" and "neon_inv_logic_op2" respectively. This would avoid >> the compiler generating wrong assembly, and instead it would probably >> lead to the compiler using a load literal. > FWIW: for cases like this, I think it's better to define a predicate > only (not a constraint). By design, the only time that constraints > are used independently of predicates is during RA, and there's nothing > that RA can/should do for immediate operands. > > Thanks, > Richard Yeah, if I use a predicate it doesn't like the fact that we are passing an argument 'imm' rather than actual immediate. To use a constraint like DL I'd also need to change the builtin to take a vector of immediates, since we can't use immediates as they don't have a mode and the constraint needs to be able to know what mode we are using. This will have to wait...
diff --git a/gcc/config/arm/mve.md b/gcc/config/arm/mve.md index 171dd384133..5c3b34dce3a 100644 --- a/gcc/config/arm/mve.md +++ b/gcc/config/arm/mve.md @@ -617,7 +617,7 @@ (define_insn "mve_vcvtaq_<supf><mode>" (define_insn "mve_vmvnq_n_<supf><mode>" [ (set (match_operand:MVE_5 0 "s_register_operand" "=w") - (unspec:MVE_5 [(match_operand:HI 1 "immediate_operand" "i")] + (unspec:MVE_5 [(match_operand:<V_elem> 1 "immediate_operand" "i")] VMVNQ_N)) ] "TARGET_HAVE_MVE"