Re: PowerPC: Update long double IEEE 128-bit tests.
On Fri, Nov 06, 2020 at 11:45:21PM -0500, Michael Meissner wrote: > On Mon, Nov 02, 2020 at 07:00:15PM -0600, Segher Boessenkool wrote: > > > + /* 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; > > Yes a double-double can hold more mantissa bits than a double, but the > exponent > size is the same (which is what I'm testing). But that is not what the comment says. My remark was about the comment. It is confusing as is. > > > +#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 > > > +#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? > > I can put in a better comment. However, this test fails because it explicitly > does not include math.h and it uses -fno-builtin. So the compiler can't > effectively map the nextafter math function. So either the compiler is wrong, or the test is? Or I do not grasp what you mean to say at all :-( Segher
Re: PowerPC: Update long double IEEE 128-bit tests.
On Mon, Nov 02, 2020 at 07:00:15PM -0600, Segher Boessenkool wrote: > 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 > > #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; Yes a double-double can hold more mantissa bits than a double, but the exponent size is the same (which is what I'm testing). > 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. I will have to look at it. > > * 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 > > +#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? I can put in a better comment. However, this test fails because it explicitly does not include math.h and it uses -fno-builtin. So the compiler can't effectively map the nextafter math function. > > * 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 :-) ) I will have to look into it. -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
Re: PowerPC: Update long double IEEE 128-bit tests.
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 > #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 > +#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
Re: PowerPC: Update long double IEEE 128-bit tests.
On Thu, 2020-10-22 at 18:07 -0400, Michael Meissner via Gcc-patches wrote: > PowerPC: Update long double IEEE 128-bit tests. > > I have split all of these patches into separate patches to hopefully get them > into the tree. > > This patch fixes 3 tests in the testsuite that fail if long double is set > to IEEE 128-bit. > > I have tested this patch with bootstrap builds on a little endian power9 > system > running Linux. With the other patches, I have built two full bootstrap builds > using this patch and the patches after this patch. One build used the current > default for long double (IBM extended double) and the other build switched the > default to IEEE 128-bit. I used the Advance Toolchain AT 14.0 compiler as the > library used by this compiler. There are no regressions between the tests. > There are 3 fortran benchmarks (ieee/large_2.f90, default_format_2.f90, and > default_format_denormal_2.f90) that now pass. > > Can I install this into the trunk? > > We have gotten some requests to back port these changes to GCC 10.x. At the > moment, I am not planning to do the back port, but I may need to in the > future. > > gcc/testsuite/ > 2020-10-22 Michael Meissner > > * c-c++-common/dfp/convert-bfp-11.c: If long double is IEEE > 128-bit, skip the test. > * gcc.dg/nextafter-2.c: On PowerPC, if long double is IEEE > 128-bit, include math.h to get the built-in mapped correctly. > * gcc.target/powerpc/pr70117.c: Add support for long double being > IEEE 128-bit. > --- > gcc/testsuite/c-c++-common/dfp/convert-bfp-11.c | 7 +++ > gcc/testsuite/gcc.dg/nextafter-2.c | 10 ++ > gcc/testsuite/gcc.target/powerpc/pr70117.c | 6 -- > 3 files changed, 21 insertions(+), 2 deletions(-) > > diff --git a/gcc/testsuite/c-c++-common/dfp/convert-bfp-11.c > b/gcc/testsuite/c-c++-common/dfp/convert-bfp-11.c > index 95c433d2c24..6ee0c1c6ae9 100644 > --- 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 > #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. */ > + if (LDBL_MAX_10_EXP > DBL_MAX_10_EXP) > +return 0; > + >convert_101 (); >convert_102 (); > > 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. */ Great comment. :-) > +#include > +#endif > + > #include "nextafter-1.c" > 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 > > #else > #error "long double must be either IBM 128-bit or IEEE 128-bit" > @@ -75,10 +77,10 @@ main (void) >if (__builtin_isnormal (ld)) > __builtin_abort (); > > - ld = LDBL_MAX; > + ld = IBM128_MAX; >if (!__builtin_isnormal (ld)) > __builtin_abort (); > - ld = -LDBL_MAX; > + ld = -IBM128_MAX; >if (!__builtin_isnormal (ld)) > __builtin_abort (); > ok lgtm, thanks -Will > -- > 2.22.0 > >
PowerPC: Update long double IEEE 128-bit tests.
PowerPC: Update long double IEEE 128-bit tests. I have split all of these patches into separate patches to hopefully get them into the tree. This patch fixes 3 tests in the testsuite that fail if long double is set to IEEE 128-bit. I have tested this patch with bootstrap builds on a little endian power9 system running Linux. With the other patches, I have built two full bootstrap builds using this patch and the patches after this patch. One build used the current default for long double (IBM extended double) and the other build switched the default to IEEE 128-bit. I used the Advance Toolchain AT 14.0 compiler as the library used by this compiler. There are no regressions between the tests. There are 3 fortran benchmarks (ieee/large_2.f90, default_format_2.f90, and default_format_denormal_2.f90) that now pass. Can I install this into the trunk? We have gotten some requests to back port these changes to GCC 10.x. At the moment, I am not planning to do the back port, but I may need to in the future. gcc/testsuite/ 2020-10-22 Michael Meissner * c-c++-common/dfp/convert-bfp-11.c: If long double is IEEE 128-bit, skip the test. * gcc.dg/nextafter-2.c: On PowerPC, if long double is IEEE 128-bit, include math.h to get the built-in mapped correctly. * gcc.target/powerpc/pr70117.c: Add support for long double being IEEE 128-bit. --- gcc/testsuite/c-c++-common/dfp/convert-bfp-11.c | 7 +++ gcc/testsuite/gcc.dg/nextafter-2.c | 10 ++ gcc/testsuite/gcc.target/powerpc/pr70117.c | 6 -- 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/gcc/testsuite/c-c++-common/dfp/convert-bfp-11.c b/gcc/testsuite/c-c++-common/dfp/convert-bfp-11.c index 95c433d2c24..6ee0c1c6ae9 100644 --- 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 #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. */ + if (LDBL_MAX_10_EXP > DBL_MAX_10_EXP) +return 0; + convert_101 (); convert_102 (); 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 +#endif + #include "nextafter-1.c" 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 #else #error "long double must be either IBM 128-bit or IEEE 128-bit" @@ -75,10 +77,10 @@ main (void) if (__builtin_isnormal (ld)) __builtin_abort (); - ld = LDBL_MAX; + ld = IBM128_MAX; if (!__builtin_isnormal (ld)) __builtin_abort (); - ld = -LDBL_MAX; + ld = -IBM128_MAX; if (!__builtin_isnormal (ld)) __builtin_abort (); -- 2.22.0 -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
