diff mbox series

[06/16] arm: add smax/smin expanders for v*hf

Message ID 20230509121937.206183-6-christophe.lyon@arm.com
State Accepted
Headers
Series [01/16] arm: [MVE intrinsics] add binary_maxvminv shape |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Christophe Lyon May 9, 2023, 12:19 p.m. UTC 
  This patch adds the missing expanders for smax/smin for v*hf modes.

2022-09-08  Christophe Lyon  <christophe.lyon@arm.com>

	gcc/
	* config/arm/vec-common.md (smin<mode>3): New.
	(smax<mode>3): New.
---
 gcc/config/arm/vec-common.md | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Kyrylo Tkachov May 9, 2023, 1:48 p.m. UTC | #1 
> -----Original Message-----
> From: Christophe Lyon <christophe.lyon@arm.com>
> Sent: Tuesday, May 9, 2023 1:19 PM
> To: gcc-patches@gcc.gnu.org; Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>;
> Richard Earnshaw <Richard.Earnshaw@arm.com>; Richard Sandiford
> <Richard.Sandiford@arm.com>
> Cc: Christophe Lyon <Christophe.Lyon@arm.com>
> Subject: [PATCH 06/16] arm: add smax/smin expanders for v*hf
> 
> This patch adds the missing expanders for smax/smin for v*hf modes.
> 
> 2022-09-08  Christophe Lyon  <christophe.lyon@arm.com>
> 
> 	gcc/
> 	* config/arm/vec-common.md (smin<mode>3): New.
> 	(smax<mode>3): New.
> ---
>  gcc/config/arm/vec-common.md | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/gcc/config/arm/vec-common.md b/gcc/config/arm/vec-
> common.md
> index b5fc86fdf28..1f9b7992da4 100644
> --- a/gcc/config/arm/vec-common.md
> +++ b/gcc/config/arm/vec-common.md
> @@ -116,6 +116,13 @@ (define_expand "smin<mode>3"
>     "ARM_HAVE_<MODE>_ARITH"
>  )
> 
> +(define_expand "smin<mode>3"
> +  [(set (match_operand:VH 0 "s_register_operand")
> +	(smin:VH (match_operand:VH 1 "s_register_operand")
> +		 (match_operand:VH 2 "s_register_operand")))]
> +   "ARM_HAVE_<MODE>_ARITH"
> +)
> +
>  (define_expand "umin<mode>3"
>    [(set (match_operand:VINTW 0 "s_register_operand")
>  	(umin:VINTW (match_operand:VINTW 1 "s_register_operand")
> @@ -130,6 +137,13 @@ (define_expand "smax<mode>3"
>     "ARM_HAVE_<MODE>_ARITH"
>  )
> 
> +(define_expand "smax<mode>3"
> +  [(set (match_operand:VH 0 "s_register_operand")
> +	(smax:VH (match_operand:VH 1 "s_register_operand")
> +		 (match_operand:VH 2 "s_register_operand")))]
> +   "ARM_HAVE_<MODE>_ARITH"
> +)

We already have expanders for smin and smax, can we just extend their mode iterators to include the VH modes?
The ARM_HAVE_<MODE>_ARITH checks should still gate them properly and we could avoid adding more bloat in this file.
Thanks,
Kyrill

> +
>  (define_expand "umax<mode>3"
>    [(set (match_operand:VINTW 0 "s_register_operand")
>  	(umax:VINTW (match_operand:VINTW 1 "s_register_operand")
> --
> 2.34.1
Christophe Lyon May 9, 2023, 5:18 p.m. UTC | #2 
On 5/9/23 15:48, Kyrylo Tkachov wrote:
> 
> 
>> -----Original Message-----
>> From: Christophe Lyon <christophe.lyon@arm.com>
>> Sent: Tuesday, May 9, 2023 1:19 PM
>> To: gcc-patches@gcc.gnu.org; Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>;
>> Richard Earnshaw <Richard.Earnshaw@arm.com>; Richard Sandiford
>> <Richard.Sandiford@arm.com>
>> Cc: Christophe Lyon <Christophe.Lyon@arm.com>
>> Subject: [PATCH 06/16] arm: add smax/smin expanders for v*hf
>>
>> This patch adds the missing expanders for smax/smin for v*hf modes.
>>
>> 2022-09-08  Christophe Lyon  <christophe.lyon@arm.com>
>>
>> 	gcc/
>> 	* config/arm/vec-common.md (smin<mode>3): New.
>> 	(smax<mode>3): New.
>> ---
>>   gcc/config/arm/vec-common.md | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/gcc/config/arm/vec-common.md b/gcc/config/arm/vec-
>> common.md
>> index b5fc86fdf28..1f9b7992da4 100644
>> --- a/gcc/config/arm/vec-common.md
>> +++ b/gcc/config/arm/vec-common.md
>> @@ -116,6 +116,13 @@ (define_expand "smin<mode>3"
>>      "ARM_HAVE_<MODE>_ARITH"
>>   )
>>
>> +(define_expand "smin<mode>3"
>> +  [(set (match_operand:VH 0 "s_register_operand")
>> +	(smin:VH (match_operand:VH 1 "s_register_operand")
>> +		 (match_operand:VH 2 "s_register_operand")))]
>> +   "ARM_HAVE_<MODE>_ARITH"
>> +)
>> +
>>   (define_expand "umin<mode>3"
>>     [(set (match_operand:VINTW 0 "s_register_operand")
>>   	(umin:VINTW (match_operand:VINTW 1 "s_register_operand")
>> @@ -130,6 +137,13 @@ (define_expand "smax<mode>3"
>>      "ARM_HAVE_<MODE>_ARITH"
>>   )
>>
>> +(define_expand "smax<mode>3"
>> +  [(set (match_operand:VH 0 "s_register_operand")
>> +	(smax:VH (match_operand:VH 1 "s_register_operand")
>> +		 (match_operand:VH 2 "s_register_operand")))]
>> +   "ARM_HAVE_<MODE>_ARITH"
>> +)
> 
> We already have expanders for smin and smax, can we just extend their mode iterators to include the VH modes?
> The ARM_HAVE_<MODE>_ARITH checks should still gate them properly and we could avoid adding more bloat in this file.

I opted for the most localized changes, to avoid breaking Neon since 
there are already so many similar iterators ;-)

It seems I can just use the existing VDQWH, which seems to be VALLW (as 
already used by smax) plus V8HF/V4HF which is just what we want.

Also, ISTM that VALLW == VDQW, am I misreading?

Thanks,

Christophe


> Thanks,
> Kyrill
> 
>> +
>>   (define_expand "umax<mode>3"
>>     [(set (match_operand:VINTW 0 "s_register_operand")
>>   	(umax:VINTW (match_operand:VINTW 1 "s_register_operand")
>> --
>> 2.34.1
>
Kyrylo Tkachov May 9, 2023, 5:31 p.m. UTC | #3 
> -----Original Message-----
> From: Christophe Lyon <Christophe.Lyon@arm.com>
> Sent: Tuesday, May 9, 2023 6:18 PM
> To: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; gcc-patches@gcc.gnu.org;
> Richard Earnshaw <Richard.Earnshaw@arm.com>; Richard Sandiford
> <Richard.Sandiford@arm.com>
> Subject: Re: [PATCH 06/16] arm: add smax/smin expanders for v*hf
> 
> 
> 
> On 5/9/23 15:48, Kyrylo Tkachov wrote:
> >
> >
> >> -----Original Message-----
> >> From: Christophe Lyon <christophe.lyon@arm.com>
> >> Sent: Tuesday, May 9, 2023 1:19 PM
> >> To: gcc-patches@gcc.gnu.org; Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>;
> >> Richard Earnshaw <Richard.Earnshaw@arm.com>; Richard Sandiford
> >> <Richard.Sandiford@arm.com>
> >> Cc: Christophe Lyon <Christophe.Lyon@arm.com>
> >> Subject: [PATCH 06/16] arm: add smax/smin expanders for v*hf
> >>
> >> This patch adds the missing expanders for smax/smin for v*hf modes.
> >>
> >> 2022-09-08  Christophe Lyon  <christophe.lyon@arm.com>
> >>
> >> 	gcc/
> >> 	* config/arm/vec-common.md (smin<mode>3): New.
> >> 	(smax<mode>3): New.
> >> ---
> >>   gcc/config/arm/vec-common.md | 14 ++++++++++++++
> >>   1 file changed, 14 insertions(+)
> >>
> >> diff --git a/gcc/config/arm/vec-common.md b/gcc/config/arm/vec-
> >> common.md
> >> index b5fc86fdf28..1f9b7992da4 100644
> >> --- a/gcc/config/arm/vec-common.md
> >> +++ b/gcc/config/arm/vec-common.md
> >> @@ -116,6 +116,13 @@ (define_expand "smin<mode>3"
> >>      "ARM_HAVE_<MODE>_ARITH"
> >>   )
> >>
> >> +(define_expand "smin<mode>3"
> >> +  [(set (match_operand:VH 0 "s_register_operand")
> >> +	(smin:VH (match_operand:VH 1 "s_register_operand")
> >> +		 (match_operand:VH 2 "s_register_operand")))]
> >> +   "ARM_HAVE_<MODE>_ARITH"
> >> +)
> >> +
> >>   (define_expand "umin<mode>3"
> >>     [(set (match_operand:VINTW 0 "s_register_operand")
> >>   	(umin:VINTW (match_operand:VINTW 1 "s_register_operand")
> >> @@ -130,6 +137,13 @@ (define_expand "smax<mode>3"
> >>      "ARM_HAVE_<MODE>_ARITH"
> >>   )
> >>
> >> +(define_expand "smax<mode>3"
> >> +  [(set (match_operand:VH 0 "s_register_operand")
> >> +	(smax:VH (match_operand:VH 1 "s_register_operand")
> >> +		 (match_operand:VH 2 "s_register_operand")))]
> >> +   "ARM_HAVE_<MODE>_ARITH"
> >> +)
> >
> > We already have expanders for smin and smax, can we just extend their
> mode iterators to include the VH modes?
> > The ARM_HAVE_<MODE>_ARITH checks should still gate them properly and
> we could avoid adding more bloat in this file.
> 
> I opted for the most localized changes, to avoid breaking Neon since
> there are already so many similar iterators ;-)
> 
> It seems I can just use the existing VDQWH, which seems to be VALLW (as
> already used by smax) plus V8HF/V4HF which is just what we want.

Yes, let's use that.

> 
> Also, ISTM that VALLW == VDQW, am I misreading?

They do look the same. I'm generally okay with removing duplicate iterators unless their name seems to have a very specific meaning that would be confusing in other contexts.
But VALLW and VDQW seem equally confusing 😉
Thanks,
KYrill

> 
> Thanks,
> 
> Christophe
> 
> 
> > Thanks,
> > Kyrill
> >
> >> +
> >>   (define_expand "umax<mode>3"
> >>     [(set (match_operand:VINTW 0 "s_register_operand")
> >>   	(umax:VINTW (match_operand:VINTW 1 "s_register_operand")
> >> --
> >> 2.34.1
> >
Christophe Lyon May 9, 2023, 5:33 p.m. UTC | #4 
On 5/9/23 19:31, Kyrylo Tkachov wrote:
> 
> 
>> -----Original Message-----
>> From: Christophe Lyon <Christophe.Lyon@arm.com>
>> Sent: Tuesday, May 9, 2023 6:18 PM
>> To: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; gcc-patches@gcc.gnu.org;
>> Richard Earnshaw <Richard.Earnshaw@arm.com>; Richard Sandiford
>> <Richard.Sandiford@arm.com>
>> Subject: Re: [PATCH 06/16] arm: add smax/smin expanders for v*hf
>>
>>
>>
>> On 5/9/23 15:48, Kyrylo Tkachov wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Christophe Lyon <christophe.lyon@arm.com>
>>>> Sent: Tuesday, May 9, 2023 1:19 PM
>>>> To: gcc-patches@gcc.gnu.org; Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>;
>>>> Richard Earnshaw <Richard.Earnshaw@arm.com>; Richard Sandiford
>>>> <Richard.Sandiford@arm.com>
>>>> Cc: Christophe Lyon <Christophe.Lyon@arm.com>
>>>> Subject: [PATCH 06/16] arm: add smax/smin expanders for v*hf
>>>>
>>>> This patch adds the missing expanders for smax/smin for v*hf modes.
>>>>
>>>> 2022-09-08  Christophe Lyon  <christophe.lyon@arm.com>
>>>>
>>>> 	gcc/
>>>> 	* config/arm/vec-common.md (smin<mode>3): New.
>>>> 	(smax<mode>3): New.
>>>> ---
>>>>    gcc/config/arm/vec-common.md | 14 ++++++++++++++
>>>>    1 file changed, 14 insertions(+)
>>>>
>>>> diff --git a/gcc/config/arm/vec-common.md b/gcc/config/arm/vec-
>>>> common.md
>>>> index b5fc86fdf28..1f9b7992da4 100644
>>>> --- a/gcc/config/arm/vec-common.md
>>>> +++ b/gcc/config/arm/vec-common.md
>>>> @@ -116,6 +116,13 @@ (define_expand "smin<mode>3"
>>>>       "ARM_HAVE_<MODE>_ARITH"
>>>>    )
>>>>
>>>> +(define_expand "smin<mode>3"
>>>> +  [(set (match_operand:VH 0 "s_register_operand")
>>>> +	(smin:VH (match_operand:VH 1 "s_register_operand")
>>>> +		 (match_operand:VH 2 "s_register_operand")))]
>>>> +   "ARM_HAVE_<MODE>_ARITH"
>>>> +)
>>>> +
>>>>    (define_expand "umin<mode>3"
>>>>      [(set (match_operand:VINTW 0 "s_register_operand")
>>>>    	(umin:VINTW (match_operand:VINTW 1 "s_register_operand")
>>>> @@ -130,6 +137,13 @@ (define_expand "smax<mode>3"
>>>>       "ARM_HAVE_<MODE>_ARITH"
>>>>    )
>>>>
>>>> +(define_expand "smax<mode>3"
>>>> +  [(set (match_operand:VH 0 "s_register_operand")
>>>> +	(smax:VH (match_operand:VH 1 "s_register_operand")
>>>> +		 (match_operand:VH 2 "s_register_operand")))]
>>>> +   "ARM_HAVE_<MODE>_ARITH"
>>>> +)
>>>
>>> We already have expanders for smin and smax, can we just extend their
>> mode iterators to include the VH modes?
>>> The ARM_HAVE_<MODE>_ARITH checks should still gate them properly and
>> we could avoid adding more bloat in this file.
>>
>> I opted for the most localized changes, to avoid breaking Neon since
>> there are already so many similar iterators ;-)
>>
>> It seems I can just use the existing VDQWH, which seems to be VALLW (as
>> already used by smax) plus V8HF/V4HF which is just what we want.
> 
> Yes, let's use that.
> 
Thanks, I'll do that. OK to push with change, or do you want me to post 
the updated patch?

>>
>> Also, ISTM that VALLW == VDQW, am I misreading?
> 
> They do look the same. I'm generally okay with removing duplicate iterators unless their name seems to have a very specific meaning that would be confusing in other contexts.
> But VALLW and VDQW seem equally confusing 😉
Thanks for confirming the "confusing" bit :-)

Christophe

> Thanks,
> KYrill
> 
>>
>> Thanks,
>>
>> Christophe
>>
>>
>>> Thanks,
>>> Kyrill
>>>
>>>> +
>>>>    (define_expand "umax<mode>3"
>>>>      [(set (match_operand:VINTW 0 "s_register_operand")
>>>>    	(umax:VINTW (match_operand:VINTW 1 "s_register_operand")
>>>> --
>>>> 2.34.1
>>>
Kyrylo Tkachov May 9, 2023, 5:34 p.m. UTC | #5 
> -----Original Message-----
> From: Christophe Lyon <Christophe.Lyon@arm.com>
> Sent: Tuesday, May 9, 2023 6:33 PM
> To: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; gcc-patches@gcc.gnu.org;
> Richard Earnshaw <Richard.Earnshaw@arm.com>; Richard Sandiford
> <Richard.Sandiford@arm.com>
> Subject: Re: [PATCH 06/16] arm: add smax/smin expanders for v*hf
> 
> 
> 
> On 5/9/23 19:31, Kyrylo Tkachov wrote:
> >
> >
> >> -----Original Message-----
> >> From: Christophe Lyon <Christophe.Lyon@arm.com>
> >> Sent: Tuesday, May 9, 2023 6:18 PM
> >> To: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; gcc-patches@gcc.gnu.org;
> >> Richard Earnshaw <Richard.Earnshaw@arm.com>; Richard Sandiford
> >> <Richard.Sandiford@arm.com>
> >> Subject: Re: [PATCH 06/16] arm: add smax/smin expanders for v*hf
> >>
> >>
> >>
> >> On 5/9/23 15:48, Kyrylo Tkachov wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Christophe Lyon <christophe.lyon@arm.com>
> >>>> Sent: Tuesday, May 9, 2023 1:19 PM
> >>>> To: gcc-patches@gcc.gnu.org; Kyrylo Tkachov
> <Kyrylo.Tkachov@arm.com>;
> >>>> Richard Earnshaw <Richard.Earnshaw@arm.com>; Richard Sandiford
> >>>> <Richard.Sandiford@arm.com>
> >>>> Cc: Christophe Lyon <Christophe.Lyon@arm.com>
> >>>> Subject: [PATCH 06/16] arm: add smax/smin expanders for v*hf
> >>>>
> >>>> This patch adds the missing expanders for smax/smin for v*hf modes.
> >>>>
> >>>> 2022-09-08  Christophe Lyon  <christophe.lyon@arm.com>
> >>>>
> >>>> 	gcc/
> >>>> 	* config/arm/vec-common.md (smin<mode>3): New.
> >>>> 	(smax<mode>3): New.
> >>>> ---
> >>>>    gcc/config/arm/vec-common.md | 14 ++++++++++++++
> >>>>    1 file changed, 14 insertions(+)
> >>>>
> >>>> diff --git a/gcc/config/arm/vec-common.md b/gcc/config/arm/vec-
> >>>> common.md
> >>>> index b5fc86fdf28..1f9b7992da4 100644
> >>>> --- a/gcc/config/arm/vec-common.md
> >>>> +++ b/gcc/config/arm/vec-common.md
> >>>> @@ -116,6 +116,13 @@ (define_expand "smin<mode>3"
> >>>>       "ARM_HAVE_<MODE>_ARITH"
> >>>>    )
> >>>>
> >>>> +(define_expand "smin<mode>3"
> >>>> +  [(set (match_operand:VH 0 "s_register_operand")
> >>>> +	(smin:VH (match_operand:VH 1 "s_register_operand")
> >>>> +		 (match_operand:VH 2 "s_register_operand")))]
> >>>> +   "ARM_HAVE_<MODE>_ARITH"
> >>>> +)
> >>>> +
> >>>>    (define_expand "umin<mode>3"
> >>>>      [(set (match_operand:VINTW 0 "s_register_operand")
> >>>>    	(umin:VINTW (match_operand:VINTW 1 "s_register_operand")
> >>>> @@ -130,6 +137,13 @@ (define_expand "smax<mode>3"
> >>>>       "ARM_HAVE_<MODE>_ARITH"
> >>>>    )
> >>>>
> >>>> +(define_expand "smax<mode>3"
> >>>> +  [(set (match_operand:VH 0 "s_register_operand")
> >>>> +	(smax:VH (match_operand:VH 1 "s_register_operand")
> >>>> +		 (match_operand:VH 2 "s_register_operand")))]
> >>>> +   "ARM_HAVE_<MODE>_ARITH"
> >>>> +)
> >>>
> >>> We already have expanders for smin and smax, can we just extend their
> >> mode iterators to include the VH modes?
> >>> The ARM_HAVE_<MODE>_ARITH checks should still gate them properly
> and
> >> we could avoid adding more bloat in this file.
> >>
> >> I opted for the most localized changes, to avoid breaking Neon since
> >> there are already so many similar iterators ;-)
> >>
> >> It seems I can just use the existing VDQWH, which seems to be VALLW (as
> >> already used by smax) plus V8HF/V4HF which is just what we want.
> >
> > Yes, let's use that.
> >
> Thanks, I'll do that. OK to push with change, or do you want me to post
> the updated patch?

Please post the updated patch for archival on the list and commit the series.
Thanks,
Kyrill

> 
> >>
> >> Also, ISTM that VALLW == VDQW, am I misreading?
> >
> > They do look the same. I'm generally okay with removing duplicate iterators
> unless their name seems to have a very specific meaning that would be
> confusing in other contexts.
> > But VALLW and VDQW seem equally confusing 😉
> Thanks for confirming the "confusing" bit :-)
> 
> Christophe
> 
> > Thanks,
> > KYrill
> >
> >>
> >> Thanks,
> >>
> >> Christophe
> >>
> >>
> >>> Thanks,
> >>> Kyrill
> >>>
> >>>> +
> >>>>    (define_expand "umax<mode>3"
> >>>>      [(set (match_operand:VINTW 0 "s_register_operand")
> >>>>    	(umax:VINTW (match_operand:VINTW 1 "s_register_operand")
> >>>> --
> >>>> 2.34.1
> >>>
diff mbox series

Patch

diff --git a/gcc/config/arm/vec-common.md b/gcc/config/arm/vec-common.md
index b5fc86fdf28..1f9b7992da4 100644
--- a/gcc/config/arm/vec-common.md
+++ b/gcc/config/arm/vec-common.md
@@ -116,6 +116,13 @@  (define_expand "smin<mode>3"
    "ARM_HAVE_<MODE>_ARITH"
 )
 
+(define_expand "smin<mode>3"
+  [(set (match_operand:VH 0 "s_register_operand")
+	(smin:VH (match_operand:VH 1 "s_register_operand")
+		 (match_operand:VH 2 "s_register_operand")))]
+   "ARM_HAVE_<MODE>_ARITH"
+)
+
 (define_expand "umin<mode>3"
   [(set (match_operand:VINTW 0 "s_register_operand")
 	(umin:VINTW (match_operand:VINTW 1 "s_register_operand")
@@ -130,6 +137,13 @@  (define_expand "smax<mode>3"
    "ARM_HAVE_<MODE>_ARITH"
 )
 
+(define_expand "smax<mode>3"
+  [(set (match_operand:VH 0 "s_register_operand")
+	(smax:VH (match_operand:VH 1 "s_register_operand")
+		 (match_operand:VH 2 "s_register_operand")))]
+   "ARM_HAVE_<MODE>_ARITH"
+)
+
 (define_expand "umax<mode>3"
   [(set (match_operand:VINTW 0 "s_register_operand")
 	(umax:VINTW (match_operand:VINTW 1 "s_register_operand")