Message ID | 1507323681.22616.7.camel@cavium.com |
---|---|
State | New |
Headers | show |
Series | [aarch64] Put vector fnma instruction into canonical form for better code generation. | expand |
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 <sellcey@cavium.com> > > * config/aarch64/aarch64-simd.md (fnma<mode>4): 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 "fnma<mode>4" > [(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.<Vtype>, %1.<Vtype>, %2.<Vtype>" > > > > 2017-10-06 Steve Ellcey <sellcey@cavium.com> > > * 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-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 <sellcey@cavium.com> > > > > * config/aarch64/aarch64-simd.md (fnma<mode>4): 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 "fnma<mode>4" > > [(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.<Vtype>, %1.<Vtype>, %2.<Vtype>" > > > > > > > > 2017-10-06 Steve Ellcey <sellcey@cavium.com> > > > > * 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" } } */
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 <james.greenhalgh@arm.com> > > > 2017-10-06 Steve Ellcey <sellcey@cavium.com> > > * config/aarch64/aarch64-simd.md (fnma<mode>4): 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 "fnma<mode>4" > [(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.<Vtype>, %1.<Vtype>, %2.<Vtype>" > > > > 2017-10-06 Steve Ellcey <sellcey@cavium.com> > > * 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" } } */ >
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 "fnma<mode>4" [(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.<Vtype>, %1.<Vtype>, %2.<Vtype>" 2017-10-06 Steve Ellcey <sellcey@cavium.com> * 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" } } */