diff mbox series

[15/20] arm: [MVE intrinsics] add unary_acc shape

Message ID 20230510133036.596530-15-christophe.lyon@arm.com
State Committed
Commit 7e3c2d23cfdc479d5022fa4de56841bd032e915b
Headers
Series [01/20] arm: [MVE intrinsics] factorize vcmp |

Commit Message

Christophe Lyon May 10, 2023, 1:30 p.m. UTC 
  This patch adds the unary_acc shape description.

2022-10-25  Christophe Lyon  <christophe.lyon@arm.com>

	gcc/
	* config/arm/arm-mve-builtins-shapes.cc (unary_acc): New.
	* config/arm/arm-mve-builtins-shapes.h (unary_acc): New.
---
 gcc/config/arm/arm-mve-builtins-shapes.cc | 28 +++++++++++++++++++++++
 gcc/config/arm/arm-mve-builtins-shapes.h  |  1 +
 2 files changed, 29 insertions(+)

Comments

Kyrylo Tkachov May 10, 2023, 2:52 p.m. UTC | #1 
> -----Original Message-----
> From: Christophe Lyon <christophe.lyon@arm.com>
> Sent: Wednesday, May 10, 2023 2:31 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 15/20] arm: [MVE intrinsics] add unary_acc shape
> 
> This patch adds the unary_acc shape description.
> 
> 2022-10-25  Christophe Lyon  <christophe.lyon@arm.com>
> 
> 	gcc/
> 	* config/arm/arm-mve-builtins-shapes.cc (unary_acc): New.
> 	* config/arm/arm-mve-builtins-shapes.h (unary_acc): New.
> ---
>  gcc/config/arm/arm-mve-builtins-shapes.cc | 28 +++++++++++++++++++++++
>  gcc/config/arm/arm-mve-builtins-shapes.h  |  1 +
>  2 files changed, 29 insertions(+)
> 
> diff --git a/gcc/config/arm/arm-mve-builtins-shapes.cc b/gcc/config/arm/arm-
> mve-builtins-shapes.cc
> index bff1c3e843b..e77a0cc20ac 100644
> --- a/gcc/config/arm/arm-mve-builtins-shapes.cc
> +++ b/gcc/config/arm/arm-mve-builtins-shapes.cc
> @@ -1066,6 +1066,34 @@ struct unary_def : public overloaded_base<0>
>  };
>  SHAPE (unary)
> 
> +/* <S0:twice>_t vfoo[_<t0>](<T0>_t)
> +
> +   i.e. a version of "unary" in which the source elements are half the
> +   size of the destination scalar, but have the same type class.
> +
> +   Example: vaddlvq.
> +   int64_t [__arm_]vaddlvq[_s32](int32x4_t a)
> +   int64_t [__arm_]vaddlvq_p[_s32](int32x4_t a, mve_pred16_t p) */
> +struct unary_acc_def : public overloaded_base<0>
> +{
> +  void
> +  build (function_builder &b, const function_group_info &group,
> +	 bool preserve_user_namespace) const override
> +  {
> +    b.add_overloaded_functions (group, MODE_none,
> preserve_user_namespace);
> +    build_all (b, "sw0,v0", group, MODE_none, preserve_user_namespace);
> +  }
> +
> +  tree
> +  resolve (function_resolver &r) const override
> +  {
> +    /* FIXME: check that the return value is actually
> +       twice as wide as arg 0.  */

Any reason why we can't add that check now?
I'd rather not add new FIXMEs here...
Thanks,
Kyrill

> +    return r.resolve_unary ();
> +  }
> +};
> +SHAPE (unary_acc)
> +
>  /* <T0>_t foo_t0[_t1](<T1>_t)
> 
>     where the target type <t0> must be specified explicitly but the source
> diff --git a/gcc/config/arm/arm-mve-builtins-shapes.h b/gcc/config/arm/arm-
> mve-builtins-shapes.h
> index fc1bacbd4da..c062fe624c4 100644
> --- a/gcc/config/arm/arm-mve-builtins-shapes.h
> +++ b/gcc/config/arm/arm-mve-builtins-shapes.h
> @@ -53,6 +53,7 @@ namespace arm_mve
>      extern const function_shape *const create;
>      extern const function_shape *const inherent;
>      extern const function_shape *const unary;
> +    extern const function_shape *const unary_acc;
>      extern const function_shape *const unary_convert;
>      extern const function_shape *const unary_int32;
>      extern const function_shape *const unary_int32_acc;
> --
> 2.34.1
Christophe Lyon May 11, 2023, 8:21 a.m. UTC | #2 
On 5/10/23 16:52, Kyrylo Tkachov wrote:
> 
> 
>> -----Original Message-----
>> From: Christophe Lyon <christophe.lyon@arm.com>
>> Sent: Wednesday, May 10, 2023 2:31 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 15/20] arm: [MVE intrinsics] add unary_acc shape
>>
>> This patch adds the unary_acc shape description.
>>
>> 2022-10-25  Christophe Lyon  <christophe.lyon@arm.com>
>>
>> 	gcc/
>> 	* config/arm/arm-mve-builtins-shapes.cc (unary_acc): New.
>> 	* config/arm/arm-mve-builtins-shapes.h (unary_acc): New.
>> ---
>>   gcc/config/arm/arm-mve-builtins-shapes.cc | 28 +++++++++++++++++++++++
>>   gcc/config/arm/arm-mve-builtins-shapes.h  |  1 +
>>   2 files changed, 29 insertions(+)
>>
>> diff --git a/gcc/config/arm/arm-mve-builtins-shapes.cc b/gcc/config/arm/arm-
>> mve-builtins-shapes.cc
>> index bff1c3e843b..e77a0cc20ac 100644
>> --- a/gcc/config/arm/arm-mve-builtins-shapes.cc
>> +++ b/gcc/config/arm/arm-mve-builtins-shapes.cc
>> @@ -1066,6 +1066,34 @@ struct unary_def : public overloaded_base<0>
>>   };
>>   SHAPE (unary)
>>
>> +/* <S0:twice>_t vfoo[_<t0>](<T0>_t)
>> +
>> +   i.e. a version of "unary" in which the source elements are half the
>> +   size of the destination scalar, but have the same type class.
>> +
>> +   Example: vaddlvq.
>> +   int64_t [__arm_]vaddlvq[_s32](int32x4_t a)
>> +   int64_t [__arm_]vaddlvq_p[_s32](int32x4_t a, mve_pred16_t p) */
>> +struct unary_acc_def : public overloaded_base<0>
>> +{
>> +  void
>> +  build (function_builder &b, const function_group_info &group,
>> +	 bool preserve_user_namespace) const override
>> +  {
>> +    b.add_overloaded_functions (group, MODE_none,
>> preserve_user_namespace);
>> +    build_all (b, "sw0,v0", group, MODE_none, preserve_user_namespace);
>> +  }
>> +
>> +  tree
>> +  resolve (function_resolver &r) const override
>> +  {
>> +    /* FIXME: check that the return value is actually
>> +       twice as wide as arg 0.  */
> 
> Any reason why we can't add that check now?
> I'd rather not add new FIXMEs here...

I understand :-)

That's because the resolver only knows about the arguments, not the 
return value:
   /* The arguments to the overloaded function.  */
   vec<tree, va_gc> &m_arglist;

I kept this like what already exists for AArch64/SVE, but we'll need to 
extend it to handle return values too, so that we can support all 
overloaded forms of vuninitialized
(see https://gcc.gnu.org/pipermail/gcc-patches/2023-April/616003.html)

I meant this extension to be a follow-up work when most intrinsics have 
been converted and the few remaining ones (eg. vuninitialized) needs an 
improved framework.  And that would enable to fix the FIXME.

Thanks,

Christophe


> Thanks,
> Kyrill
> 
>> +    return r.resolve_unary ();
>> +  }
>> +};
>> +SHAPE (unary_acc)
>> +
>>   /* <T0>_t foo_t0[_t1](<T1>_t)
>>
>>      where the target type <t0> must be specified explicitly but the source
>> diff --git a/gcc/config/arm/arm-mve-builtins-shapes.h b/gcc/config/arm/arm-
>> mve-builtins-shapes.h
>> index fc1bacbd4da..c062fe624c4 100644
>> --- a/gcc/config/arm/arm-mve-builtins-shapes.h
>> +++ b/gcc/config/arm/arm-mve-builtins-shapes.h
>> @@ -53,6 +53,7 @@ namespace arm_mve
>>       extern const function_shape *const create;
>>       extern const function_shape *const inherent;
>>       extern const function_shape *const unary;
>> +    extern const function_shape *const unary_acc;
>>       extern const function_shape *const unary_convert;
>>       extern const function_shape *const unary_int32;
>>       extern const function_shape *const unary_int32_acc;
>> --
>> 2.34.1
>
Kyrylo Tkachov May 11, 2023, 8:23 a.m. UTC | #3 
> -----Original Message-----
> From: Christophe Lyon <Christophe.Lyon@arm.com>
> Sent: Thursday, May 11, 2023 9:21 AM
> 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 15/20] arm: [MVE intrinsics] add unary_acc shape
> 
> 
> 
> On 5/10/23 16:52, Kyrylo Tkachov wrote:
> >
> >
> >> -----Original Message-----
> >> From: Christophe Lyon <christophe.lyon@arm.com>
> >> Sent: Wednesday, May 10, 2023 2:31 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 15/20] arm: [MVE intrinsics] add unary_acc shape
> >>
> >> This patch adds the unary_acc shape description.
> >>
> >> 2022-10-25  Christophe Lyon  <christophe.lyon@arm.com>
> >>
> >> 	gcc/
> >> 	* config/arm/arm-mve-builtins-shapes.cc (unary_acc): New.
> >> 	* config/arm/arm-mve-builtins-shapes.h (unary_acc): New.
> >> ---
> >>   gcc/config/arm/arm-mve-builtins-shapes.cc | 28
> +++++++++++++++++++++++
> >>   gcc/config/arm/arm-mve-builtins-shapes.h  |  1 +
> >>   2 files changed, 29 insertions(+)
> >>
> >> diff --git a/gcc/config/arm/arm-mve-builtins-shapes.cc
> b/gcc/config/arm/arm-
> >> mve-builtins-shapes.cc
> >> index bff1c3e843b..e77a0cc20ac 100644
> >> --- a/gcc/config/arm/arm-mve-builtins-shapes.cc
> >> +++ b/gcc/config/arm/arm-mve-builtins-shapes.cc
> >> @@ -1066,6 +1066,34 @@ struct unary_def : public overloaded_base<0>
> >>   };
> >>   SHAPE (unary)
> >>
> >> +/* <S0:twice>_t vfoo[_<t0>](<T0>_t)
> >> +
> >> +   i.e. a version of "unary" in which the source elements are half the
> >> +   size of the destination scalar, but have the same type class.
> >> +
> >> +   Example: vaddlvq.
> >> +   int64_t [__arm_]vaddlvq[_s32](int32x4_t a)
> >> +   int64_t [__arm_]vaddlvq_p[_s32](int32x4_t a, mve_pred16_t p) */
> >> +struct unary_acc_def : public overloaded_base<0>
> >> +{
> >> +  void
> >> +  build (function_builder &b, const function_group_info &group,
> >> +	 bool preserve_user_namespace) const override
> >> +  {
> >> +    b.add_overloaded_functions (group, MODE_none,
> >> preserve_user_namespace);
> >> +    build_all (b, "sw0,v0", group, MODE_none,
> preserve_user_namespace);
> >> +  }
> >> +
> >> +  tree
> >> +  resolve (function_resolver &r) const override
> >> +  {
> >> +    /* FIXME: check that the return value is actually
> >> +       twice as wide as arg 0.  */
> >
> > Any reason why we can't add that check now?
> > I'd rather not add new FIXMEs here...
> 
> I understand :-)
> 
> That's because the resolver only knows about the arguments, not the
> return value:
>    /* The arguments to the overloaded function.  */
>    vec<tree, va_gc> &m_arglist;
> 
> I kept this like what already exists for AArch64/SVE, but we'll need to
> extend it to handle return values too, so that we can support all
> overloaded forms of vuninitialized
> (see https://gcc.gnu.org/pipermail/gcc-patches/2023-April/616003.html)
> 
> I meant this extension to be a follow-up work when most intrinsics have
> been converted and the few remaining ones (eg. vuninitialized) needs an
> improved framework.  And that would enable to fix the FIXME.

Thanks for explaining.
The series is ok for trunk then.
Kyrill

> 
> Thanks,
> 
> Christophe
> 
> 
> > Thanks,
> > Kyrill
> >
> >> +    return r.resolve_unary ();
> >> +  }
> >> +};
> >> +SHAPE (unary_acc)
> >> +
> >>   /* <T0>_t foo_t0[_t1](<T1>_t)
> >>
> >>      where the target type <t0> must be specified explicitly but the source
> >> diff --git a/gcc/config/arm/arm-mve-builtins-shapes.h
> b/gcc/config/arm/arm-
> >> mve-builtins-shapes.h
> >> index fc1bacbd4da..c062fe624c4 100644
> >> --- a/gcc/config/arm/arm-mve-builtins-shapes.h
> >> +++ b/gcc/config/arm/arm-mve-builtins-shapes.h
> >> @@ -53,6 +53,7 @@ namespace arm_mve
> >>       extern const function_shape *const create;
> >>       extern const function_shape *const inherent;
> >>       extern const function_shape *const unary;
> >> +    extern const function_shape *const unary_acc;
> >>       extern const function_shape *const unary_convert;
> >>       extern const function_shape *const unary_int32;
> >>       extern const function_shape *const unary_int32_acc;
> >> --
> >> 2.34.1
> >
Christophe Lyon May 11, 2023, 8:24 a.m. UTC | #4 
On 5/11/23 10:23, Kyrylo Tkachov wrote:
> 
> 
>> -----Original Message-----
>> From: Christophe Lyon <Christophe.Lyon@arm.com>
>> Sent: Thursday, May 11, 2023 9:21 AM
>> 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 15/20] arm: [MVE intrinsics] add unary_acc shape
>>
>>
>>
>> On 5/10/23 16:52, Kyrylo Tkachov wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Christophe Lyon <christophe.lyon@arm.com>
>>>> Sent: Wednesday, May 10, 2023 2:31 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 15/20] arm: [MVE intrinsics] add unary_acc shape
>>>>
>>>> This patch adds the unary_acc shape description.
>>>>
>>>> 2022-10-25  Christophe Lyon  <christophe.lyon@arm.com>
>>>>
>>>> 	gcc/
>>>> 	* config/arm/arm-mve-builtins-shapes.cc (unary_acc): New.
>>>> 	* config/arm/arm-mve-builtins-shapes.h (unary_acc): New.
>>>> ---
>>>>    gcc/config/arm/arm-mve-builtins-shapes.cc | 28
>> +++++++++++++++++++++++
>>>>    gcc/config/arm/arm-mve-builtins-shapes.h  |  1 +
>>>>    2 files changed, 29 insertions(+)
>>>>
>>>> diff --git a/gcc/config/arm/arm-mve-builtins-shapes.cc
>> b/gcc/config/arm/arm-
>>>> mve-builtins-shapes.cc
>>>> index bff1c3e843b..e77a0cc20ac 100644
>>>> --- a/gcc/config/arm/arm-mve-builtins-shapes.cc
>>>> +++ b/gcc/config/arm/arm-mve-builtins-shapes.cc
>>>> @@ -1066,6 +1066,34 @@ struct unary_def : public overloaded_base<0>
>>>>    };
>>>>    SHAPE (unary)
>>>>
>>>> +/* <S0:twice>_t vfoo[_<t0>](<T0>_t)
>>>> +
>>>> +   i.e. a version of "unary" in which the source elements are half the
>>>> +   size of the destination scalar, but have the same type class.
>>>> +
>>>> +   Example: vaddlvq.
>>>> +   int64_t [__arm_]vaddlvq[_s32](int32x4_t a)
>>>> +   int64_t [__arm_]vaddlvq_p[_s32](int32x4_t a, mve_pred16_t p) */
>>>> +struct unary_acc_def : public overloaded_base<0>
>>>> +{
>>>> +  void
>>>> +  build (function_builder &b, const function_group_info &group,
>>>> +	 bool preserve_user_namespace) const override
>>>> +  {
>>>> +    b.add_overloaded_functions (group, MODE_none,
>>>> preserve_user_namespace);
>>>> +    build_all (b, "sw0,v0", group, MODE_none,
>> preserve_user_namespace);
>>>> +  }
>>>> +
>>>> +  tree
>>>> +  resolve (function_resolver &r) const override
>>>> +  {
>>>> +    /* FIXME: check that the return value is actually
>>>> +       twice as wide as arg 0.  */
>>>
>>> Any reason why we can't add that check now?
>>> I'd rather not add new FIXMEs here...
>>
>> I understand :-)
>>
>> That's because the resolver only knows about the arguments, not the
>> return value:
>>     /* The arguments to the overloaded function.  */
>>     vec<tree, va_gc> &m_arglist;
>>
>> I kept this like what already exists for AArch64/SVE, but we'll need to
>> extend it to handle return values too, so that we can support all
>> overloaded forms of vuninitialized
>> (see https://gcc.gnu.org/pipermail/gcc-patches/2023-April/616003.html)
>>
>> I meant this extension to be a follow-up work when most intrinsics have
>> been converted and the few remaining ones (eg. vuninitialized) needs an
>> improved framework.  And that would enable to fix the FIXME.
> 
> Thanks for explaining.
> The series is ok for trunk then.

Great, thanks!

> Kyrill
> 
>>
>> Thanks,
>>
>> Christophe
>>
>>
>>> Thanks,
>>> Kyrill
>>>
>>>> +    return r.resolve_unary ();
>>>> +  }
>>>> +};
>>>> +SHAPE (unary_acc)
>>>> +
>>>>    /* <T0>_t foo_t0[_t1](<T1>_t)
>>>>
>>>>       where the target type <t0> must be specified explicitly but the source
>>>> diff --git a/gcc/config/arm/arm-mve-builtins-shapes.h
>> b/gcc/config/arm/arm-
>>>> mve-builtins-shapes.h
>>>> index fc1bacbd4da..c062fe624c4 100644
>>>> --- a/gcc/config/arm/arm-mve-builtins-shapes.h
>>>> +++ b/gcc/config/arm/arm-mve-builtins-shapes.h
>>>> @@ -53,6 +53,7 @@ namespace arm_mve
>>>>        extern const function_shape *const create;
>>>>        extern const function_shape *const inherent;
>>>>        extern const function_shape *const unary;
>>>> +    extern const function_shape *const unary_acc;
>>>>        extern const function_shape *const unary_convert;
>>>>        extern const function_shape *const unary_int32;
>>>>        extern const function_shape *const unary_int32_acc;
>>>> --
>>>> 2.34.1
>>>
Richard Sandiford May 11, 2023, 8:30 a.m. UTC | #5 
Christophe Lyon <christophe.lyon@arm.com> writes:
> On 5/10/23 16:52, Kyrylo Tkachov wrote:
>> 
>> 
>>> -----Original Message-----
>>> From: Christophe Lyon <christophe.lyon@arm.com>
>>> Sent: Wednesday, May 10, 2023 2:31 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 15/20] arm: [MVE intrinsics] add unary_acc shape
>>>
>>> This patch adds the unary_acc shape description.
>>>
>>> 2022-10-25  Christophe Lyon  <christophe.lyon@arm.com>
>>>
>>> 	gcc/
>>> 	* config/arm/arm-mve-builtins-shapes.cc (unary_acc): New.
>>> 	* config/arm/arm-mve-builtins-shapes.h (unary_acc): New.
>>> ---
>>>   gcc/config/arm/arm-mve-builtins-shapes.cc | 28 +++++++++++++++++++++++
>>>   gcc/config/arm/arm-mve-builtins-shapes.h  |  1 +
>>>   2 files changed, 29 insertions(+)
>>>
>>> diff --git a/gcc/config/arm/arm-mve-builtins-shapes.cc b/gcc/config/arm/arm-
>>> mve-builtins-shapes.cc
>>> index bff1c3e843b..e77a0cc20ac 100644
>>> --- a/gcc/config/arm/arm-mve-builtins-shapes.cc
>>> +++ b/gcc/config/arm/arm-mve-builtins-shapes.cc
>>> @@ -1066,6 +1066,34 @@ struct unary_def : public overloaded_base<0>
>>>   };
>>>   SHAPE (unary)
>>>
>>> +/* <S0:twice>_t vfoo[_<t0>](<T0>_t)
>>> +
>>> +   i.e. a version of "unary" in which the source elements are half the
>>> +   size of the destination scalar, but have the same type class.
>>> +
>>> +   Example: vaddlvq.
>>> +   int64_t [__arm_]vaddlvq[_s32](int32x4_t a)
>>> +   int64_t [__arm_]vaddlvq_p[_s32](int32x4_t a, mve_pred16_t p) */
>>> +struct unary_acc_def : public overloaded_base<0>
>>> +{
>>> +  void
>>> +  build (function_builder &b, const function_group_info &group,
>>> +	 bool preserve_user_namespace) const override
>>> +  {
>>> +    b.add_overloaded_functions (group, MODE_none,
>>> preserve_user_namespace);
>>> +    build_all (b, "sw0,v0", group, MODE_none, preserve_user_namespace);
>>> +  }
>>> +
>>> +  tree
>>> +  resolve (function_resolver &r) const override
>>> +  {
>>> +    /* FIXME: check that the return value is actually
>>> +       twice as wide as arg 0.  */
>> 
>> Any reason why we can't add that check now?
>> I'd rather not add new FIXMEs here...
>
> I understand :-)
>
> That's because the resolver only knows about the arguments, not the 
> return value:
>    /* The arguments to the overloaded function.  */
>    vec<tree, va_gc> &m_arglist;
>
> I kept this like what already exists for AArch64/SVE, but we'll need to 
> extend it to handle return values too, so that we can support all 
> overloaded forms of vuninitialized
> (see https://gcc.gnu.org/pipermail/gcc-patches/2023-April/616003.html)
>
> I meant this extension to be a follow-up work when most intrinsics have 
> been converted and the few remaining ones (eg. vuninitialized) needs an 
> improved framework.  And that would enable to fix the FIXME.

We can't resolve based on the return type though.  It has to be
arguments only.  E.g.:

   decltype(foo(a, b))

has to be well-defined, even though decltype (by design) provides no
context about "what the caller wants".

Thanks,
Richard
Christophe Lyon May 11, 2023, 9:54 a.m. UTC | #6 
On 5/11/23 10:30, Richard Sandiford wrote:
> Christophe Lyon <christophe.lyon@arm.com> writes:
>> On 5/10/23 16:52, Kyrylo Tkachov wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Christophe Lyon <christophe.lyon@arm.com>
>>>> Sent: Wednesday, May 10, 2023 2:31 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 15/20] arm: [MVE intrinsics] add unary_acc shape
>>>>
>>>> This patch adds the unary_acc shape description.
>>>>
>>>> 2022-10-25  Christophe Lyon  <christophe.lyon@arm.com>
>>>>
>>>> 	gcc/
>>>> 	* config/arm/arm-mve-builtins-shapes.cc (unary_acc): New.
>>>> 	* config/arm/arm-mve-builtins-shapes.h (unary_acc): New.
>>>> ---
>>>>    gcc/config/arm/arm-mve-builtins-shapes.cc | 28 +++++++++++++++++++++++
>>>>    gcc/config/arm/arm-mve-builtins-shapes.h  |  1 +
>>>>    2 files changed, 29 insertions(+)
>>>>
>>>> diff --git a/gcc/config/arm/arm-mve-builtins-shapes.cc b/gcc/config/arm/arm-
>>>> mve-builtins-shapes.cc
>>>> index bff1c3e843b..e77a0cc20ac 100644
>>>> --- a/gcc/config/arm/arm-mve-builtins-shapes.cc
>>>> +++ b/gcc/config/arm/arm-mve-builtins-shapes.cc
>>>> @@ -1066,6 +1066,34 @@ struct unary_def : public overloaded_base<0>
>>>>    };
>>>>    SHAPE (unary)
>>>>
>>>> +/* <S0:twice>_t vfoo[_<t0>](<T0>_t)
>>>> +
>>>> +   i.e. a version of "unary" in which the source elements are half the
>>>> +   size of the destination scalar, but have the same type class.
>>>> +
>>>> +   Example: vaddlvq.
>>>> +   int64_t [__arm_]vaddlvq[_s32](int32x4_t a)
>>>> +   int64_t [__arm_]vaddlvq_p[_s32](int32x4_t a, mve_pred16_t p) */
>>>> +struct unary_acc_def : public overloaded_base<0>
>>>> +{
>>>> +  void
>>>> +  build (function_builder &b, const function_group_info &group,
>>>> +	 bool preserve_user_namespace) const override
>>>> +  {
>>>> +    b.add_overloaded_functions (group, MODE_none,
>>>> preserve_user_namespace);
>>>> +    build_all (b, "sw0,v0", group, MODE_none, preserve_user_namespace);
>>>> +  }
>>>> +
>>>> +  tree
>>>> +  resolve (function_resolver &r) const override
>>>> +  {
>>>> +    /* FIXME: check that the return value is actually
>>>> +       twice as wide as arg 0.  */
>>>
>>> Any reason why we can't add that check now?
>>> I'd rather not add new FIXMEs here...
>>
>> I understand :-)
>>
>> That's because the resolver only knows about the arguments, not the
>> return value:
>>     /* The arguments to the overloaded function.  */
>>     vec<tree, va_gc> &m_arglist;
>>
>> I kept this like what already exists for AArch64/SVE, but we'll need to
>> extend it to handle return values too, so that we can support all
>> overloaded forms of vuninitialized
>> (see https://gcc.gnu.org/pipermail/gcc-patches/2023-April/616003.html)
>>
>> I meant this extension to be a follow-up work when most intrinsics have
>> been converted and the few remaining ones (eg. vuninitialized) needs an
>> improved framework.  And that would enable to fix the FIXME.
> 
> We can't resolve based on the return type though.  It has to be
> arguments only.  E.g.:
> 
>     decltype(foo(a, b))
> 
> has to be well-defined, even though decltype (by design) provides no
> context about "what the caller wants".
> 

So in fact we can probably get rid of (most of) the remaining 
definitions of vuninitializedq in arm_mve.h, but not by looking at the 
return type (re-reading this I'm wondering whether I overlooked this 
when I started the series....)

But for things like vaddlvq, we can't check that the result is actually 
written in a twice-as-large as the argument location?

Thanks,

Christophe


> Thanks,
> Richard
Richard Sandiford May 11, 2023, 10:58 a.m. UTC | #7 
Christophe Lyon <christophe.lyon@arm.com> writes:
> On 5/11/23 10:30, Richard Sandiford wrote:
>> Christophe Lyon <christophe.lyon@arm.com> writes:
>>> On 5/10/23 16:52, Kyrylo Tkachov wrote:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Christophe Lyon <christophe.lyon@arm.com>
>>>>> Sent: Wednesday, May 10, 2023 2:31 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 15/20] arm: [MVE intrinsics] add unary_acc shape
>>>>>
>>>>> This patch adds the unary_acc shape description.
>>>>>
>>>>> 2022-10-25  Christophe Lyon  <christophe.lyon@arm.com>
>>>>>
>>>>> 	gcc/
>>>>> 	* config/arm/arm-mve-builtins-shapes.cc (unary_acc): New.
>>>>> 	* config/arm/arm-mve-builtins-shapes.h (unary_acc): New.
>>>>> ---
>>>>>    gcc/config/arm/arm-mve-builtins-shapes.cc | 28 +++++++++++++++++++++++
>>>>>    gcc/config/arm/arm-mve-builtins-shapes.h  |  1 +
>>>>>    2 files changed, 29 insertions(+)
>>>>>
>>>>> diff --git a/gcc/config/arm/arm-mve-builtins-shapes.cc b/gcc/config/arm/arm-
>>>>> mve-builtins-shapes.cc
>>>>> index bff1c3e843b..e77a0cc20ac 100644
>>>>> --- a/gcc/config/arm/arm-mve-builtins-shapes.cc
>>>>> +++ b/gcc/config/arm/arm-mve-builtins-shapes.cc
>>>>> @@ -1066,6 +1066,34 @@ struct unary_def : public overloaded_base<0>
>>>>>    };
>>>>>    SHAPE (unary)
>>>>>
>>>>> +/* <S0:twice>_t vfoo[_<t0>](<T0>_t)
>>>>> +
>>>>> +   i.e. a version of "unary" in which the source elements are half the
>>>>> +   size of the destination scalar, but have the same type class.
>>>>> +
>>>>> +   Example: vaddlvq.
>>>>> +   int64_t [__arm_]vaddlvq[_s32](int32x4_t a)
>>>>> +   int64_t [__arm_]vaddlvq_p[_s32](int32x4_t a, mve_pred16_t p) */
>>>>> +struct unary_acc_def : public overloaded_base<0>
>>>>> +{
>>>>> +  void
>>>>> +  build (function_builder &b, const function_group_info &group,
>>>>> +	 bool preserve_user_namespace) const override
>>>>> +  {
>>>>> +    b.add_overloaded_functions (group, MODE_none,
>>>>> preserve_user_namespace);
>>>>> +    build_all (b, "sw0,v0", group, MODE_none, preserve_user_namespace);
>>>>> +  }
>>>>> +
>>>>> +  tree
>>>>> +  resolve (function_resolver &r) const override
>>>>> +  {
>>>>> +    /* FIXME: check that the return value is actually
>>>>> +       twice as wide as arg 0.  */
>>>>
>>>> Any reason why we can't add that check now?
>>>> I'd rather not add new FIXMEs here...
>>>
>>> I understand :-)
>>>
>>> That's because the resolver only knows about the arguments, not the
>>> return value:
>>>     /* The arguments to the overloaded function.  */
>>>     vec<tree, va_gc> &m_arglist;
>>>
>>> I kept this like what already exists for AArch64/SVE, but we'll need to
>>> extend it to handle return values too, so that we can support all
>>> overloaded forms of vuninitialized
>>> (see https://gcc.gnu.org/pipermail/gcc-patches/2023-April/616003.html)
>>>
>>> I meant this extension to be a follow-up work when most intrinsics have
>>> been converted and the few remaining ones (eg. vuninitialized) needs an
>>> improved framework.  And that would enable to fix the FIXME.
>> 
>> We can't resolve based on the return type though.  It has to be
>> arguments only.  E.g.:
>> 
>>     decltype(foo(a, b))
>> 
>> has to be well-defined, even though decltype (by design) provides no
>> context about "what the caller wants".
>> 
>
> So in fact we can probably get rid of (most of) the remaining 
> definitions of vuninitializedq in arm_mve.h, but not by looking at the 
> return type (re-reading this I'm wondering whether I overlooked this 
> when I started the series....)
>
> But for things like vaddlvq, we can't check that the result is actually 
> written in a twice-as-large as the argument location?

No.  All we can/should do is to resolve the typeless builtin to a fully-typed
builtin, based on the argument types.  The return type of that fully-typed
builtin determines the type of the function call expression (the CALL_EXPR).
It's then up to the frontend to do semantic/type checking of the
resolved expression type.

In other words, information only flows in one direction:

  argument types -> function overloading -> function return type

Thanks,
Richard
diff mbox series

Patch

diff --git a/gcc/config/arm/arm-mve-builtins-shapes.cc b/gcc/config/arm/arm-mve-builtins-shapes.cc
index bff1c3e843b..e77a0cc20ac 100644
--- a/gcc/config/arm/arm-mve-builtins-shapes.cc
+++ b/gcc/config/arm/arm-mve-builtins-shapes.cc
@@ -1066,6 +1066,34 @@  struct unary_def : public overloaded_base<0>
 };
 SHAPE (unary)
 
+/* <S0:twice>_t vfoo[_<t0>](<T0>_t)
+
+   i.e. a version of "unary" in which the source elements are half the
+   size of the destination scalar, but have the same type class.
+
+   Example: vaddlvq.
+   int64_t [__arm_]vaddlvq[_s32](int32x4_t a)
+   int64_t [__arm_]vaddlvq_p[_s32](int32x4_t a, mve_pred16_t p) */
+struct unary_acc_def : public overloaded_base<0>
+{
+  void
+  build (function_builder &b, const function_group_info &group,
+	 bool preserve_user_namespace) const override
+  {
+    b.add_overloaded_functions (group, MODE_none, preserve_user_namespace);
+    build_all (b, "sw0,v0", group, MODE_none, preserve_user_namespace);
+  }
+
+  tree
+  resolve (function_resolver &r) const override
+  {
+    /* FIXME: check that the return value is actually
+       twice as wide as arg 0.  */
+    return r.resolve_unary ();
+  }
+};
+SHAPE (unary_acc)
+
 /* <T0>_t foo_t0[_t1](<T1>_t)
 
    where the target type <t0> must be specified explicitly but the source
diff --git a/gcc/config/arm/arm-mve-builtins-shapes.h b/gcc/config/arm/arm-mve-builtins-shapes.h
index fc1bacbd4da..c062fe624c4 100644
--- a/gcc/config/arm/arm-mve-builtins-shapes.h
+++ b/gcc/config/arm/arm-mve-builtins-shapes.h
@@ -53,6 +53,7 @@  namespace arm_mve
     extern const function_shape *const create;
     extern const function_shape *const inherent;
     extern const function_shape *const unary;
+    extern const function_shape *const unary_acc;
     extern const function_shape *const unary_convert;
     extern const function_shape *const unary_int32;
     extern const function_shape *const unary_int32_acc;