diff mbox series

[2/2] AArch64 Add support for neg on v1df

Message ID patch-16239-tamar@arm.com
State New, archived
Headers
Series [1/2] middle-end: RFC: On expansion of conditional branches, give hint if argument is a truth type to backend |

Commit Message

Tamar Christina Sept. 23, 2022, 9:17 a.m. UTC 
  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




--
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];
+    }
+}
+

Comments

Richard Sandiford Sept. 23, 2022, 9:30 a.m. UTC | #1 
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];
> +    }
> +}
> +
Tamar Christina Sept. 23, 2022, 9:43 a.m. UTC | #2 
> -----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];
> > +    }
> > +}
> > +
Richard Sandiford Sept. 23, 2022, 10:03 a.m. UTC | #3 
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];
>> > +    }
>> > +}
>> > +
Tamar Christina Sept. 23, 2022, 10:10 a.m. UTC | #4 
> -----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];
> >> > +    }
> >> > +}
> >> > +
Richard Sandiford Sept. 23, 2022, 2:01 p.m. UTC | #5 
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];
>> >> > +    }
>> >> > +}
>> >> > +
Richard Biener Sept. 26, 2022, 8:26 a.m. UTC | #6 
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];
> >> >> > +    }
> >> >> > +}
> >> >> > +
>
diff mbox series

Patch

--- 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];
+    }
+}
+