| Message ID | 20210707195837.GA28511@ibm-toto.the-meissners.org |
|---|---|
| State | New |
| Headers | show |
| Series | Repost: [PATCH] Fix long double tests when default long double is not IBM. | expand |
Hi Mike, On 7/7/21 2:58 PM, Michael Meissner wrote: > [PATCH] Fix long double tests when default long double is not IBM. > > This patch adds 3 more selections to target-supports.exp to see if we can force > the compiler to use a particular long double format (IEEE 128-bit, IBM extended > double, 64-bit), and the library support will track the changes for the long > double. This is needed because two of the tests in the test suite use long > double, and they are actually testing IBM extended double. > > This patch also forces the two tests that explicitly require long double > to use the IBM double-double encoding to explicitly run the test. This > requires GLIBC 2.32 or greater in order to do the switch. > > I have run tests on a little endian power9 system with 3 compilers. There were > no regressions with these patches, and the two tests in the following patches > now work if the default long double is not IBM 128-bit: > > * One compiler used the default IBM 128-bit format; > * One compiler used the IEEE 128-bit format; (and) > * One compiler used 64-bit long doubles. > > I have also tested compilers on a big endian power8 system with a compiler > defaulting to power8 code generation and another with the default cpu > set. There were no regressions. > > Can I check this patch into the master branch? > > 2021-07-07 Michael Meissner <meissner@linux.ibm.com> > > gcc/testsuite/ > PR target/70117 > * gcc.target/powerpc/pr70117.c: Force the long double type to use > the IBM 128-bit format. > * c-c++-common/dfp/convert-bfp-11.c: Force using IBM 128-bit long > double. Remove check for 64-bit long double. > * lib/target-supports.exp > (add_options_for_ppc_long_double_override_ibm128): New function. > (check_effective_target_ppc_long_double_override_ibm128): New > function. > (add_options_for_ppc_long_double_override_ieee128): New function. > (check_effective_target_ppc_long_double_override_ieee128): New > function. > (add_options_for_ppc_long_double_override_64bit): New function. > (check_effective_target_ppc_long_double_override_64bit): New > function. > --- > .../c-c++-common/dfp/convert-bfp-11.c | 18 +-- > gcc/testsuite/gcc.target/powerpc/pr70117.c | 6 +- > gcc/testsuite/lib/target-supports.exp | 107 ++++++++++++++++++ > 3 files changed, 121 insertions(+), 10 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..35da07d1fa4 100644 > --- a/gcc/testsuite/c-c++-common/dfp/convert-bfp-11.c > +++ b/gcc/testsuite/c-c++-common/dfp/convert-bfp-11.c > @@ -1,9 +1,14 @@ > -/* { dg-skip-if "" { ! "powerpc*-*-linux*" } } */ I suppose since the compile to check this in the effective target check is cached, this doesn't hurt much. Ok. > +/* { dg-require-effective-target dfp } */ > +/* { dg-require-effective-target ppc_long_double_override_ibm128 } */ > +/* { dg-add-options ppc_long_double_override_ibm128 } */ > > -/* Test decimal float conversions to and from IBM 128-bit long double. > - Checks are skipped at runtime if long double is not 128 bits. > - Don't force 128-bit long doubles because runtime support depends > - on glibc. */ > +/* We force the long double type to be IBM 128-bit because the CONVERT_TO_PINF > + tests will fail if we use IEEE 128-bit floating point. This is due to IEEE > + 128-bit having a larger exponent range than IBM 128-bit extended double. So > + tests that would generate an infinity with IBM 128-bit will generate a > + normal number with IEEE 128-bit. */ > + > +/* Test decimal float conversions to and from IBM 128-bit long double. */ > > #include "convert.h" > > @@ -36,9 +41,6 @@ CONVERT_TO_PINF (312, tf, sd, 1.6e+308L, d32) > int > main () > { > - if (sizeof (long double) != 16) > - return 0; > - > convert_101 (); > convert_102 (); > > diff --git a/gcc/testsuite/gcc.target/powerpc/pr70117.c b/gcc/testsuite/gcc.target/powerpc/pr70117.c > index 3bbd2c595e0..8a5fad1dee0 100644 > --- a/gcc/testsuite/gcc.target/powerpc/pr70117.c > +++ b/gcc/testsuite/gcc.target/powerpc/pr70117.c > @@ -1,5 +1,7 @@ > -/* { dg-do run { target { powerpc*-*-linux* powerpc*-*-darwin* powerpc*-*-aix* rs6000-*-* } } } */ > -/* { dg-options "-std=c99 -mlong-double-128 -O2" } */ > +/* { dg-do run } */ > +/* { dg-require-effective-target ppc_long_double_override_ibm128 } */ > +/* { dg-options "-std=c99 -O2" } */ > +/* { dg-add-options ppc_long_double_override_ibm128 } */ > > #include <float.h> > > diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp > index 789723fb287..0a392cb0fd5 100644 > --- a/gcc/testsuite/lib/target-supports.exp > +++ b/gcc/testsuite/lib/target-supports.exp > @@ -2360,6 +2360,113 @@ proc check_effective_target_ppc_ieee128_ok { } { > }] > } > > +# Check if we can explicitly override the long double format to use the IBM > +# 128-bit extended double format, and GLIBC supports doing this override by > +# switching the sprintf to handle IBM 128-bit long double. > + > +proc add_options_for_ppc_long_double_override_ibm128 { flags } { > + if { [istarget powerpc*-*-*] } { > + return "$flags -mlong-double-128 -Wno-psabi -mabi=ibmlongdouble" Just for my edification, can you remind me why we need -Wno-psabi? What warning are we disabling? Same question for ieee variant. LGTM in any event. Recommend approval by maintainers... Thanks! Bill > + } > + return "$flags" > +} > + > +proc check_effective_target_ppc_long_double_override_ibm128 { } { > + return [check_runtime_nocache ppc_long_double_override_ibm128 { > + #include <string.h> > + #include <stdio.h> > + volatile __ibm128 a = (__ibm128) 3.0; > + volatile long double one = 1.0L; > + volatile long double two = 2.0L; > + volatile long double b; > + char buffer[20]; > + int main() > + { > + #if !defined(_ARCH_PPC) || !defined(__LONG_DOUBLE_IBM128__) > + return 1; > + #else > + b = one + two; > + if (memcmp ((void *)&a, (void *)&b, sizeof (long double)) != 0) > + return 1; > + sprintf (buffer, "%lg", b); > + return strcmp (buffer, "3") != 0; > + #endif > + } > + } [add_options_for_ppc_long_double_override_ibm128 ""]] > +} > + > +# Check if we can explicitly override the long double format to use the IEEE > +# 128-bit format, and GLIBC supports doing this override by switching the > +# sprintf to handle IEEE 128-bit long double. > + > +proc add_options_for_ppc_long_double_override_ieee128 { flags } { > + if { [istarget powerpc*-*-*] } { > + return "$flags -mlong-double-128 -Wno-psabi -mabi=ieeelongdouble" > + } > + return "$flags" > +} > + > +proc check_effective_target_ppc_long_double_override_ieee128 { } { > + return [check_runtime_nocache ppc_long_double_override_ieee128 { > + #include <string.h> > + #include <stdio.h> > + volatile _Float128 a = 3.0f128; > + volatile long double one = 1.0L; > + volatile long double two = 2.0L; > + volatile long double b; > + char buffer[20]; > + int main() > + { > + #if !defined(_ARCH_PPC) || !defined(__LONG_DOUBLE_IEEE128__) > + return 1; > + #else > + b = one + two; > + if (memcmp ((void *)&a, (void *)&b, sizeof (long double)) != 0) > + return 1; > + sprintf (buffer, "%lg", b); > + return strcmp (buffer, "3") != 0; > + #endif > + } > + } [add_options_for_ppc_long_double_override_ieee128 ""]] > +} > + > +# Check if we can explicitly override the long double format to use 64-bit > +# floating point, and GLIBC supports doing this override by switching the > +# sprintf to handle 64-bit long double. > + > +proc add_options_for_ppc_long_double_override_64bit { flags } { > + if { [istarget powerpc*-*-*] } { > + return "$flags -mlong-double-64" > + } > + return "$flags" > +} > + > +proc check_effective_target_ppc_long_double_override_64bit { } { > + return [check_runtime_nocache ppc_long_double_override_64bit { > + #include <string.h> > + #include <stdio.h> > + volatile double a = 3.0; > + volatile long double one = 1.0L; > + volatile long double two = 2.0L; > + volatile long double b; > + char buffer[20]; > + int main() > + { > + #if !defined(_ARCH_PPC) || defined(__LONG_DOUBLE_128__) > + return 1; > + #else > + if (sizeof (long double) != sizeof (double)) > + return 1; > + b = one + two; > + if (memcmp ((void *)&a, (void *)&b, sizeof (long double)) != 0) > + return 1; > + sprintf (buffer, "%lg", b); > + return strcmp (buffer, "3") != 0; > + #endif > + } > + } [add_options_for_ppc_long_double_override_64bit ""]] > +} > + > # Return 1 if the target supports executing VSX instructions, 0 > # otherwise. Cache the result. >
On Wed, Jul 14, 2021 at 11:11:29AM -0500, Bill Schmidt wrote: > Just for my edification, can you remind me why we need -Wno-psabi? > What warning are we disabling? Same question for ieee variant. > > LGTM in any event. Recommend approval by maintainers... Unless you configured GCC with a 2.32 or new glibc (such as in Advance Toolchain 14), when you change the default long double representation, the compiler gives a warning that you can't depend on GLIBC to have all of the necessary support for __float128. Using -Wno-psabi supresses this warning. For assembly tests, we don't care if GLIBC supports it or not, but we don't want the warning. Note, there is still a hole with Fortran, in that it doesn't support multiple long double types like C/C++ do, so you can't really change the long double behavior and expect your program to work.
hi! On Wed, Jul 07, 2021 at 03:58:37PM -0400, Michael Meissner wrote: > +/* We force the long double type to be IBM 128-bit because the CONVERT_TO_PINF There is no "forcing" here. "We use ..." or "We require ..." is fine. "Force" suggests something tries to prevent you. "Override" is worse. What is overridden, who's decision is overridden? > +# Check if we can explicitly override the long double format to use the IBM > +# 128-bit extended double format, and GLIBC supports doing this override by > +# switching the sprintf to handle IBM 128-bit long double. > + > +proc add_options_for_ppc_long_double_override_ibm128 { flags } { So this name does not say what it does at all (it does not say anything about glibc). > + if { [istarget powerpc*-*-*] } { > + return "$flags -mlong-double-128 -Wno-psabi -mabi=ibmlongdouble" > + } > + return "$flags" > +} (And neither does this code. The comment is for the *next* function!) > +proc check_effective_target_ppc_long_double_override_ibm128 { } { So this returns false if your libc does not handle printf for your selected long double type. That is problematic, the name does not suggest anything like that. > + return [check_runtime_nocache ppc_long_double_override_ibm128 { > + #include <string.h> > + #include <stdio.h> > + volatile __ibm128 a = (__ibm128) 3.0; > + volatile long double one = 1.0L; > + volatile long double two = 2.0L; > + volatile long double b; > + char buffer[20]; > + int main() > + { > + #if !defined(_ARCH_PPC) || !defined(__LONG_DOUBLE_IBM128__) > + return 1; This is only ever called for Power. Remove the first part please. When can the second part trigger? > + #else > + b = one + two; > + if (memcmp ((void *)&a, (void *)&b, sizeof (long double)) != 0) > + return 1; > + sprintf (buffer, "%lg", b); > + return strcmp (buffer, "3") != 0; > + #endif > + } > + } [add_options_for_ppc_long_double_override_ibm128 ""]] > +} Those casts are useless btw, don't do that. And that sizeof *has* to be 16, and so does that of __ibm128, so just write 16 please. Many more of the same... Use something like the stuff starting at foreach { armfunc armflag armdefs } { (for arm) in target-supports.exp? Segher
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..35da07d1fa4 100644 --- a/gcc/testsuite/c-c++-common/dfp/convert-bfp-11.c +++ b/gcc/testsuite/c-c++-common/dfp/convert-bfp-11.c @@ -1,9 +1,14 @@ -/* { dg-skip-if "" { ! "powerpc*-*-linux*" } } */ +/* { dg-require-effective-target dfp } */ +/* { dg-require-effective-target ppc_long_double_override_ibm128 } */ +/* { dg-add-options ppc_long_double_override_ibm128 } */ -/* Test decimal float conversions to and from IBM 128-bit long double. - Checks are skipped at runtime if long double is not 128 bits. - Don't force 128-bit long doubles because runtime support depends - on glibc. */ +/* We force the long double type to be IBM 128-bit because the CONVERT_TO_PINF + tests will fail if we use IEEE 128-bit floating point. This is due to IEEE + 128-bit having a larger exponent range than IBM 128-bit extended double. So + tests that would generate an infinity with IBM 128-bit will generate a + normal number with IEEE 128-bit. */ + +/* Test decimal float conversions to and from IBM 128-bit long double. */ #include "convert.h" @@ -36,9 +41,6 @@ CONVERT_TO_PINF (312, tf, sd, 1.6e+308L, d32) int main () { - if (sizeof (long double) != 16) - return 0; - convert_101 (); convert_102 (); diff --git a/gcc/testsuite/gcc.target/powerpc/pr70117.c b/gcc/testsuite/gcc.target/powerpc/pr70117.c index 3bbd2c595e0..8a5fad1dee0 100644 --- a/gcc/testsuite/gcc.target/powerpc/pr70117.c +++ b/gcc/testsuite/gcc.target/powerpc/pr70117.c @@ -1,5 +1,7 @@ -/* { dg-do run { target { powerpc*-*-linux* powerpc*-*-darwin* powerpc*-*-aix* rs6000-*-* } } } */ -/* { dg-options "-std=c99 -mlong-double-128 -O2" } */ +/* { dg-do run } */ +/* { dg-require-effective-target ppc_long_double_override_ibm128 } */ +/* { dg-options "-std=c99 -O2" } */ +/* { dg-add-options ppc_long_double_override_ibm128 } */ #include <float.h> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index 789723fb287..0a392cb0fd5 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -2360,6 +2360,113 @@ proc check_effective_target_ppc_ieee128_ok { } { }] } +# Check if we can explicitly override the long double format to use the IBM +# 128-bit extended double format, and GLIBC supports doing this override by +# switching the sprintf to handle IBM 128-bit long double. + +proc add_options_for_ppc_long_double_override_ibm128 { flags } { + if { [istarget powerpc*-*-*] } { + return "$flags -mlong-double-128 -Wno-psabi -mabi=ibmlongdouble" + } + return "$flags" +} + +proc check_effective_target_ppc_long_double_override_ibm128 { } { + return [check_runtime_nocache ppc_long_double_override_ibm128 { + #include <string.h> + #include <stdio.h> + volatile __ibm128 a = (__ibm128) 3.0; + volatile long double one = 1.0L; + volatile long double two = 2.0L; + volatile long double b; + char buffer[20]; + int main() + { + #if !defined(_ARCH_PPC) || !defined(__LONG_DOUBLE_IBM128__) + return 1; + #else + b = one + two; + if (memcmp ((void *)&a, (void *)&b, sizeof (long double)) != 0) + return 1; + sprintf (buffer, "%lg", b); + return strcmp (buffer, "3") != 0; + #endif + } + } [add_options_for_ppc_long_double_override_ibm128 ""]] +} + +# Check if we can explicitly override the long double format to use the IEEE +# 128-bit format, and GLIBC supports doing this override by switching the +# sprintf to handle IEEE 128-bit long double. + +proc add_options_for_ppc_long_double_override_ieee128 { flags } { + if { [istarget powerpc*-*-*] } { + return "$flags -mlong-double-128 -Wno-psabi -mabi=ieeelongdouble" + } + return "$flags" +} + +proc check_effective_target_ppc_long_double_override_ieee128 { } { + return [check_runtime_nocache ppc_long_double_override_ieee128 { + #include <string.h> + #include <stdio.h> + volatile _Float128 a = 3.0f128; + volatile long double one = 1.0L; + volatile long double two = 2.0L; + volatile long double b; + char buffer[20]; + int main() + { + #if !defined(_ARCH_PPC) || !defined(__LONG_DOUBLE_IEEE128__) + return 1; + #else + b = one + two; + if (memcmp ((void *)&a, (void *)&b, sizeof (long double)) != 0) + return 1; + sprintf (buffer, "%lg", b); + return strcmp (buffer, "3") != 0; + #endif + } + } [add_options_for_ppc_long_double_override_ieee128 ""]] +} + +# Check if we can explicitly override the long double format to use 64-bit +# floating point, and GLIBC supports doing this override by switching the +# sprintf to handle 64-bit long double. + +proc add_options_for_ppc_long_double_override_64bit { flags } { + if { [istarget powerpc*-*-*] } { + return "$flags -mlong-double-64" + } + return "$flags" +} + +proc check_effective_target_ppc_long_double_override_64bit { } { + return [check_runtime_nocache ppc_long_double_override_64bit { + #include <string.h> + #include <stdio.h> + volatile double a = 3.0; + volatile long double one = 1.0L; + volatile long double two = 2.0L; + volatile long double b; + char buffer[20]; + int main() + { + #if !defined(_ARCH_PPC) || defined(__LONG_DOUBLE_128__) + return 1; + #else + if (sizeof (long double) != sizeof (double)) + return 1; + b = one + two; + if (memcmp ((void *)&a, (void *)&b, sizeof (long double)) != 0) + return 1; + sprintf (buffer, "%lg", b); + return strcmp (buffer, "3") != 0; + #endif + } + } [add_options_for_ppc_long_double_override_64bit ""]] +} + # Return 1 if the target supports executing VSX instructions, 0 # otherwise. Cache the result.
[PATCH] Fix long double tests when default long double is not IBM. This patch adds 3 more selections to target-supports.exp to see if we can force the compiler to use a particular long double format (IEEE 128-bit, IBM extended double, 64-bit), and the library support will track the changes for the long double. This is needed because two of the tests in the test suite use long double, and they are actually testing IBM extended double. This patch also forces the two tests that explicitly require long double to use the IBM double-double encoding to explicitly run the test. This requires GLIBC 2.32 or greater in order to do the switch. I have run tests on a little endian power9 system with 3 compilers. There were no regressions with these patches, and the two tests in the following patches now work if the default long double is not IBM 128-bit: * One compiler used the default IBM 128-bit format; * One compiler used the IEEE 128-bit format; (and) * One compiler used 64-bit long doubles. I have also tested compilers on a big endian power8 system with a compiler defaulting to power8 code generation and another with the default cpu set. There were no regressions. Can I check this patch into the master branch? 2021-07-07 Michael Meissner <meissner@linux.ibm.com> gcc/testsuite/ PR target/70117 * gcc.target/powerpc/pr70117.c: Force the long double type to use the IBM 128-bit format. * c-c++-common/dfp/convert-bfp-11.c: Force using IBM 128-bit long double. Remove check for 64-bit long double. * lib/target-supports.exp (add_options_for_ppc_long_double_override_ibm128): New function. (check_effective_target_ppc_long_double_override_ibm128): New function. (add_options_for_ppc_long_double_override_ieee128): New function. (check_effective_target_ppc_long_double_override_ieee128): New function. (add_options_for_ppc_long_double_override_64bit): New function. (check_effective_target_ppc_long_double_override_64bit): New function. --- .../c-c++-common/dfp/convert-bfp-11.c | 18 +-- gcc/testsuite/gcc.target/powerpc/pr70117.c | 6 +- gcc/testsuite/lib/target-supports.exp | 107 ++++++++++++++++++ 3 files changed, 121 insertions(+), 10 deletions(-)