Hi,
on 2024/4/17 13:12, HAO CHEN GUI wrote:
> Hi,
> This patch fixes loss of return statement in maxbcd of bcd-4.c. Without
> return statement, it returns an invalid bcd number and make the test
> noneffective. The patch also enables test to run on Power9 and Big Endian,
> as all bcd instructions are supported from Power9.
>
> Bootstrapped and tested on powerpc64-linux BE and LE with no
> regressions. Is it OK for the trunk?
OK with two nits below tweaked, thanks!
>
> Thanks
> Gui Haochen
>
>
> ChangeLog
> rs6000: Fix bcd test case
>
> gcc/testsuite/
> * gcc.target/powerpc/bcd-4.c: Enable the case to be tested on Power9.
> Enable the case to be run on big endian. Fix function maxbcd and
> other misc. problems.
>
>
> patch.diff
> diff --git a/gcc/testsuite/gcc.target/powerpc/bcd-4.c
> b/gcc/testsuite/gcc.target/powerpc/bcd-4.c
> index 2c8554dfe82..8c0bac2720f 100644
> --- a/gcc/testsuite/gcc.target/powerpc/bcd-4.c
> +++ b/gcc/testsuite/gcc.target/powerpc/bcd-4.c
> @@ -1,7 +1,7 @@
> /* { dg-do run } */
> /* { dg-require-effective-target int128 } */
> -/* { dg-require-effective-target power10_hw } */
> -/* { dg-options "-mdejagnu-cpu=power10 -O2 -save-temps" } */
> +/* { dg-require-effective-target p9vector_hw } */
> +/* { dg-options "-mdejagnu-cpu=power9 -O2 -save-temps" } */
> /* { dg-final { scan-assembler-times {\mbcdadd\M} 7 } } */
> /* { dg-final { scan-assembler-times {\mbcdsub\M} 18 } } */
> /* { dg-final { scan-assembler-times {\mbcds\M} 2 } } */
> @@ -44,10 +44,20 @@ vector unsigned char maxbcd(unsigned int sign)
> vector unsigned char result;
> int i;
>
> +#ifdef _BIG_ENDIAN
> + for (i = 0; i < 15; i++)
> +#else
> for (i = 15; i > 0; i--)
> +#endif
> result[i] = 0x99;
>
> - result[0] = sign << 4 | 0x9;
> +#ifdef _BIG_ENDIAN
Nit: I'd prefer __BIG_ENDIAN__, although both work in most cases,
RS6000_CPU_CPP_ENDIAN_BUILTINS in netbsd.h doesn't define _BIG_ENDIAN.
So could you do a global replacement with __BIG_ENDIAN__?
> + result[15] = 0x90 | sign;
> +#else
> + result[0] = 0x90 | sign;
> +#endif
> +
> + return result;
> }
>
> vector unsigned char num2bcd(long int a, int encoding)
> @@ -70,9 +80,17 @@ vector unsigned char num2bcd(long int a, int encoding)
>
> hi = a % 10; // 1st digit
> a = a / 10;
> +#ifdef _BIG_ENDIAN
> + result[15] = hi << 4| sign;
> +#else
> result[0] = hi << 4| sign;
> +#endif
>
> +#ifdef _BIG_ENDIAN
> + for (i = 14; i >= 0; i--)
> +#else
> for (i = 1; i < 16; i++)
> +#endif
> {
> low = a % 10;
> a = a / 10;
> @@ -117,7 +135,11 @@ int main ()
> }
>
> /* result should be positive */
> +#ifdef _BIG_ENDIAN
> + if ((result[15] & 0xF) != BCD_POS0)
> +#else
> if ((result[0] & 0xF) != BCD_POS0)
> +#endif
> #if DEBUG
> printf("ERROR: __builtin_bcdadd sign of result is %d. Does not match "
> "expected_result = %d\n",
> @@ -150,7 +172,11 @@ int main ()
> }
>
> /* Result should be positive, alternate encoding. */
> +#ifdef _BIG_ENDIAN
> + if ((result[15] & 0xF) != BCD_POS1)
> +#else
> if ((result[0] & 0xF) != BCD_POS1)
> +#endif
> #if DEBUG
> printf("ERROR: __builtin_bcdadd sign of result is %d. Does not "
> "match expected_result = %d\n",
> @@ -183,7 +209,11 @@ int main ()
> }
>
> /* result should be negative */
> +#ifdef _BIG_ENDIAN
> + if ((result[15] & 0xF) != BCD_NEG)
> +#else
> if ((result[0] & 0xF) != BCD_NEG)
> +#endif
> #if DEBUG
> printf("ERROR: __builtin_bcdadd sign, neg of result is %d. Does not "
> "match expected_result = %d\n",
> @@ -217,7 +247,11 @@ int main ()
> }
>
> /* result should be positive, alt encoding */
Nit: This comment is inconsistent with the below check, may be:
/* result should be negative. */
BR,
Kewen
> +#ifdef _BIG_ENDIAN
> + if ((result[15] & 0xF) != BCD_NEG)
> +#else
> if ((result[0] & 0xF) != BCD_NEG)
> +#endif
> #if DEBUG
> printf("ERROR: __builtin_bcdadd sign, of result is %d. Does not match "
> "expected_result = %d\n",
> @@ -250,7 +284,11 @@ int main ()
> }
>
> /* result should be positive */
> +#ifdef _BIG_ENDIAN
> + if ((result[15] & 0xF) != BCD_POS1)
> +#else
> if ((result[0] & 0xF) != BCD_POS1)
> +#endif
> #if DEBUG
> printf("ERROR: __builtin_bcdsub sign, result is %d. Does not match "
> "expected_result = %d\n",
> @@ -283,7 +321,7 @@ int main ()
> abort();
> #endif
>
> - a = maxbcd(BCD_NEG);
> + a = maxbcd(BCD_POS0);
> b = maxbcd(BCD_NEG);
>
> if (__builtin_bcdsub_ofl (a, b, 0) == 0)
> @@ -462,8 +500,12 @@ int main ()
> }
>
> /* result should be positive */
> +#ifdef _BIG_ENDIAN
> + if ((result[15] & 0xF) != BCD_POS0)
> +#else
> if ((result[0] & 0xF) != BCD_POS0)
> -#if 0
> +#endif
> +#if DEBUG
> printf("ERROR: __builtin_bcdmul10 sign, result is %d. Does not match "
> "expected_result = %d\n",
> result[0] & 0xF, BCD_POS1);
> @@ -492,7 +534,11 @@ int main ()
> }
>
> /* result should be positive */
> +#ifdef _BIG_ENDIAN
> + if ((result[15] & 0xF) != BCD_POS0)
> +#else
> if ((result[0] & 0xF) != BCD_POS0)
> +#endif
> #if DEBUG
> printf("ERROR: __builtin_bcddiv10 sign, result is %d. Does not match "
> "expected_result = %d\n",
