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)
> {
