Re: [PATCH][aarch64] Put vector fnma instruction into canonical form for better code generation.

[James Greenhalgh]

On Fri, Oct 06, 2017 at 10:01:21PM +0100, Steve Ellcey wrote:
> This patch is a follow up to a discussion at:
> 
> https://gcc.gnu.org/ml/gcc/2017-06/msg00126.html
> 
> For some reason the simd version of fnma in aarch64-simd.md
> is not in the canonical form of having the neg operator on 
> the first operand and instead has it on the second.  This 
> results in sub-optimal code generation (an extra dup instruction).
> 
> I have moved the 'neg', rebuilt GCC and retested with this patch
> There were no regressions.  OK to checkin?

OK.

Thanks,
James

Reviewed-by: James Greenhalgh 

> 
> 
> 2017-10-06  Steve Ellcey  
> 
>   * config/aarch64/aarch64-simd.md (fnma4): Move neg operator
>   to canonical location.
> 
> 
> diff --git a/gcc/config/aarch64/aarch64-simd.md 
> b/gcc/config/aarch64/aarch64-sim
> d.md
> index 12da8be..d9ced50 100644
> --- a/gcc/config/aarch64/aarch64-simd.md
> +++ b/gcc/config/aarch64/aarch64-simd.md
> @@ -1777,9 +1777,8 @@
>  (define_insn "fnma4"
>    [(set (match_operand:VHSDF 0 "register_operand" "=w")
>   (fma:VHSDF
> -   (match_operand:VHSDF 1 "register_operand" "w")
> -  (neg:VHSDF
> - (match_operand:VHSDF 2 "register_operand" "w"))
> +   (neg:VHSDF (match_operand:VHSDF 1 "register_operand" "w"))
> +   (match_operand:VHSDF 2 "register_operand" "w")
>     (match_operand:VHSDF 3 "register_operand" "0")))]
>    "TARGET_SIMD"
>    "fmls\\t%0., %1., %2."
> 
> 
> 
> 2017-10-06  Steve Ellcey  
> 
>   * gcc.target/aarch64/fmls.c: New test.
> 
> 
> diff --git a/gcc/testsuite/gcc.target/aarch64/fmls.c 
> b/gcc/testsuite/gcc.target/
> aarch64/fmls.c
> index e69de29..1ea0e6a 100644
> --- a/gcc/testsuite/gcc.target/aarch64/fmls.c
> +++ b/gcc/testsuite/gcc.target/aarch64/fmls.c
> @@ -0,0 +1,19 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3" } */
> +
> +#define vector __attribute__((vector_size(16)))
> +vector double a = {1.0,1.0};
> +vector double b = {2.0,2.0};
> +double x = 3.0;
> +
> +
> +void __attribute__ ((noinline))
> +vf (double x, vector double *v1, vector double *v2, vector double *result)
> +{
> +  vector double s = v1[0];
> +  vector double t = -v2[0];
> +  vector double m = {x,x};
> +  vector double r = t * m + s;
> +  result[0] = r;
> +}
> +/* { dg-final { scan-assembler-not "dup" } } */
> 

Re: [PATCH][aarch64] Put vector fnma instruction into canonical form for better code generation.

[Steve Ellcey]

Re-ping with an added cc to the aarch64 maintainers.

Steve Ellcey

On Tue, 2017-10-24 at 11:06 -0700, Steve Ellcey wrote:
> Ping.
> 
> Steve Ellcey
> 
> On Fri, 2017-10-06 at 14:01 -0700, Steve Ellcey wrote:
> > 
> > This patch is a follow up to a discussion at:
> > 
> > https://gcc.gnu.org/ml/gcc/2017-06/msg00126.html
> > 
> > For some reason the simd version of fnma in aarch64-simd.md
> > is not in the canonical form of having the neg operator on 
> > the first operand and instead has it on the second.  This 
> > results in sub-optimal code generation (an extra dup instruction).
> > 
> > I have moved the 'neg', rebuilt GCC and retested with this patch
> > There were no regressions.  OK to checkin?
> > 
> > 
> > 2017-10-06  Steve Ellcey  
> > 
> > * config/aarch64/aarch64-simd.md (fnma4): Move neg
> > operator
> > to canonical location.
> > 
> > 
> > diff --git a/gcc/config/aarch64/aarch64-simd.md
> > b/gcc/config/aarch64/aarch64-sim
> > d.md
> > index 12da8be..d9ced50 100644
> > --- a/gcc/config/aarch64/aarch64-simd.md
> > +++ b/gcc/config/aarch64/aarch64-simd.md
> > @@ -1777,9 +1777,8 @@
> >  (define_insn "fnma4"
> >    [(set (match_operand:VHSDF 0 "register_operand" "=w")
> >     (fma:VHSDF
> > -     (match_operand:VHSDF 1 "register_operand" "w")
> > -  (neg:VHSDF
> > -   (match_operand:VHSDF 2 "register_operand" "w"))
> > +     (neg:VHSDF (match_operand:VHSDF 1 "register_operand"
> > "w"))
> > +     (match_operand:VHSDF 2 "register_operand" "w")
> >       (match_operand:VHSDF 3 "register_operand" "0")))]
> >    "TARGET_SIMD"
> >    "fmls\\t%0., %1., %2."
> > 
> > 
> > 
> > 2017-10-06  Steve Ellcey  
> > 
> > * gcc.target/aarch64/fmls.c: New test.
> > 
> > 
> > diff --git a/gcc/testsuite/gcc.target/aarch64/fmls.c
> > b/gcc/testsuite/gcc.target/
> > aarch64/fmls.c
> > index e69de29..1ea0e6a 100644
> > --- a/gcc/testsuite/gcc.target/aarch64/fmls.c
> > +++ b/gcc/testsuite/gcc.target/aarch64/fmls.c
> > @@ -0,0 +1,19 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O3" } */
> > +
> > +#define vector __attribute__((vector_size(16)))
> > +vector double a = {1.0,1.0};
> > +vector double b = {2.0,2.0};
> > +double x = 3.0;
> > +
> > +
> > +void __attribute__ ((noinline))
> > +vf (double x, vector double *v1, vector double *v2, vector double
> > *result)
> > +{
> > +  vector double s = v1[0];
> > +  vector double t = -v2[0];
> > +  vector double m = {x,x};
> > +  vector double r = t * m + s;
> > +  result[0] = r;
> > +}
> > +/* { dg-final { scan-assembler-not "dup" } } */

Re: [PATCH][aarch64] Put vector fnma instruction into canonical form for better code generation.

[Steve Ellcey]

Ping.

Steve Ellcey

On Fri, 2017-10-06 at 14:01 -0700, Steve Ellcey wrote:
> This patch is a follow up to a discussion at:
> 
> https://gcc.gnu.org/ml/gcc/2017-06/msg00126.html
> 
> For some reason the simd version of fnma in aarch64-simd.md
> is not in the canonical form of having the neg operator on 
> the first operand and instead has it on the second.  This 
> results in sub-optimal code generation (an extra dup instruction).
> 
> I have moved the 'neg', rebuilt GCC and retested with this patch
> There were no regressions.  OK to checkin?
> 
> 
> 2017-10-06  Steve Ellcey  
> 
>   * config/aarch64/aarch64-simd.md (fnma4): Move neg
> operator
>   to canonical location.
> 
> 
> diff --git a/gcc/config/aarch64/aarch64-simd.md
> b/gcc/config/aarch64/aarch64-sim
> d.md
> index 12da8be..d9ced50 100644
> --- a/gcc/config/aarch64/aarch64-simd.md
> +++ b/gcc/config/aarch64/aarch64-simd.md
> @@ -1777,9 +1777,8 @@
>  (define_insn "fnma4"
>    [(set (match_operand:VHSDF 0 "register_operand" "=w")
>   (fma:VHSDF
> -   (match_operand:VHSDF 1 "register_operand" "w")
> -  (neg:VHSDF
> - (match_operand:VHSDF 2 "register_operand" "w"))
> +   (neg:VHSDF (match_operand:VHSDF 1 "register_operand" "w"))
> +   (match_operand:VHSDF 2 "register_operand" "w")
>     (match_operand:VHSDF 3 "register_operand" "0")))]
>    "TARGET_SIMD"
>    "fmls\\t%0., %1., %2."
> 
> 
> 
> 2017-10-06  Steve Ellcey  
> 
>   * gcc.target/aarch64/fmls.c: New test.
> 
> 
> diff --git a/gcc/testsuite/gcc.target/aarch64/fmls.c
> b/gcc/testsuite/gcc.target/
> aarch64/fmls.c
> index e69de29..1ea0e6a 100644
> --- a/gcc/testsuite/gcc.target/aarch64/fmls.c
> +++ b/gcc/testsuite/gcc.target/aarch64/fmls.c
> @@ -0,0 +1,19 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3" } */
> +
> +#define vector __attribute__((vector_size(16)))
> +vector double a = {1.0,1.0};
> +vector double b = {2.0,2.0};
> +double x = 3.0;
> +
> +
> +void __attribute__ ((noinline))
> +vf (double x, vector double *v1, vector double *v2, vector double
> *result)
> +{
> +  vector double s = v1[0];
> +  vector double t = -v2[0];
> +  vector double m = {x,x};
> +  vector double r = t * m + s;
> +  result[0] = r;
> +}
> +/* { dg-final { scan-assembler-not "dup" } } */

[PATCH][aarch64] Put vector fnma instruction into canonical form for better code generation.

[Steve Ellcey]

This patch is a follow up to a discussion at:

https://gcc.gnu.org/ml/gcc/2017-06/msg00126.html

For some reason the simd version of fnma in aarch64-simd.md
is not in the canonical form of having the neg operator on 
the first operand and instead has it on the second.  This 
results in sub-optimal code generation (an extra dup instruction).

I have moved the 'neg', rebuilt GCC and retested with this patch
There were no regressions.  OK to checkin?


2017-10-06  Steve Ellcey  

* config/aarch64/aarch64-simd.md (fnma4): Move neg operator
to canonical location.


diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-sim
d.md
index 12da8be..d9ced50 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -1777,9 +1777,8 @@
 (define_insn "fnma4"
   [(set (match_operand:VHSDF 0 "register_operand" "=w")
    (fma:VHSDF
-     (match_operand:VHSDF 1 "register_operand" "w")
-  (neg:VHSDF
-   (match_operand:VHSDF 2 "register_operand" "w"))
+     (neg:VHSDF (match_operand:VHSDF 1 "register_operand" "w"))
+     (match_operand:VHSDF 2 "register_operand" "w")
      (match_operand:VHSDF 3 "register_operand" "0")))]
   "TARGET_SIMD"
   "fmls\\t%0., %1., %2."



2017-10-06  Steve Ellcey  

* gcc.target/aarch64/fmls.c: New test.


diff --git a/gcc/testsuite/gcc.target/aarch64/fmls.c b/gcc/testsuite/gcc.target/
aarch64/fmls.c
index e69de29..1ea0e6a 100644
--- a/gcc/testsuite/gcc.target/aarch64/fmls.c
+++ b/gcc/testsuite/gcc.target/aarch64/fmls.c
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+#define vector __attribute__((vector_size(16)))
+vector double a = {1.0,1.0};
+vector double b = {2.0,2.0};
+double x = 3.0;
+
+
+void __attribute__ ((noinline))
+vf (double x, vector double *v1, vector double *v2, vector double *result)
+{
+  vector double s = v1[0];
+  vector double t = -v2[0];
+  vector double m = {x,x};
+  vector double r = t * m + s;
+  result[0] = r;
+}
+/* { dg-final { scan-assembler-not "dup" } } */