| Message ID | 0f9bde3b-e6f3-1d4f-cf76-6ac345bfe07d@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | rs6000: Fix vec_cpsgn parameter order (PR101985) | expand |
Hi! Ping, please. :) Bill On 9/24/21 10:20 AM, Bill Schmidt wrote: > Hi! > > This fixes a bug reported in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101985. > > The vec_cpsgn built-in function API differs in argument order from the > copysign<mode>3 convention. Currently that pattern is incorrctly used to > implement vec_cpsgn. Fix that while leaving the existing pattern in place > to implement copysignf for vector modes. > > Part of the fix when using the new built-in support requires an adjustment > to a pending patch that replaces much of altivec.h with an automatically > generated file. So that adjustment will be coming later... > > Also fix a bug in the new built-in overload infrastructure where we were > using the VSX form of the VEC_COPYSIGN built-in when we should default to > the VMX form. > > Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no regressions. > Is this okay for trunk? > > Thanks! > Bill > > > 2021-09-24 Bill Schmidt <wschmidt@linux.ibm.com> > > gcc/ > PR target/101985 > * config/rs6000/altivec.h (vec_cpsgn): Adjust. > * config/rs6000/rs6000-overload.def (VEC_COPYSIGN): Use SKIP to > avoid generating an automatic #define of vec_cpsgn. Use the > correct built-in for V4SFmode that doesn't depend on VSX. > > gcc/testsuite/ > PR target/101985 > * gcc.target/powerpc/pr101985.c: New. > --- > gcc/config/rs6000/altivec.h | 2 +- > gcc/config/rs6000/rs6000-overload.def | 4 ++-- > gcc/testsuite/gcc.target/powerpc/pr101985-1.c | 18 ++++++++++++++++++ > gcc/testsuite/gcc.target/powerpc/pr101985-2.c | 18 ++++++++++++++++++ > 4 files changed, 39 insertions(+), 3 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/powerpc/pr101985-1.c > create mode 100644 gcc/testsuite/gcc.target/powerpc/pr101985-2.c > > diff --git a/gcc/config/rs6000/altivec.h b/gcc/config/rs6000/altivec.h > index 5b631c7ebaf..ea72c9c1789 100644 > --- a/gcc/config/rs6000/altivec.h > +++ b/gcc/config/rs6000/altivec.h > @@ -129,7 +129,7 @@ > #define vec_vcfux __builtin_vec_vcfux > #define vec_cts __builtin_vec_cts > #define vec_ctu __builtin_vec_ctu > -#define vec_cpsgn __builtin_vec_copysign > +#define vec_cpsgn(x,y) __builtin_vec_copysign(y,x) > #define vec_double __builtin_vec_double > #define vec_doublee __builtin_vec_doublee > #define vec_doubleo __builtin_vec_doubleo > diff --git a/gcc/config/rs6000/rs6000-overload.def b/gcc/config/rs6000/rs6000-overload.def > index 141f831e2c0..4f583312f36 100644 > --- a/gcc/config/rs6000/rs6000-overload.def > +++ b/gcc/config/rs6000/rs6000-overload.def > @@ -1154,9 +1154,9 @@ > vus __builtin_vec_convert_4f32_8f16 (vf, vf); > CONVERT_4F32_8F16 > > -[VEC_COPYSIGN, vec_cpsgn, __builtin_vec_copysign] > +[VEC_COPYSIGN, SKIP, __builtin_vec_copysign] > vf __builtin_vec_copysign (vf, vf); > - CPSGNSP > + COPYSIGN_V4SF > vd __builtin_vec_copysign (vd, vd); > CPSGNDP > > diff --git a/gcc/testsuite/gcc.target/powerpc/pr101985-1.c b/gcc/testsuite/gcc.target/powerpc/pr101985-1.c > new file mode 100644 > index 00000000000..a1ec2d68d53 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/pr101985-1.c > @@ -0,0 +1,18 @@ > +/* PR target/101985 */ > +/* { dg-do run } */ > +/* { dg-require-effective-target vsx_hw } */ > +/* { dg-options "-O2" } */ > + > +#include <altivec.h> > + > +int > +main (void) > +{ > + vector float a = { 1, 2, - 3, - 4}; > + vector float b = {-10, 20, -30, 40}; > + vector float c = { 10, 20, -30, -40}; > + a = vec_cpsgn (a, b); > + if (! vec_all_eq (a, c)) > + __builtin_abort (); > + return 0; > +} > diff --git a/gcc/testsuite/gcc.target/powerpc/pr101985-2.c b/gcc/testsuite/gcc.target/powerpc/pr101985-2.c > new file mode 100644 > index 00000000000..71cc254c170 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/pr101985-2.c > @@ -0,0 +1,18 @@ > +/* PR target/101985 */ > +/* { dg-do run } */ > +/* { dg-require-effective-target vsx_hw } */ > +/* { dg-options "-O2" } */ > + > +#include <altivec.h> > + > +int > +main (void) > +{ > + vector double a = { 1, -4}; > + vector double b = { -10, 40}; > + vector double c = { 10, -40}; > + a = vec_cpsgn (a, b); > + if (! vec_all_eq (a, c)) > + __builtin_abort (); > + return 0; > +}
On Fri, Sep 24, 2021 at 11:20 AM Bill Schmidt <wschmidt@linux.ibm.com> wrote: > > Hi! > > This fixes a bug reported in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101985. > > The vec_cpsgn built-in function API differs in argument order from the > copysign<mode>3 convention. Currently that pattern is incorrectly used to > implement vec_cpsgn. Fix that while leaving the existing pattern in place > to implement copysignf for vector modes. It's a little confusing what "that" is. Maybe clarify that the patch is changing the PowerPC VSX function to invoke the GCC built-in with the argument in the correct order. > > Part of the fix when using the new built-in support requires an adjustment > to a pending patch that replaces much of altivec.h with an automatically > generated file. So that adjustment will be coming later... > > Also fix a bug in the new built-in overload infrastructure where we were > using the VSX form of the VEC_COPYSIGN built-in when we should default to > the VMX form. > > Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no regressions. > Is this okay for trunk? > > Thanks! > Bill > > > 2021-09-24 Bill Schmidt <wschmidt@linux.ibm.com> > > gcc/ > PR target/101985 > * config/rs6000/altivec.h (vec_cpsgn): Adjust. Maybe a little more information than "Adjust"? Swap arguments? > * config/rs6000/rs6000-overload.def (VEC_COPYSIGN): Use SKIP to > avoid generating an automatic #define of vec_cpsgn. Use the > correct built-in for V4SFmode that doesn't depend on VSX. > > gcc/testsuite/ > PR target/101985 > * gcc.target/powerpc/pr101985.c: New. Okay. Thanks, David
On Fri, Sep 24, 2021 at 10:20:46AM -0500, Bill Schmidt wrote: > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/pr101985-1.c > @@ -0,0 +1,18 @@ > +/* PR target/101985 */ > +/* { dg-do run } */ > +/* { dg-require-effective-target vsx_hw } */ > +/* { dg-options "-O2" } */ If you need vsx_hw (or vsx_ok), you need -mvsx in the options as well. (Always, so in both testcases here). Segher
Hi! On 10/12/21 4:37 PM, Segher Boessenkool wrote: > On Fri, Sep 24, 2021 at 10:20:46AM -0500, Bill Schmidt wrote: >> --- /dev/null >> +++ b/gcc/testsuite/gcc.target/powerpc/pr101985-1.c >> @@ -0,0 +1,18 @@ >> +/* PR target/101985 */ >> +/* { dg-do run } */ >> +/* { dg-require-effective-target vsx_hw } */ >> +/* { dg-options "-O2" } */ > If you need vsx_hw (or vsx_ok), you need -mvsx in the options as well. > (Always, so in both testcases here). Whoops. Fixed, and adjusted ChangeLog/commit message per David. Committed. Thanks for the reviews!! Bill > > > Segher
diff --git a/gcc/config/rs6000/altivec.h b/gcc/config/rs6000/altivec.h index 5b631c7ebaf..ea72c9c1789 100644 --- a/gcc/config/rs6000/altivec.h +++ b/gcc/config/rs6000/altivec.h @@ -129,7 +129,7 @@ #define vec_vcfux __builtin_vec_vcfux #define vec_cts __builtin_vec_cts #define vec_ctu __builtin_vec_ctu -#define vec_cpsgn __builtin_vec_copysign +#define vec_cpsgn(x,y) __builtin_vec_copysign(y,x) #define vec_double __builtin_vec_double #define vec_doublee __builtin_vec_doublee #define vec_doubleo __builtin_vec_doubleo diff --git a/gcc/config/rs6000/rs6000-overload.def b/gcc/config/rs6000/rs6000-overload.def index 141f831e2c0..4f583312f36 100644 --- a/gcc/config/rs6000/rs6000-overload.def +++ b/gcc/config/rs6000/rs6000-overload.def @@ -1154,9 +1154,9 @@ vus __builtin_vec_convert_4f32_8f16 (vf, vf); CONVERT_4F32_8F16 -[VEC_COPYSIGN, vec_cpsgn, __builtin_vec_copysign] +[VEC_COPYSIGN, SKIP, __builtin_vec_copysign] vf __builtin_vec_copysign (vf, vf); - CPSGNSP + COPYSIGN_V4SF vd __builtin_vec_copysign (vd, vd); CPSGNDP diff --git a/gcc/testsuite/gcc.target/powerpc/pr101985-1.c b/gcc/testsuite/gcc.target/powerpc/pr101985-1.c new file mode 100644 index 00000000000..a1ec2d68d53 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/pr101985-1.c @@ -0,0 +1,18 @@ +/* PR target/101985 */ +/* { dg-do run } */ +/* { dg-require-effective-target vsx_hw } */ +/* { dg-options "-O2" } */ + +#include <altivec.h> + +int +main (void) +{ + vector float a = { 1, 2, - 3, - 4}; + vector float b = {-10, 20, -30, 40}; + vector float c = { 10, 20, -30, -40}; + a = vec_cpsgn (a, b); + if (! vec_all_eq (a, c)) + __builtin_abort (); + return 0; +} diff --git a/gcc/testsuite/gcc.target/powerpc/pr101985-2.c b/gcc/testsuite/gcc.target/powerpc/pr101985-2.c new file mode 100644 index 00000000000..71cc254c170 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/pr101985-2.c @@ -0,0 +1,18 @@ +/* PR target/101985 */ +/* { dg-do run } */ +/* { dg-require-effective-target vsx_hw } */ +/* { dg-options "-O2" } */ + +#include <altivec.h> + +int +main (void) +{ + vector double a = { 1, -4}; + vector double b = { -10, 40}; + vector double c = { 10, -40}; + a = vec_cpsgn (a, b); + if (! vec_all_eq (a, c)) + __builtin_abort (); + return 0; +}