[PATCH] rs6000, Add missing overloaded bcd builtin tests

[PATCH] rs6000, Add missing overloaded bcd builtin tests

[Carl Love]

GCC maintainers:

The following patch adds tests for two of the rs6000 overloaded built-
ins that do not have tests.  Additionally the GCC documentation file
doc/extend.texi is updated to include the built-in definitions as they
were missing.

The patch has been tested on a Power 10 system with no regressions. 
Please let me know if this patch is acceptable for mainline.

                 Carl

-------------------------------------------
rs6000, Add missing overloaded bcd builtin tests

The two BCD overloaded built-ins __builtin_bcdsub_ge and __builtin_bcdsub_le
do not have a corresponding test.  Add tests to existing test file and update
the documentation with the built-in definitions.

gcc/ChangeLog:
	* doc/extend.texi (__builtin_bcdsub_le, __builtin_bcdsub_ge): Add
	documentation for the builti-ins.

gcc/testsuite/ChangeLog:
	* bcd-3.c (do_sub_ge, do_suble): Add functions to test builtins
	__builtin_bcdsub_ge and __builtin_bcdsub_le).
---
 gcc/doc/extend.texi                      |  4 ++++
 gcc/testsuite/gcc.target/powerpc/bcd-3.c | 22 +++++++++++++++++++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index cf0d0c63cce..fa7402813e7 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -20205,12 +20205,16 @@ int __builtin_bcdadd_ov (vector unsigned char, vector unsigned char, const int);
 vector __int128 __builtin_bcdsub (vector __int128, vector __int128, const int);
 vector unsigned char __builtin_bcdsub (vector unsigned char, vector unsigned char,
                                        const int);
+int __builtin_bcdsub_le (vector __int128, vector __int128, const int);
+int __builtin_bcdsub_le (vector unsigned char, vector unsigned char, const int);
 int __builtin_bcdsub_lt (vector __int128, vector __int128, const int);
 int __builtin_bcdsub_lt (vector unsigned char, vector unsigned char, const int);
 int __builtin_bcdsub_eq (vector __int128, vector __int128, const int);
 int __builtin_bcdsub_eq (vector unsigned char, vector unsigned char, const int);
 int __builtin_bcdsub_gt (vector __int128, vector __int128, const int);
 int __builtin_bcdsub_gt (vector unsigned char, vector unsigned char, const int);
+int __builtin_bcdsub_ge (vector __int128, vector __int128, const int);
+int __builtin_bcdsub_ge (vector unsigned char, vector unsigned char, const int);
 int __builtin_bcdsub_ov (vector __int128, vector __int128, const int);
 int __builtin_bcdsub_ov (vector unsigned char, vector unsigned char, const int);
 @end smallexample
diff --git a/gcc/testsuite/gcc.target/powerpc/bcd-3.c b/gcc/testsuite/gcc.target/powerpc/bcd-3.c
index 7948a0c95e2..9891f4ff08e 100644
--- a/gcc/testsuite/gcc.target/powerpc/bcd-3.c
+++ b/gcc/testsuite/gcc.target/powerpc/bcd-3.c
@@ -3,7 +3,7 @@
 /* { dg-require-effective-target powerpc_p8vector_ok } */
 /* { dg-options "-mdejagnu-cpu=power8 -O2" } */
 /* { dg-final { scan-assembler-times "bcdadd\[.\] " 4 } } */
-/* { dg-final { scan-assembler-times "bcdsub\[.\] " 4 } } */
+/* { dg-final { scan-assembler-times "bcdsub\[.\] " 6 } } */
 /* { dg-final { scan-assembler-not   "bl __builtin"   } } */
 /* { dg-final { scan-assembler-not   "mtvsr"   	      } } */
 /* { dg-final { scan-assembler-not   "mfvsr"   	      } } */
@@ -93,6 +93,26 @@ do_sub_gt (vector_128_t a, vector_128_t b, int *p)
   return ret;
 }
 
+vector_128_t
+do_sub_ge (vector_128_t a, vector_128_t b, int *p)
+{
+  vector_128_t ret = __builtin_bcdsub (a, b, 0);
+  if (__builtin_bcdsub_ge (a, b, 0))
+    *p = 1;
+
+  return ret;
+}
+
+vector_128_t
+do_sub_le (vector_128_t a, vector_128_t b, int *p)
+{
+  vector_128_t ret = __builtin_bcdsub (a, b, 0);
+  if (__builtin_bcdsub_le (a, b, 0))
+    *p = 1;
+
+  return ret;
+}
+
 vector_128_t
 do_sub_ov (vector_128_t a, vector_128_t b, int *p)
 {
-- 
2.37.2

Re: [PATCH] rs6000, Add missing overloaded bcd builtin tests

[Kewen.Lin]

Hi Carl,

on 2023/10/31 08:08, Carl Love wrote:
> GCC maintainers:
> 
> The following patch adds tests for two of the rs6000 overloaded built-
> ins that do not have tests.  Additionally the GCC documentation file

I just found that actually they have the test coverage, because we have

#define __builtin_bcdcmpeq(a,b)   __builtin_vec_bcdsub_eq(a,b,0)
#define __builtin_bcdcmpgt(a,b)   __builtin_vec_bcdsub_gt(a,b,0)
#define __builtin_bcdcmplt(a,b)   __builtin_vec_bcdsub_lt(a,b,0)
#define __builtin_bcdcmpge(a,b)   __builtin_vec_bcdsub_ge(a,b,0)
#define __builtin_bcdcmple(a,b)   __builtin_vec_bcdsub_le(a,b,0)

in altivec.h and gcc/testsuite/gcc.target/powerpc/bcd-4.c tests all these
__builtin_bcdcmp* ...

> doc/extend.texi is updated to include the built-in definitions as they
> were missing.

... since we already document __builtin_vec_bcdsub_{eq,gt,lt}, I think
it's still good to supplement the documentation and add the explicit
testing cases.

> 
> The patch has been tested on a Power 10 system with no regressions. 
> Please let me know if this patch is acceptable for mainline.
> 
>                  Carl
> 
> -------------------------------------------
> rs6000, Add missing overloaded bcd builtin tests
> 
> The two BCD overloaded built-ins __builtin_bcdsub_ge and __builtin_bcdsub_le
> do not have a corresponding test.  Add tests to existing test file and update
> the documentation with the built-in definitions.

As above, this commit log doesn't describe the actuality well, please update
it with something like:

Currently we have the documentation for __builtin_vec_bcdsub_{eq,gt,lt} but
not for __builtin_bcdsub_[gl]e, this patch is to supplement the descriptions
for them.  Although they are mainly for __builtin_bcdcmp{ge,le}, we already
have some testing coverage for __builtin_vec_bcdsub_{eq,gt,lt}, this patch
adds the corresponding explicit test cases as well.

> 
> gcc/ChangeLog:
> 	* doc/extend.texi (__builtin_bcdsub_le, __builtin_bcdsub_ge): Add
> 	documentation for the builti-ins.
> 
> gcc/testsuite/ChangeLog:
> 	* bcd-3.c (do_sub_ge, do_suble): Add functions to test builtins
> 	__builtin_bcdsub_ge and __builtin_bcdsub_le).

1) Unexpected ")" at the end.

2) I supposed git gcc-verify would complain on this changelog entry.

Should be starting with:

	* gcc.target/powerpc/bcd-3.c (....

, no?

OK for trunk with the above comments addressed, thanks!

BR,
Kewen

> ---
>  gcc/doc/extend.texi                      |  4 ++++
>  gcc/testsuite/gcc.target/powerpc/bcd-3.c | 22 +++++++++++++++++++++-
>  2 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index cf0d0c63cce..fa7402813e7 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -20205,12 +20205,16 @@ int __builtin_bcdadd_ov (vector unsigned char, vector unsigned char, const int);
>  vector __int128 __builtin_bcdsub (vector __int128, vector __int128, const int);
>  vector unsigned char __builtin_bcdsub (vector unsigned char, vector unsigned char,
>                                         const int);
> +int __builtin_bcdsub_le (vector __int128, vector __int128, const int);
> +int __builtin_bcdsub_le (vector unsigned char, vector unsigned char, const int);
>  int __builtin_bcdsub_lt (vector __int128, vector __int128, const int);
>  int __builtin_bcdsub_lt (vector unsigned char, vector unsigned char, const int);
>  int __builtin_bcdsub_eq (vector __int128, vector __int128, const int);
>  int __builtin_bcdsub_eq (vector unsigned char, vector unsigned char, const int);
>  int __builtin_bcdsub_gt (vector __int128, vector __int128, const int);
>  int __builtin_bcdsub_gt (vector unsigned char, vector unsigned char, const int);
> +int __builtin_bcdsub_ge (vector __int128, vector __int128, const int);
> +int __builtin_bcdsub_ge (vector unsigned char, vector unsigned char, const int);
>  int __builtin_bcdsub_ov (vector __int128, vector __int128, const int);
>  int __builtin_bcdsub_ov (vector unsigned char, vector unsigned char, const int);
>  @end smallexample
> diff --git a/gcc/testsuite/gcc.target/powerpc/bcd-3.c b/gcc/testsuite/gcc.target/powerpc/bcd-3.c
> index 7948a0c95e2..9891f4ff08e 100644
> --- a/gcc/testsuite/gcc.target/powerpc/bcd-3.c
> +++ b/gcc/testsuite/gcc.target/powerpc/bcd-3.c
> @@ -3,7 +3,7 @@
>  /* { dg-require-effective-target powerpc_p8vector_ok } */
>  /* { dg-options "-mdejagnu-cpu=power8 -O2" } */
>  /* { dg-final { scan-assembler-times "bcdadd\[.\] " 4 } } */
> -/* { dg-final { scan-assembler-times "bcdsub\[.\] " 4 } } */
> +/* { dg-final { scan-assembler-times "bcdsub\[.\] " 6 } } */
>  /* { dg-final { scan-assembler-not   "bl __builtin"   } } */
>  /* { dg-final { scan-assembler-not   "mtvsr"   	      } } */
>  /* { dg-final { scan-assembler-not   "mfvsr"   	      } } */
> @@ -93,6 +93,26 @@ do_sub_gt (vector_128_t a, vector_128_t b, int *p)
>    return ret;
>  }
>  
> +vector_128_t
> +do_sub_ge (vector_128_t a, vector_128_t b, int *p)
> +{
> +  vector_128_t ret = __builtin_bcdsub (a, b, 0);
> +  if (__builtin_bcdsub_ge (a, b, 0))
> +    *p = 1;
> +
> +  return ret;
> +}
> +
> +vector_128_t
> +do_sub_le (vector_128_t a, vector_128_t b, int *p)
> +{
> +  vector_128_t ret = __builtin_bcdsub (a, b, 0);
> +  if (__builtin_bcdsub_le (a, b, 0))
> +    *p = 1;
> +
> +  return ret;
> +}
> +
>  vector_128_t
>  do_sub_ov (vector_128_t a, vector_128_t b, int *p)
>  {

Re: [PATCH] rs6000, Add missing overloaded bcd builtin tests

[Carl Love]

On Tue, 2023-10-31 at 10:34 +0800, Kewen.Lin wrote:
> Hi Carl,
> 
> on 2023/10/31 08:08, Carl Love wrote:
> > GCC maintainers:
> > 
> > The following patch adds tests for two of the rs6000 overloaded
> > built-
> > ins that do not have tests.  Additionally the GCC documentation
> > file
> 
> I just found that actually they have the test coverage, because we
> have
> 
> #define __builtin_bcdcmpeq(a,b)   __builtin_vec_bcdsub_eq(a,b,0)
> #define __builtin_bcdcmpgt(a,b)   __builtin_vec_bcdsub_gt(a,b,0)
> #define __builtin_bcdcmplt(a,b)   __builtin_vec_bcdsub_lt(a,b,0)
> #define __builtin_bcdcmpge(a,b)   __builtin_vec_bcdsub_ge(a,b,0)
> #define __builtin_bcdcmple(a,b)   __builtin_vec_bcdsub_le(a,b,0)
> 
> in altivec.h and gcc/testsuite/gcc.target/powerpc/bcd-4.c tests all
> these

OK, my simple scripts are not going to pickup the stuff in altivec.h. 
They were just grepping for the built-in name in the test file
directory.

> __builtin_bcdcmp* ...
> 
> > doc/extend.texi is updated to include the built-in definitions as
> > they
> > were missing.
> 
> ... since we already document __builtin_vec_bcdsub_{eq,gt,lt}, I
> think
> it's still good to supplement the documentation and add the explicit
> testing cases.
> 
> > The patch has been tested on a Power 10 system with no
> > regressions. 
> > Please let me know if this patch is acceptable for mainline.
> > 
> >                  Carl
> > 
> > -------------------------------------------
> > rs6000, Add missing overloaded bcd builtin tests
> > 
> > The two BCD overloaded built-ins __builtin_bcdsub_ge and
> > __builtin_bcdsub_le
> > do not have a corresponding test.  Add tests to existing test file
> > and update
> > the documentation with the built-in definitions.
> 
> As above, this commit log doesn't describe the actuality well, please
> update
> it with something like:
> 
> Currently we have the documentation for
> __builtin_vec_bcdsub_{eq,gt,lt} but
> not for __builtin_bcdsub_[gl]e, this patch is to supplement the
> descriptions
> for them.  Although they are mainly for __builtin_bcdcmp{ge,le}, we
> already
> have some testing coverage for __builtin_vec_bcdsub_{eq,gt,lt}, this
> patch
> adds the corresponding explicit test cases as well.
> 

OK, replaced the commit log with the suggestion.

> > gcc/ChangeLog:
> > 	* doc/extend.texi (__builtin_bcdsub_le, __builtin_bcdsub_ge):
> > Add
> > 	documentation for the builti-ins.
> > 
> > gcc/testsuite/ChangeLog:
> > 	* bcd-3.c (do_sub_ge, do_suble): Add functions to test builtins
> > 	__builtin_bcdsub_ge and __builtin_bcdsub_le).
> 
> 1) Unexpected ")" at the end.
> 
> 2) I supposed git gcc-verify would complain on this changelog entry.
> 
> Should be starting with:
> 
> 	* gcc.target/powerpc/bcd-3.c (....
> 
> , no?
> 

Yes, I ment to run the commit check but obviously got distracted and
didn't.  Sorry about that.  

> OK for trunk with the above comments addressed, thanks!
> 
OK, thanks.

                    Carl 

> BR,
> Kewen
> 
> > ---
> >  gcc/doc/extend.texi                      |  4 ++++
> >  gcc/testsuite/gcc.target/powerpc/bcd-3.c | 22
> > +++++++++++++++++++++-
> >  2 files changed, 25 insertions(+), 1 deletion(-)
> > 
> > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> > index cf0d0c63cce..fa7402813e7 100644
> > --- a/gcc/doc/extend.texi
> > +++ b/gcc/doc/extend.texi
> > @@ -20205,12 +20205,16 @@ int __builtin_bcdadd_ov (vector unsigned
> > char, vector unsigned char, const int);
> >  vector __int128 __builtin_bcdsub (vector __int128, vector
> > __int128, const int);
> >  vector unsigned char __builtin_bcdsub (vector unsigned char,
> > vector unsigned char,
> >                                         const int);
> > +int __builtin_bcdsub_le (vector __int128, vector __int128, const
> > int);
> > +int __builtin_bcdsub_le (vector unsigned char, vector unsigned
> > char, const int);
> >  int __builtin_bcdsub_lt (vector __int128, vector __int128, const
> > int);
> >  int __builtin_bcdsub_lt (vector unsigned char, vector unsigned
> > char, const int);
> >  int __builtin_bcdsub_eq (vector __int128, vector __int128, const
> > int);
> >  int __builtin_bcdsub_eq (vector unsigned char, vector unsigned
> > char, const int);
> >  int __builtin_bcdsub_gt (vector __int128, vector __int128, const
> > int);
> >  int __builtin_bcdsub_gt (vector unsigned char, vector unsigned
> > char, const int);
> > +int __builtin_bcdsub_ge (vector __int128, vector __int128, const
> > int);
> > +int __builtin_bcdsub_ge (vector unsigned char, vector unsigned
> > char, const int);
> >  int __builtin_bcdsub_ov (vector __int128, vector __int128, const
> > int);
> >  int __builtin_bcdsub_ov (vector unsigned char, vector unsigned
> > char, const int);
> >  @end smallexample
> > diff --git a/gcc/testsuite/gcc.target/powerpc/bcd-3.c
> > b/gcc/testsuite/gcc.target/powerpc/bcd-3.c
> > index 7948a0c95e2..9891f4ff08e 100644
> > --- a/gcc/testsuite/gcc.target/powerpc/bcd-3.c
> > +++ b/gcc/testsuite/gcc.target/powerpc/bcd-3.c
> > @@ -3,7 +3,7 @@
> >  /* { dg-require-effective-target powerpc_p8vector_ok } */
> >  /* { dg-options "-mdejagnu-cpu=power8 -O2" } */
> >  /* { dg-final { scan-assembler-times "bcdadd\[.\] " 4 } } */
> > -/* { dg-final { scan-assembler-times "bcdsub\[.\] " 4 } } */
> > +/* { dg-final { scan-assembler-times "bcdsub\[.\] " 6 } } */
> >  /* { dg-final { scan-assembler-not   "bl __builtin"   } } */
> >  /* { dg-final { scan-assembler-not   "mtvsr"   	      } } */
> >  /* { dg-final { scan-assembler-not   "mfvsr"   	      } } */
> > @@ -93,6 +93,26 @@ do_sub_gt (vector_128_t a, vector_128_t b, int
> > *p)
> >    return ret;
> >  }
> >  
> > +vector_128_t
> > +do_sub_ge (vector_128_t a, vector_128_t b, int *p)
> > +{
> > +  vector_128_t ret = __builtin_bcdsub (a, b, 0);
> > +  if (__builtin_bcdsub_ge (a, b, 0))
> > +    *p = 1;
> > +
> > +  return ret;
> > +}
> > +
> > +vector_128_t
> > +do_sub_le (vector_128_t a, vector_128_t b, int *p)
> > +{
> > +  vector_128_t ret = __builtin_bcdsub (a, b, 0);
> > +  if (__builtin_bcdsub_le (a, b, 0))
> > +    *p = 1;
> > +
> > +  return ret;
> > +}
> > +
> >  vector_128_t
> >  do_sub_ov (vector_128_t a, vector_128_t b, int *p)
> >  {

Re: [PATCH] rs6000, Add missing overloaded bcd builtin tests

[Segher Boessenkool]

On Tue, Oct 31, 2023 at 08:31:25AM -0700, Carl Love wrote:
> > I just found that actually they have the test coverage, because we
> > have
> > 
> > #define __builtin_bcdcmpeq(a,b)   __builtin_vec_bcdsub_eq(a,b,0)
> > #define __builtin_bcdcmpgt(a,b)   __builtin_vec_bcdsub_gt(a,b,0)
> > #define __builtin_bcdcmplt(a,b)   __builtin_vec_bcdsub_lt(a,b,0)
> > #define __builtin_bcdcmpge(a,b)   __builtin_vec_bcdsub_ge(a,b,0)
> > #define __builtin_bcdcmple(a,b)   __builtin_vec_bcdsub_le(a,b,0)
> > 
> > in altivec.h and gcc/testsuite/gcc.target/powerpc/bcd-4.c tests all
> > these
> 
> OK, my simple scripts are not going to pickup the stuff in altivec.h. 
> They were just grepping for the built-in name in the test file
> directory.

You could use gcov to see which rs6000 builtins are not exercised by
anything in the testsuite, maybe.  This probably can be automated pretty
nicely.


Segher

Re: [PATCH] rs6000, Add missing overloaded bcd builtin tests

[Carl Love]

Segher:

On Tue, 2023-10-31 at 11:17 -0500, Segher Boessenkool wrote:

<snip>
> 
> You could use gcov to see which rs6000 builtins are not exercised by
> anything in the testsuite, maybe.  This probably can be automated
> pretty
> nicely.

I will take a look at gcov.  I just did some relatively simple scripts
to go look for test cases.  For the non-overloaded built-ins, the
scrips had to exclude built-ins referenced by the overloaded built-ins.

This patch is just the first of a series of patches that I am working
on to try and clean up the built-in stuff per some comments in a PR. 
The internal LTC issue is
 
https://github.ibm.com/ltc-toolchain/power-gcc/issues/1288

The goal is to make sure there are test cases and documentation for all
of the overloaded and non overloaded built-in definitions.  Just a low
priority project to fill any spare cycles.  :-)

                      Carl