Re: PowerPC: Update long double IEEE 128-bit tests.

[Segher Boessenkool <segher@kernel.crashing.org>]

Hi!

On Thu, Oct 22, 2020 at 06:07:14PM -0400, Michael Meissner wrote:
> This patch fixes 3 tests in the testsuite that fail if long double is set
> to IEEE 128-bit.

> 	* c-c++-common/dfp/convert-bfp-11.c: If long double is IEEE
> 	128-bit, skip the test.

> --- a/gcc/testsuite/c-c++-common/dfp/convert-bfp-11.c
> +++ b/gcc/testsuite/c-c++-common/dfp/convert-bfp-11.c
> @@ -5,6 +5,7 @@
>     Don't force 128-bit long doubles because runtime support depends
>     on glibc.  */
>  
> +#include <float.h>
>  #include "convert.h"
>  
>  volatile _Decimal32 sd;
> @@ -39,6 +40,12 @@ main ()
>    if (sizeof (long double) != 16)
>      return 0;
>  
> +  /* This test is written to test IBM extended double, which is a pair of
> +     doubles.  If long double can hold a larger value than a double can, such
> +     as when long double is IEEE 128-bit, just exit immediately.  */

A double-double can hold bigger values than a double can, as well
(if X is the biggest double, then X+Y is a valid double-double whenever
you take Y small enough).

> +  if (LDBL_MAX_10_EXP > DBL_MAX_10_EXP)
> +    return 0;

This is testing something different though: whether the base-10
logarithm of the maximum finite double is different from that of the
maximum finite double-double.

Is there no more direct test you can do?  Just test __FLOAT128__ maybe?
The test is not even compiled if not powerpc*-linux, so you can test
such macros just fine.

> 	* gcc.dg/nextafter-2.c: On PowerPC, if long double is IEEE
> 	128-bit, include math.h to get the built-in mapped correctly.

> diff --git a/gcc/testsuite/gcc.dg/nextafter-2.c b/gcc/testsuite/gcc.dg/nextafter-2.c
> index e51ae94be0c..64e9e3c485f 100644
> --- a/gcc/testsuite/gcc.dg/nextafter-2.c
> +++ b/gcc/testsuite/gcc.dg/nextafter-2.c
> @@ -13,4 +13,14 @@
>  #  define NO_LONG_DOUBLE 1
>  # endif
>  #endif
> +
> +#if defined(_ARCH_PPC) && defined(__LONG_DOUBLE_IEEE128__)
> +/* On PowerPC systems, long double uses either the IBM long double format, or
> +   IEEE 128-bit format.  The compiler switches the long double built-in
> +   function names and glibc switches the names when math.h is included.
> +   Because this test is run with -fno-builtin, include math.h so that the
> +   appropriate nextafter functions are called.  */
> +#include <math.h>
> +#endif
> +
>  #include "nextafter-1.c"

Please explain *what* mappings are made?  And why is it okay to do this
in the testsuite, when all "normal" code (that does not do this) will
just fail?

> 	* gcc.target/powerpc/pr70117.c: Add support for long double being
> 	IEEE 128-bit.

That is not what the patch does -- it instead changes the code because
it does not work correctly with long double ieee128 (which it already
did claim to support!)

So what are the actual changes doing, why are they correct, why was the
original not correct?

(It is easy to make a test not fail anymore: just delete it!  Something
here should be better than that :-) )

> diff --git a/gcc/testsuite/gcc.target/powerpc/pr70117.c b/gcc/testsuite/gcc.target/powerpc/pr70117.c
> index 3bbd2c595e0..928efe39c7b 100644
> --- a/gcc/testsuite/gcc.target/powerpc/pr70117.c
> +++ b/gcc/testsuite/gcc.target/powerpc/pr70117.c
> @@ -9,9 +9,11 @@
>     128-bit floating point, because the type is not enabled on those
>     systems.  */
>  #define LDOUBLE __ibm128
> +#define IBM128_MAX ((__ibm128) 1.79769313486231580793728971405301199e+308L)
>  
>  #elif defined(__LONG_DOUBLE_IBM128__)
>  #define LDOUBLE long double
> +#define IBM128_MAX LDBL_MAX

Can you define this in such a way that it always works?


Segher