public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 2/2]AArch64 Add support for neg on v1df
@ 2022-09-23  9:17 Tamar Christina
  2022-09-23  9:30 ` Richard Sandiford
  0 siblings, 1 reply; 7+ messages in thread
From: Tamar Christina @ 2022-09-23  9:17 UTC (permalink / raw)
  To: gcc-patches
  Cc: nd, Richard.Earnshaw, Marcus.Shawcroft, Kyrylo.Tkachov,
	richard.sandiford

[-- Attachment #1: Type: text/plain, Size: 1856 bytes --]

Hi All,

This adds support for using scalar fneg on the V1DF type.

Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.

Ok for master?

Thanks,
Tamar

gcc/ChangeLog:

	* config/aarch64/aarch64-simd.md (negv1df2): New.

gcc/testsuite/ChangeLog:

	* gcc.target/aarch64/simd/addsub_2.c: New test.

--- inline copy of patch -- 
diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index f4152160084d6b6f34bd69f0ba6386c1ab50f77e..cf8c094bd4b76981cef2dd5dd7b8e6be0d56101f 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -2713,6 +2713,14 @@ (define_insn "neg<mode>2"
   [(set_attr "type" "neon_fp_neg_<stype><q>")]
 )
 
+(define_insn "negv1df2"
+ [(set (match_operand:V1DF 0 "register_operand" "=w")
+       (neg:V1DF (match_operand:V1DF 1 "register_operand" "w")))]
+ "TARGET_SIMD"
+ "fneg\\t%d0, %d1"
+  [(set_attr "type" "neon_fp_neg_d")]
+)
+
 (define_insn "abs<mode>2"
  [(set (match_operand:VHSDF 0 "register_operand" "=w")
        (abs:VHSDF (match_operand:VHSDF 1 "register_operand" "w")))]
diff --git a/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c b/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c
new file mode 100644
index 0000000000000000000000000000000000000000..55a7365e897f8af509de953129e0f516974f7ca8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c
@@ -0,0 +1,22 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast" } */
+/* { dg-final { check-function-bodies "**" "" "" { target { le } } } } */
+
+#pragma GCC target "+nosve"
+
+/*
+** f1:
+** ...
+**	fneg	d[0-9]+, d[0-9]+
+**	fadd	v[0-9]+.2s, v[0-9]+.2s, v[0-9]+.2s
+** ...
+*/
+void f1 (float *restrict a, float *restrict b, float *res, int n)
+{
+   for (int i = 0; i < 2; i+=2)
+    {
+      res[i+0] = a[i+0] + b[i+0];
+      res[i+1] = a[i+1] - b[i+1];
+    }
+}
+




-- 

[-- Attachment #2: rb16239.patch --]
[-- Type: text/plain, Size: 1514 bytes --]

diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index f4152160084d6b6f34bd69f0ba6386c1ab50f77e..cf8c094bd4b76981cef2dd5dd7b8e6be0d56101f 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -2713,6 +2713,14 @@ (define_insn "neg<mode>2"
   [(set_attr "type" "neon_fp_neg_<stype><q>")]
 )
 
+(define_insn "negv1df2"
+ [(set (match_operand:V1DF 0 "register_operand" "=w")
+       (neg:V1DF (match_operand:V1DF 1 "register_operand" "w")))]
+ "TARGET_SIMD"
+ "fneg\\t%d0, %d1"
+  [(set_attr "type" "neon_fp_neg_d")]
+)
+
 (define_insn "abs<mode>2"
  [(set (match_operand:VHSDF 0 "register_operand" "=w")
        (abs:VHSDF (match_operand:VHSDF 1 "register_operand" "w")))]
diff --git a/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c b/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c
new file mode 100644
index 0000000000000000000000000000000000000000..55a7365e897f8af509de953129e0f516974f7ca8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c
@@ -0,0 +1,22 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast" } */
+/* { dg-final { check-function-bodies "**" "" "" { target { le } } } } */
+
+#pragma GCC target "+nosve"
+
+/*
+** f1:
+** ...
+**	fneg	d[0-9]+, d[0-9]+
+**	fadd	v[0-9]+.2s, v[0-9]+.2s, v[0-9]+.2s
+** ...
+*/
+void f1 (float *restrict a, float *restrict b, float *res, int n)
+{
+   for (int i = 0; i < 2; i+=2)
+    {
+      res[i+0] = a[i+0] + b[i+0];
+      res[i+1] = a[i+1] - b[i+1];
+    }
+}
+




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2]AArch64 Add support for neg on v1df
  2022-09-23  9:17 [PATCH 2/2]AArch64 Add support for neg on v1df Tamar Christina
@ 2022-09-23  9:30 ` Richard Sandiford
  2022-09-23  9:43   ` Tamar Christina
  0 siblings, 1 reply; 7+ messages in thread
From: Richard Sandiford @ 2022-09-23  9:30 UTC (permalink / raw)
  To: Tamar Christina
  Cc: gcc-patches, nd, Richard.Earnshaw, Marcus.Shawcroft, Kyrylo.Tkachov

Tamar Christina <tamar.christina@arm.com> writes:
> Hi All,
>
> This adds support for using scalar fneg on the V1DF type.
>
> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
>
> Ok for master?

Why just this one operation though?  Couldn't we extend iterators
like GPF_F16 to include V1DF, avoiding the need for new patterns?

Richard

>
> Thanks,
> Tamar
>
> gcc/ChangeLog:
>
> 	* config/aarch64/aarch64-simd.md (negv1df2): New.
>
> gcc/testsuite/ChangeLog:
>
> 	* gcc.target/aarch64/simd/addsub_2.c: New test.
>
> --- inline copy of patch -- 
> diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
> index f4152160084d6b6f34bd69f0ba6386c1ab50f77e..cf8c094bd4b76981cef2dd5dd7b8e6be0d56101f 100644
> --- a/gcc/config/aarch64/aarch64-simd.md
> +++ b/gcc/config/aarch64/aarch64-simd.md
> @@ -2713,6 +2713,14 @@ (define_insn "neg<mode>2"
>    [(set_attr "type" "neon_fp_neg_<stype><q>")]
>  )
>  
> +(define_insn "negv1df2"
> + [(set (match_operand:V1DF 0 "register_operand" "=w")
> +       (neg:V1DF (match_operand:V1DF 1 "register_operand" "w")))]
> + "TARGET_SIMD"
> + "fneg\\t%d0, %d1"
> +  [(set_attr "type" "neon_fp_neg_d")]
> +)
> +
>  (define_insn "abs<mode>2"
>   [(set (match_operand:VHSDF 0 "register_operand" "=w")
>         (abs:VHSDF (match_operand:VHSDF 1 "register_operand" "w")))]
> diff --git a/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c b/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..55a7365e897f8af509de953129e0f516974f7ca8
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c
> @@ -0,0 +1,22 @@
> +/* { dg-do compile } */
> +/* { dg-options "-Ofast" } */
> +/* { dg-final { check-function-bodies "**" "" "" { target { le } } } } */
> +
> +#pragma GCC target "+nosve"
> +
> +/*
> +** f1:
> +** ...
> +**	fneg	d[0-9]+, d[0-9]+
> +**	fadd	v[0-9]+.2s, v[0-9]+.2s, v[0-9]+.2s
> +** ...
> +*/
> +void f1 (float *restrict a, float *restrict b, float *res, int n)
> +{
> +   for (int i = 0; i < 2; i+=2)
> +    {
> +      res[i+0] = a[i+0] + b[i+0];
> +      res[i+1] = a[i+1] - b[i+1];
> +    }
> +}
> +

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH 2/2]AArch64 Add support for neg on v1df
  2022-09-23  9:30 ` Richard Sandiford
@ 2022-09-23  9:43   ` Tamar Christina
  2022-09-23 10:03     ` Richard Sandiford
  0 siblings, 1 reply; 7+ messages in thread
From: Tamar Christina @ 2022-09-23  9:43 UTC (permalink / raw)
  To: Richard Sandiford
  Cc: gcc-patches, nd, Richard Earnshaw, Marcus Shawcroft, Kyrylo Tkachov

> -----Original Message-----
> From: Richard Sandiford <richard.sandiford@arm.com>
> Sent: Friday, September 23, 2022 5:30 AM
> To: Tamar Christina <Tamar.Christina@arm.com>
> Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; Richard Earnshaw
> <Richard.Earnshaw@arm.com>; Marcus Shawcroft
> <Marcus.Shawcroft@arm.com>; Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
> Subject: Re: [PATCH 2/2]AArch64 Add support for neg on v1df
> 
> Tamar Christina <tamar.christina@arm.com> writes:
> > Hi All,
> >
> > This adds support for using scalar fneg on the V1DF type.
> >
> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> >
> > Ok for master?
> 
> Why just this one operation though?  Couldn't we extend iterators like
> GPF_F16 to include V1DF, avoiding the need for new patterns?
> 

Simply because it's the only one I know how to generate code for.
I can change GPF_F16 but I don't know under which circumstances we'd generate
a V1DF for the other operations.

So if it's ok to do so without full test coverage I'm happy to do so...

Tamar.

> Richard
> 
> >
> > Thanks,
> > Tamar
> >
> > gcc/ChangeLog:
> >
> > 	* config/aarch64/aarch64-simd.md (negv1df2): New.
> >
> > gcc/testsuite/ChangeLog:
> >
> > 	* gcc.target/aarch64/simd/addsub_2.c: New test.
> >
> > --- inline copy of patch --
> > diff --git a/gcc/config/aarch64/aarch64-simd.md
> > b/gcc/config/aarch64/aarch64-simd.md
> > index
> >
> f4152160084d6b6f34bd69f0ba6386c1ab50f77e..cf8c094bd4b76981cef2dd5dd7
> b8
> > e6be0d56101f 100644
> > --- a/gcc/config/aarch64/aarch64-simd.md
> > +++ b/gcc/config/aarch64/aarch64-simd.md
> > @@ -2713,6 +2713,14 @@ (define_insn "neg<mode>2"
> >    [(set_attr "type" "neon_fp_neg_<stype><q>")]
> >  )
> >
> > +(define_insn "negv1df2"
> > + [(set (match_operand:V1DF 0 "register_operand" "=w")
> > +       (neg:V1DF (match_operand:V1DF 1 "register_operand" "w")))]
> > +"TARGET_SIMD"
> > + "fneg\\t%d0, %d1"
> > +  [(set_attr "type" "neon_fp_neg_d")]
> > +)
> > +
> >  (define_insn "abs<mode>2"
> >   [(set (match_operand:VHSDF 0 "register_operand" "=w")
> >         (abs:VHSDF (match_operand:VHSDF 1 "register_operand" "w")))]
> > diff --git a/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c
> > b/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c
> > new file mode 100644
> > index
> >
> 0000000000000000000000000000000000000000..55a7365e897f8af509de953129
> e0
> > f516974f7ca8
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c
> > @@ -0,0 +1,22 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-Ofast" } */
> > +/* { dg-final { check-function-bodies "**" "" "" { target { le } } }
> > +} */
> > +
> > +#pragma GCC target "+nosve"
> > +
> > +/*
> > +** f1:
> > +** ...
> > +**	fneg	d[0-9]+, d[0-9]+
> > +**	fadd	v[0-9]+.2s, v[0-9]+.2s, v[0-9]+.2s
> > +** ...
> > +*/
> > +void f1 (float *restrict a, float *restrict b, float *res, int n) {
> > +   for (int i = 0; i < 2; i+=2)
> > +    {
> > +      res[i+0] = a[i+0] + b[i+0];
> > +      res[i+1] = a[i+1] - b[i+1];
> > +    }
> > +}
> > +

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2]AArch64 Add support for neg on v1df
  2022-09-23  9:43   ` Tamar Christina
@ 2022-09-23 10:03     ` Richard Sandiford
  2022-09-23 10:10       ` Tamar Christina
  0 siblings, 1 reply; 7+ messages in thread
From: Richard Sandiford @ 2022-09-23 10:03 UTC (permalink / raw)
  To: Tamar Christina
  Cc: gcc-patches, nd, Richard Earnshaw, Marcus Shawcroft, Kyrylo Tkachov

Tamar Christina <Tamar.Christina@arm.com> writes:
>> -----Original Message-----
>> From: Richard Sandiford <richard.sandiford@arm.com>
>> Sent: Friday, September 23, 2022 5:30 AM
>> To: Tamar Christina <Tamar.Christina@arm.com>
>> Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; Richard Earnshaw
>> <Richard.Earnshaw@arm.com>; Marcus Shawcroft
>> <Marcus.Shawcroft@arm.com>; Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
>> Subject: Re: [PATCH 2/2]AArch64 Add support for neg on v1df
>> 
>> Tamar Christina <tamar.christina@arm.com> writes:
>> > Hi All,
>> >
>> > This adds support for using scalar fneg on the V1DF type.
>> >
>> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
>> >
>> > Ok for master?
>> 
>> Why just this one operation though?  Couldn't we extend iterators like
>> GPF_F16 to include V1DF, avoiding the need for new patterns?
>> 
>
> Simply because it's the only one I know how to generate code for.
> I can change GPF_F16 but I don't know under which circumstances we'd generate
> a V1DF for the other operations.

We'd do it for things like:

__Float64x1_t foo (__Float64x1_t x) { return -x; }

if the pattern is available, instead of using subregs.  So one way
would be to scan the expand rtl dump for subregs.

If the point is that there is no observable difference between
defining 1-element vector ops and not, except for this one case,
then that suggests we should handle this case in target-independent
code instead.  There's no point forcing every target that has V1DF
to define a duplicate of the DF neg pattern.

Thanks,
Richard
>
> So if it's ok to do so without full test coverage I'm happy to do so...
>
> Tamar.
>
>> Richard
>> 
>> >
>> > Thanks,
>> > Tamar
>> >
>> > gcc/ChangeLog:
>> >
>> > 	* config/aarch64/aarch64-simd.md (negv1df2): New.
>> >
>> > gcc/testsuite/ChangeLog:
>> >
>> > 	* gcc.target/aarch64/simd/addsub_2.c: New test.
>> >
>> > --- inline copy of patch --
>> > diff --git a/gcc/config/aarch64/aarch64-simd.md
>> > b/gcc/config/aarch64/aarch64-simd.md
>> > index
>> >
>> f4152160084d6b6f34bd69f0ba6386c1ab50f77e..cf8c094bd4b76981cef2dd5dd7
>> b8
>> > e6be0d56101f 100644
>> > --- a/gcc/config/aarch64/aarch64-simd.md
>> > +++ b/gcc/config/aarch64/aarch64-simd.md
>> > @@ -2713,6 +2713,14 @@ (define_insn "neg<mode>2"
>> >    [(set_attr "type" "neon_fp_neg_<stype><q>")]
>> >  )
>> >
>> > +(define_insn "negv1df2"
>> > + [(set (match_operand:V1DF 0 "register_operand" "=w")
>> > +       (neg:V1DF (match_operand:V1DF 1 "register_operand" "w")))]
>> > +"TARGET_SIMD"
>> > + "fneg\\t%d0, %d1"
>> > +  [(set_attr "type" "neon_fp_neg_d")]
>> > +)
>> > +
>> >  (define_insn "abs<mode>2"
>> >   [(set (match_operand:VHSDF 0 "register_operand" "=w")
>> >         (abs:VHSDF (match_operand:VHSDF 1 "register_operand" "w")))]
>> > diff --git a/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c
>> > b/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c
>> > new file mode 100644
>> > index
>> >
>> 0000000000000000000000000000000000000000..55a7365e897f8af509de953129
>> e0
>> > f516974f7ca8
>> > --- /dev/null
>> > +++ b/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c
>> > @@ -0,0 +1,22 @@
>> > +/* { dg-do compile } */
>> > +/* { dg-options "-Ofast" } */
>> > +/* { dg-final { check-function-bodies "**" "" "" { target { le } } }
>> > +} */
>> > +
>> > +#pragma GCC target "+nosve"
>> > +
>> > +/*
>> > +** f1:
>> > +** ...
>> > +**	fneg	d[0-9]+, d[0-9]+
>> > +**	fadd	v[0-9]+.2s, v[0-9]+.2s, v[0-9]+.2s
>> > +** ...
>> > +*/
>> > +void f1 (float *restrict a, float *restrict b, float *res, int n) {
>> > +   for (int i = 0; i < 2; i+=2)
>> > +    {
>> > +      res[i+0] = a[i+0] + b[i+0];
>> > +      res[i+1] = a[i+1] - b[i+1];
>> > +    }
>> > +}
>> > +

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH 2/2]AArch64 Add support for neg on v1df
  2022-09-23 10:03     ` Richard Sandiford
@ 2022-09-23 10:10       ` Tamar Christina
  2022-09-23 14:01         ` Richard Sandiford
  0 siblings, 1 reply; 7+ messages in thread
From: Tamar Christina @ 2022-09-23 10:10 UTC (permalink / raw)
  To: Richard Sandiford
  Cc: gcc-patches, nd, Richard Earnshaw, Marcus Shawcroft, Kyrylo Tkachov

> -----Original Message-----
> From: Richard Sandiford <richard.sandiford@arm.com>
> Sent: Friday, September 23, 2022 6:04 AM
> To: Tamar Christina <Tamar.Christina@arm.com>
> Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; Richard Earnshaw
> <Richard.Earnshaw@arm.com>; Marcus Shawcroft
> <Marcus.Shawcroft@arm.com>; Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
> Subject: Re: [PATCH 2/2]AArch64 Add support for neg on v1df
> 
> Tamar Christina <Tamar.Christina@arm.com> writes:
> >> -----Original Message-----
> >> From: Richard Sandiford <richard.sandiford@arm.com>
> >> Sent: Friday, September 23, 2022 5:30 AM
> >> To: Tamar Christina <Tamar.Christina@arm.com>
> >> Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; Richard Earnshaw
> >> <Richard.Earnshaw@arm.com>; Marcus Shawcroft
> >> <Marcus.Shawcroft@arm.com>; Kyrylo Tkachov
> <Kyrylo.Tkachov@arm.com>
> >> Subject: Re: [PATCH 2/2]AArch64 Add support for neg on v1df
> >>
> >> Tamar Christina <tamar.christina@arm.com> writes:
> >> > Hi All,
> >> >
> >> > This adds support for using scalar fneg on the V1DF type.
> >> >
> >> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> >> >
> >> > Ok for master?
> >>
> >> Why just this one operation though?  Couldn't we extend iterators
> >> like
> >> GPF_F16 to include V1DF, avoiding the need for new patterns?
> >>
> >
> > Simply because it's the only one I know how to generate code for.
> > I can change GPF_F16 but I don't know under which circumstances we'd
> > generate a V1DF for the other operations.
> 
> We'd do it for things like:
> 
> __Float64x1_t foo (__Float64x1_t x) { return -x; }
> 
> if the pattern is available, instead of using subregs.  So one way would be to
> scan the expand rtl dump for subregs.

Ahh yes, I forgot about that ACLE type.

> 
> If the point is that there is no observable difference between defining 1-
> element vector ops and not, except for this one case, then that suggests we
> should handle this case in target-independent code instead.  There's no point
> forcing every target that has V1DF to define a duplicate of the DF neg
> pattern.

My original approach was to indeed use DF instead of V1DF, however since we
do define V1DF I had expected the mode to be somewhat usable.

So I'm happy to do whichever one you prefer now that I know how to test it.
I can either change my mid-end code, or extend the coverage of V1DF, any preference? 😊

Tamar

> 
> Thanks,
> Richard
> >
> > So if it's ok to do so without full test coverage I'm happy to do so...
> >
> > Tamar.
> >
> >> Richard
> >>
> >> >
> >> > Thanks,
> >> > Tamar
> >> >
> >> > gcc/ChangeLog:
> >> >
> >> > 	* config/aarch64/aarch64-simd.md (negv1df2): New.
> >> >
> >> > gcc/testsuite/ChangeLog:
> >> >
> >> > 	* gcc.target/aarch64/simd/addsub_2.c: New test.
> >> >
> >> > --- inline copy of patch --
> >> > diff --git a/gcc/config/aarch64/aarch64-simd.md
> >> > b/gcc/config/aarch64/aarch64-simd.md
> >> > index
> >> >
> >>
> f4152160084d6b6f34bd69f0ba6386c1ab50f77e..cf8c094bd4b76981cef2dd5dd7
> >> b8
> >> > e6be0d56101f 100644
> >> > --- a/gcc/config/aarch64/aarch64-simd.md
> >> > +++ b/gcc/config/aarch64/aarch64-simd.md
> >> > @@ -2713,6 +2713,14 @@ (define_insn "neg<mode>2"
> >> >    [(set_attr "type" "neon_fp_neg_<stype><q>")]
> >> >  )
> >> >
> >> > +(define_insn "negv1df2"
> >> > + [(set (match_operand:V1DF 0 "register_operand" "=w")
> >> > +       (neg:V1DF (match_operand:V1DF 1 "register_operand" "w")))]
> >> > +"TARGET_SIMD"
> >> > + "fneg\\t%d0, %d1"
> >> > +  [(set_attr "type" "neon_fp_neg_d")]
> >> > +)
> >> > +
> >> >  (define_insn "abs<mode>2"
> >> >   [(set (match_operand:VHSDF 0 "register_operand" "=w")
> >> >         (abs:VHSDF (match_operand:VHSDF 1 "register_operand"
> >> > "w")))] diff --git
> >> > a/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c
> >> > b/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c
> >> > new file mode 100644
> >> > index
> >> >
> >>
> 0000000000000000000000000000000000000000..55a7365e897f8af509de953129
> >> e0
> >> > f516974f7ca8
> >> > --- /dev/null
> >> > +++ b/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c
> >> > @@ -0,0 +1,22 @@
> >> > +/* { dg-do compile } */
> >> > +/* { dg-options "-Ofast" } */
> >> > +/* { dg-final { check-function-bodies "**" "" "" { target { le } }
> >> > +} } */
> >> > +
> >> > +#pragma GCC target "+nosve"
> >> > +
> >> > +/*
> >> > +** f1:
> >> > +** ...
> >> > +**	fneg	d[0-9]+, d[0-9]+
> >> > +**	fadd	v[0-9]+.2s, v[0-9]+.2s, v[0-9]+.2s
> >> > +** ...
> >> > +*/
> >> > +void f1 (float *restrict a, float *restrict b, float *res, int n) {
> >> > +   for (int i = 0; i < 2; i+=2)
> >> > +    {
> >> > +      res[i+0] = a[i+0] + b[i+0];
> >> > +      res[i+1] = a[i+1] - b[i+1];
> >> > +    }
> >> > +}
> >> > +

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2]AArch64 Add support for neg on v1df
  2022-09-23 10:10       ` Tamar Christina
@ 2022-09-23 14:01         ` Richard Sandiford
  2022-09-26  8:26           ` Richard Biener
  0 siblings, 1 reply; 7+ messages in thread
From: Richard Sandiford @ 2022-09-23 14:01 UTC (permalink / raw)
  To: Tamar Christina
  Cc: gcc-patches, nd, Richard Earnshaw, Marcus Shawcroft,
	Kyrylo Tkachov, rguenther

Tamar Christina <Tamar.Christina@arm.com> writes:
>> -----Original Message-----
>> From: Richard Sandiford <richard.sandiford@arm.com>
>> Sent: Friday, September 23, 2022 6:04 AM
>> To: Tamar Christina <Tamar.Christina@arm.com>
>> Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; Richard Earnshaw
>> <Richard.Earnshaw@arm.com>; Marcus Shawcroft
>> <Marcus.Shawcroft@arm.com>; Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
>> Subject: Re: [PATCH 2/2]AArch64 Add support for neg on v1df
>> 
>> Tamar Christina <Tamar.Christina@arm.com> writes:
>> >> -----Original Message-----
>> >> From: Richard Sandiford <richard.sandiford@arm.com>
>> >> Sent: Friday, September 23, 2022 5:30 AM
>> >> To: Tamar Christina <Tamar.Christina@arm.com>
>> >> Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; Richard Earnshaw
>> >> <Richard.Earnshaw@arm.com>; Marcus Shawcroft
>> >> <Marcus.Shawcroft@arm.com>; Kyrylo Tkachov
>> <Kyrylo.Tkachov@arm.com>
>> >> Subject: Re: [PATCH 2/2]AArch64 Add support for neg on v1df
>> >>
>> >> Tamar Christina <tamar.christina@arm.com> writes:
>> >> > Hi All,
>> >> >
>> >> > This adds support for using scalar fneg on the V1DF type.
>> >> >
>> >> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
>> >> >
>> >> > Ok for master?
>> >>
>> >> Why just this one operation though?  Couldn't we extend iterators
>> >> like
>> >> GPF_F16 to include V1DF, avoiding the need for new patterns?
>> >>
>> >
>> > Simply because it's the only one I know how to generate code for.
>> > I can change GPF_F16 but I don't know under which circumstances we'd
>> > generate a V1DF for the other operations.
>> 
>> We'd do it for things like:
>> 
>> __Float64x1_t foo (__Float64x1_t x) { return -x; }
>> 
>> if the pattern is available, instead of using subregs.  So one way would be to
>> scan the expand rtl dump for subregs.
>
> Ahh yes, I forgot about that ACLE type.
>
>> 
>> If the point is that there is no observable difference between defining 1-
>> element vector ops and not, except for this one case, then that suggests we
>> should handle this case in target-independent code instead.  There's no point
>> forcing every target that has V1DF to define a duplicate of the DF neg
>> pattern.
>
> My original approach was to indeed use DF instead of V1DF, however since we
> do define V1DF I had expected the mode to be somewhat usable.
>
> So I'm happy to do whichever one you prefer now that I know how to test it.
> I can either change my mid-end code, or extend the coverage of V1DF, any preference? 😊

I don't mind really, as long as we're consistent.  Maybe Richi has an opinion.

If he doesn't mind either, then I guess it makes sense to define the ops
as completely as possible (e.g. equivalently to V2SF), although it doesn't
need to be all in one go.

Thanks,
Richard

> Tamar
>
>> 
>> Thanks,
>> Richard
>> >
>> > So if it's ok to do so without full test coverage I'm happy to do so...
>> >
>> > Tamar.
>> >
>> >> Richard
>> >>
>> >> >
>> >> > Thanks,
>> >> > Tamar
>> >> >
>> >> > gcc/ChangeLog:
>> >> >
>> >> > 	* config/aarch64/aarch64-simd.md (negv1df2): New.
>> >> >
>> >> > gcc/testsuite/ChangeLog:
>> >> >
>> >> > 	* gcc.target/aarch64/simd/addsub_2.c: New test.
>> >> >
>> >> > --- inline copy of patch --
>> >> > diff --git a/gcc/config/aarch64/aarch64-simd.md
>> >> > b/gcc/config/aarch64/aarch64-simd.md
>> >> > index
>> >> >
>> >>
>> f4152160084d6b6f34bd69f0ba6386c1ab50f77e..cf8c094bd4b76981cef2dd5dd7
>> >> b8
>> >> > e6be0d56101f 100644
>> >> > --- a/gcc/config/aarch64/aarch64-simd.md
>> >> > +++ b/gcc/config/aarch64/aarch64-simd.md
>> >> > @@ -2713,6 +2713,14 @@ (define_insn "neg<mode>2"
>> >> >    [(set_attr "type" "neon_fp_neg_<stype><q>")]
>> >> >  )
>> >> >
>> >> > +(define_insn "negv1df2"
>> >> > + [(set (match_operand:V1DF 0 "register_operand" "=w")
>> >> > +       (neg:V1DF (match_operand:V1DF 1 "register_operand" "w")))]
>> >> > +"TARGET_SIMD"
>> >> > + "fneg\\t%d0, %d1"
>> >> > +  [(set_attr "type" "neon_fp_neg_d")]
>> >> > +)
>> >> > +
>> >> >  (define_insn "abs<mode>2"
>> >> >   [(set (match_operand:VHSDF 0 "register_operand" "=w")
>> >> >         (abs:VHSDF (match_operand:VHSDF 1 "register_operand"
>> >> > "w")))] diff --git
>> >> > a/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c
>> >> > b/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c
>> >> > new file mode 100644
>> >> > index
>> >> >
>> >>
>> 0000000000000000000000000000000000000000..55a7365e897f8af509de953129
>> >> e0
>> >> > f516974f7ca8
>> >> > --- /dev/null
>> >> > +++ b/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c
>> >> > @@ -0,0 +1,22 @@
>> >> > +/* { dg-do compile } */
>> >> > +/* { dg-options "-Ofast" } */
>> >> > +/* { dg-final { check-function-bodies "**" "" "" { target { le } }
>> >> > +} } */
>> >> > +
>> >> > +#pragma GCC target "+nosve"
>> >> > +
>> >> > +/*
>> >> > +** f1:
>> >> > +** ...
>> >> > +**	fneg	d[0-9]+, d[0-9]+
>> >> > +**	fadd	v[0-9]+.2s, v[0-9]+.2s, v[0-9]+.2s
>> >> > +** ...
>> >> > +*/
>> >> > +void f1 (float *restrict a, float *restrict b, float *res, int n) {
>> >> > +   for (int i = 0; i < 2; i+=2)
>> >> > +    {
>> >> > +      res[i+0] = a[i+0] + b[i+0];
>> >> > +      res[i+1] = a[i+1] - b[i+1];
>> >> > +    }
>> >> > +}
>> >> > +

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2]AArch64 Add support for neg on v1df
  2022-09-23 14:01         ` Richard Sandiford
@ 2022-09-26  8:26           ` Richard Biener
  0 siblings, 0 replies; 7+ messages in thread
From: Richard Biener @ 2022-09-26  8:26 UTC (permalink / raw)
  To: Richard Sandiford
  Cc: Tamar Christina, gcc-patches, nd, Richard Earnshaw,
	Marcus Shawcroft, Kyrylo Tkachov

On Fri, 23 Sep 2022, Richard Sandiford wrote:

> Tamar Christina <Tamar.Christina@arm.com> writes:
> >> -----Original Message-----
> >> From: Richard Sandiford <richard.sandiford@arm.com>
> >> Sent: Friday, September 23, 2022 6:04 AM
> >> To: Tamar Christina <Tamar.Christina@arm.com>
> >> Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; Richard Earnshaw
> >> <Richard.Earnshaw@arm.com>; Marcus Shawcroft
> >> <Marcus.Shawcroft@arm.com>; Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
> >> Subject: Re: [PATCH 2/2]AArch64 Add support for neg on v1df
> >> 
> >> Tamar Christina <Tamar.Christina@arm.com> writes:
> >> >> -----Original Message-----
> >> >> From: Richard Sandiford <richard.sandiford@arm.com>
> >> >> Sent: Friday, September 23, 2022 5:30 AM
> >> >> To: Tamar Christina <Tamar.Christina@arm.com>
> >> >> Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; Richard Earnshaw
> >> >> <Richard.Earnshaw@arm.com>; Marcus Shawcroft
> >> >> <Marcus.Shawcroft@arm.com>; Kyrylo Tkachov
> >> <Kyrylo.Tkachov@arm.com>
> >> >> Subject: Re: [PATCH 2/2]AArch64 Add support for neg on v1df
> >> >>
> >> >> Tamar Christina <tamar.christina@arm.com> writes:
> >> >> > Hi All,
> >> >> >
> >> >> > This adds support for using scalar fneg on the V1DF type.
> >> >> >
> >> >> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> >> >> >
> >> >> > Ok for master?
> >> >>
> >> >> Why just this one operation though?  Couldn't we extend iterators
> >> >> like
> >> >> GPF_F16 to include V1DF, avoiding the need for new patterns?
> >> >>
> >> >
> >> > Simply because it's the only one I know how to generate code for.
> >> > I can change GPF_F16 but I don't know under which circumstances we'd
> >> > generate a V1DF for the other operations.
> >> 
> >> We'd do it for things like:
> >> 
> >> __Float64x1_t foo (__Float64x1_t x) { return -x; }
> >> 
> >> if the pattern is available, instead of using subregs.  So one way would be to
> >> scan the expand rtl dump for subregs.
> >
> > Ahh yes, I forgot about that ACLE type.
> >
> >> 
> >> If the point is that there is no observable difference between defining 1-
> >> element vector ops and not, except for this one case, then that suggests we
> >> should handle this case in target-independent code instead.  There's no point
> >> forcing every target that has V1DF to define a duplicate of the DF neg
> >> pattern.
> >
> > My original approach was to indeed use DF instead of V1DF, however since we
> > do define V1DF I had expected the mode to be somewhat usable.
> >
> > So I'm happy to do whichever one you prefer now that I know how to test it.
> > I can either change my mid-end code, or extend the coverage of V1DF, any preference? ?
> 
> I don't mind really, as long as we're consistent.  Maybe Richi has an opinion.
> 
> If he doesn't mind either, then I guess it makes sense to define the ops
> as completely as possible (e.g. equivalently to V2SF), although it doesn't
> need to be all in one go.

I don't mind either, we'll see if theres a target vector registers
not overlapping FP regisers at some point, then it probably matters
so it does seem we should support both variants from the middle-end
at least.  If we have some noop-conversion target hook that tells
us this RTL expansion could use a fallback generating subregs
for V1mode modes.

Richard.

> Thanks,
> Richard
> 
> > Tamar
> >
> >> 
> >> Thanks,
> >> Richard
> >> >
> >> > So if it's ok to do so without full test coverage I'm happy to do so...
> >> >
> >> > Tamar.
> >> >
> >> >> Richard
> >> >>
> >> >> >
> >> >> > Thanks,
> >> >> > Tamar
> >> >> >
> >> >> > gcc/ChangeLog:
> >> >> >
> >> >> > 	* config/aarch64/aarch64-simd.md (negv1df2): New.
> >> >> >
> >> >> > gcc/testsuite/ChangeLog:
> >> >> >
> >> >> > 	* gcc.target/aarch64/simd/addsub_2.c: New test.
> >> >> >
> >> >> > --- inline copy of patch --
> >> >> > diff --git a/gcc/config/aarch64/aarch64-simd.md
> >> >> > b/gcc/config/aarch64/aarch64-simd.md
> >> >> > index
> >> >> >
> >> >>
> >> f4152160084d6b6f34bd69f0ba6386c1ab50f77e..cf8c094bd4b76981cef2dd5dd7
> >> >> b8
> >> >> > e6be0d56101f 100644
> >> >> > --- a/gcc/config/aarch64/aarch64-simd.md
> >> >> > +++ b/gcc/config/aarch64/aarch64-simd.md
> >> >> > @@ -2713,6 +2713,14 @@ (define_insn "neg<mode>2"
> >> >> >    [(set_attr "type" "neon_fp_neg_<stype><q>")]
> >> >> >  )
> >> >> >
> >> >> > +(define_insn "negv1df2"
> >> >> > + [(set (match_operand:V1DF 0 "register_operand" "=w")
> >> >> > +       (neg:V1DF (match_operand:V1DF 1 "register_operand" "w")))]
> >> >> > +"TARGET_SIMD"
> >> >> > + "fneg\\t%d0, %d1"
> >> >> > +  [(set_attr "type" "neon_fp_neg_d")]
> >> >> > +)
> >> >> > +
> >> >> >  (define_insn "abs<mode>2"
> >> >> >   [(set (match_operand:VHSDF 0 "register_operand" "=w")
> >> >> >         (abs:VHSDF (match_operand:VHSDF 1 "register_operand"
> >> >> > "w")))] diff --git
> >> >> > a/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c
> >> >> > b/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c
> >> >> > new file mode 100644
> >> >> > index
> >> >> >
> >> >>
> >> 0000000000000000000000000000000000000000..55a7365e897f8af509de953129
> >> >> e0
> >> >> > f516974f7ca8
> >> >> > --- /dev/null
> >> >> > +++ b/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c
> >> >> > @@ -0,0 +1,22 @@
> >> >> > +/* { dg-do compile } */
> >> >> > +/* { dg-options "-Ofast" } */
> >> >> > +/* { dg-final { check-function-bodies "**" "" "" { target { le } }
> >> >> > +} } */
> >> >> > +
> >> >> > +#pragma GCC target "+nosve"
> >> >> > +
> >> >> > +/*
> >> >> > +** f1:
> >> >> > +** ...
> >> >> > +**	fneg	d[0-9]+, d[0-9]+
> >> >> > +**	fadd	v[0-9]+.2s, v[0-9]+.2s, v[0-9]+.2s
> >> >> > +** ...
> >> >> > +*/
> >> >> > +void f1 (float *restrict a, float *restrict b, float *res, int n) {
> >> >> > +   for (int i = 0; i < 2; i+=2)
> >> >> > +    {
> >> >> > +      res[i+0] = a[i+0] + b[i+0];
> >> >> > +      res[i+1] = a[i+1] - b[i+1];
> >> >> > +    }
> >> >> > +}
> >> >> > +
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-09-26  8:26 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-23  9:17 [PATCH 2/2]AArch64 Add support for neg on v1df Tamar Christina
2022-09-23  9:30 ` Richard Sandiford
2022-09-23  9:43   ` Tamar Christina
2022-09-23 10:03     ` Richard Sandiford
2022-09-23 10:10       ` Tamar Christina
2022-09-23 14:01         ` Richard Sandiford
2022-09-26  8:26           ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).