| Message ID | 20240711214305.3193022-1-christophe.lyon@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [01/15] arm: [MVE intrinsics] improve comment for orrq shape | expand |
ping for the series? On Thu, 11 Jul 2024 at 23:43, Christophe Lyon <christophe.lyon@linaro.org> wrote: > > Add a comment about the lack of "n" forms for floating-point nor 8-bit > integers, to make it clearer why we use build_16_32 for MODE_n. > > 2024-07-11 Christophe Lyon <christophe.lyon@linaro.org> > > gcc/ > * config/arm/arm-mve-builtins-shapes.cc (binary_orrq_def): Improve comment. > --- > gcc/config/arm/arm-mve-builtins-shapes.cc | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/gcc/config/arm/arm-mve-builtins-shapes.cc b/gcc/config/arm/arm-mve-builtins-shapes.cc > index ba20c6a8f73..e01939469e3 100644 > --- a/gcc/config/arm/arm-mve-builtins-shapes.cc > +++ b/gcc/config/arm/arm-mve-builtins-shapes.cc > @@ -865,7 +865,12 @@ SHAPE (binary_opt_n) > int16x8_t [__arm_]vorrq_m[_s16](int16x8_t inactive, int16x8_t a, int16x8_t b, mve_pred16_t p) > int16x8_t [__arm_]vorrq_x[_s16](int16x8_t a, int16x8_t b, mve_pred16_t p) > int16x8_t [__arm_]vorrq[_n_s16](int16x8_t a, const int16_t imm) > - int16x8_t [__arm_]vorrq_m_n[_s16](int16x8_t a, const int16_t imm, mve_pred16_t p) */ > + int16x8_t [__arm_]vorrq_m_n[_s16](int16x8_t a, const int16_t imm, mve_pred16_t p) > + > + No "_n" forms for floating-point, nor 8-bit integers: > + float16x8_t [__arm_]vorrq[_f16](float16x8_t a, float16x8_t b) > + float16x8_t [__arm_]vorrq_m[_f16](float16x8_t inactive, float16x8_t a, float16x8_t b, mve_pred16_t p) > + float16x8_t [__arm_]vorrq_x[_f16](float16x8_t a, float16x8_t b, mve_pred16_t p) */ > struct binary_orrq_def : public overloaded_base<0> > { > bool > -- > 2.34.1 >
Hi Christophe,
Maybe this patch was based on an older source, but the comment now reads:
/* <T0>_t vfoo[t0](<T0>_t, <T0>_t)
<T0>_t vfoo[_n_t0](<T0>_t, <S0>_t)
Where the _n form only supports s16/s32/u16/u32 types as for vorrq.
Example: vorrq.
int16x8_t [__arm_]vorrq[_s16](int16x8_t a, int16x8_t b)
int16x8_t [__arm_]vorrq_m[_s16](int16x8_t inactive, int16x8_t a,
int16x8_t b, mve_pred16_t p)
int16x8_t [__arm_]vorrq_x[_s16](int16x8_t a, int16x8_t b,
mve_pred16_t p)
int16x8_t [__arm_]vorrq[_n_s16](int16x8_t a, const int16_t imm)
int16x8_t [__arm_]vorrq_m_n[_s16](int16x8_t a, const int16_t imm,
mve_pred16_t p) */
So it already says it only supports _n for 16/32-bit signed and unsigned
integers. I don't have a problem with your changes necessarily, but it
does seem a bit double. Anyway, no strong objection, just making sure we
are really wanting to emphasize further that there are no _n for FP or
8-bit types.
On 11/07/2024 22:42, Christophe Lyon wrote:
> Add a comment about the lack of "n" forms for floating-point nor 8-bit
> integers, to make it clearer why we use build_16_32 for MODE_n.
>
> 2024-07-11 Christophe Lyon <christophe.lyon@linaro.org>
>
> gcc/
> * config/arm/arm-mve-builtins-shapes.cc (binary_orrq_def): Improve comment.
> ---
> gcc/config/arm/arm-mve-builtins-shapes.cc | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/config/arm/arm-mve-builtins-shapes.cc b/gcc/config/arm/arm-mve-builtins-shapes.cc
> index ba20c6a8f73..e01939469e3 100644
> --- a/gcc/config/arm/arm-mve-builtins-shapes.cc
> +++ b/gcc/config/arm/arm-mve-builtins-shapes.cc
> @@ -865,7 +865,12 @@ SHAPE (binary_opt_n)
> int16x8_t [__arm_]vorrq_m[_s16](int16x8_t inactive, int16x8_t a, int16x8_t b, mve_pred16_t p)
> int16x8_t [__arm_]vorrq_x[_s16](int16x8_t a, int16x8_t b, mve_pred16_t p)
> int16x8_t [__arm_]vorrq[_n_s16](int16x8_t a, const int16_t imm)
> - int16x8_t [__arm_]vorrq_m_n[_s16](int16x8_t a, const int16_t imm, mve_pred16_t p) */
> + int16x8_t [__arm_]vorrq_m_n[_s16](int16x8_t a, const int16_t imm, mve_pred16_t p)
> +
> + No "_n" forms for floating-point, nor 8-bit integers:
> + float16x8_t [__arm_]vorrq[_f16](float16x8_t a, float16x8_t b)
> + float16x8_t [__arm_]vorrq_m[_f16](float16x8_t inactive, float16x8_t a, float16x8_t b, mve_pred16_t p)
> + float16x8_t [__arm_]vorrq_x[_f16](float16x8_t a, float16x8_t b, mve_pred16_t p) */
> struct binary_orrq_def : public overloaded_base<0>
> {
> bool
On Fri, 2 Aug 2024 at 17:46, Andre Vieira (lists) <andre.simoesdiasvieira@arm.com> wrote: > > Hi Christophe, > > Maybe this patch was based on an older source, but the comment now reads: > > /* <T0>_t vfoo[t0](<T0>_t, <T0>_t) > <T0>_t vfoo[_n_t0](<T0>_t, <S0>_t) > > Where the _n form only supports s16/s32/u16/u32 types as for vorrq. > > Example: vorrq. > int16x8_t [__arm_]vorrq[_s16](int16x8_t a, int16x8_t b) > int16x8_t [__arm_]vorrq_m[_s16](int16x8_t inactive, int16x8_t a, > int16x8_t b, mve_pred16_t p) > int16x8_t [__arm_]vorrq_x[_s16](int16x8_t a, int16x8_t b, > mve_pred16_t p) > int16x8_t [__arm_]vorrq[_n_s16](int16x8_t a, const int16_t imm) > int16x8_t [__arm_]vorrq_m_n[_s16](int16x8_t a, const int16_t imm, > mve_pred16_t p) */ > > So it already says it only supports _n for 16/32-bit signed and unsigned > integers. I don't have a problem with your changes necessarily, but it > does seem a bit double. Anyway, no strong objection, just making sure we > are really wanting to emphasize further that there are no _n for FP or > 8-bit types. > Indeed it's a bit verbose :-) I added it when I restarted working on this series as it was not immediately clear what the first comment implied... Thanks Christophe > On 11/07/2024 22:42, Christophe Lyon wrote: > > Add a comment about the lack of "n" forms for floating-point nor 8-bit > > integers, to make it clearer why we use build_16_32 for MODE_n. > > > > 2024-07-11 Christophe Lyon <christophe.lyon@linaro.org> > > > > gcc/ > > * config/arm/arm-mve-builtins-shapes.cc (binary_orrq_def): Improve comment. > > --- > > gcc/config/arm/arm-mve-builtins-shapes.cc | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/gcc/config/arm/arm-mve-builtins-shapes.cc b/gcc/config/arm/arm-mve-builtins-shapes.cc > > index ba20c6a8f73..e01939469e3 100644 > > --- a/gcc/config/arm/arm-mve-builtins-shapes.cc > > +++ b/gcc/config/arm/arm-mve-builtins-shapes.cc > > @@ -865,7 +865,12 @@ SHAPE (binary_opt_n) > > int16x8_t [__arm_]vorrq_m[_s16](int16x8_t inactive, int16x8_t a, int16x8_t b, mve_pred16_t p) > > int16x8_t [__arm_]vorrq_x[_s16](int16x8_t a, int16x8_t b, mve_pred16_t p) > > int16x8_t [__arm_]vorrq[_n_s16](int16x8_t a, const int16_t imm) > > - int16x8_t [__arm_]vorrq_m_n[_s16](int16x8_t a, const int16_t imm, mve_pred16_t p) */ > > + int16x8_t [__arm_]vorrq_m_n[_s16](int16x8_t a, const int16_t imm, mve_pred16_t p) > > + > > + No "_n" forms for floating-point, nor 8-bit integers: > > + float16x8_t [__arm_]vorrq[_f16](float16x8_t a, float16x8_t b) > > + float16x8_t [__arm_]vorrq_m[_f16](float16x8_t inactive, float16x8_t a, float16x8_t b, mve_pred16_t p) > > + float16x8_t [__arm_]vorrq_x[_f16](float16x8_t a, float16x8_t b, mve_pred16_t p) */ > > struct binary_orrq_def : public overloaded_base<0> > > { > > bool
diff --git a/gcc/config/arm/arm-mve-builtins-shapes.cc b/gcc/config/arm/arm-mve-builtins-shapes.cc index ba20c6a8f73..e01939469e3 100644 --- a/gcc/config/arm/arm-mve-builtins-shapes.cc +++ b/gcc/config/arm/arm-mve-builtins-shapes.cc @@ -865,7 +865,12 @@ SHAPE (binary_opt_n) int16x8_t [__arm_]vorrq_m[_s16](int16x8_t inactive, int16x8_t a, int16x8_t b, mve_pred16_t p) int16x8_t [__arm_]vorrq_x[_s16](int16x8_t a, int16x8_t b, mve_pred16_t p) int16x8_t [__arm_]vorrq[_n_s16](int16x8_t a, const int16_t imm) - int16x8_t [__arm_]vorrq_m_n[_s16](int16x8_t a, const int16_t imm, mve_pred16_t p) */ + int16x8_t [__arm_]vorrq_m_n[_s16](int16x8_t a, const int16_t imm, mve_pred16_t p) + + No "_n" forms for floating-point, nor 8-bit integers: + float16x8_t [__arm_]vorrq[_f16](float16x8_t a, float16x8_t b) + float16x8_t [__arm_]vorrq_m[_f16](float16x8_t inactive, float16x8_t a, float16x8_t b, mve_pred16_t p) + float16x8_t [__arm_]vorrq_x[_f16](float16x8_t a, float16x8_t b, mve_pred16_t p) */ struct binary_orrq_def : public overloaded_base<0> { bool