, PR 104253, Fix __ibm128 conversions on IEEE 128-bit system
Commit Message
Use correct names for __ibm128 if long double is IEEE 128-bit.
If you are on a PowerPC system where the default long double is IEEE
128-bit, GCC will use the wrong names for some of the conversion functions
for the __ibm128 type.
What is happening is when the defult long double is IEEE 128-bit, the
various convert, truncation, and extend functions did not specify a
default name for the conversion. The machine indepentent portions of the
compiler would construct a call with an 'if' name (IFmode being the mode
for IBM 128-bit floating point). This patch specifies to use the
tradiational 'tf' name for these conversion functions.
I have tested this on the following systems with no regressions. Can I check
this into the trunk? This will need to be back ported to the GCC 11 branch
also.
* Power10, little endian, with IBM 128-bit long double default
* Power10, little endian, with IEEE 128-bit long double default
* Power9, little endian, with IBM 128-bit long double default
* Power8, big endian, both 64/32-bit targets.
Note, there is another bug that the original test case uncovered that we need a
tweak for built-in handling for the __builtin_pack_ibm128 and
__builtin_unpack_ibm128 built-in functions. This patch fixes the more general
problem that the wrong conversion functions between DImode and IFmode were called.
2022-01-28 Michael Meissner <meissner@the-meissners.org>
gcc/
PR target/104253
* config/rs6000/rs6000.cc (init_float128_ibm): Use the TF names
for builtin conversions between __ibm128 and DImode when long
double uses the IEEE 128-bit format.
gcc/testsuite/
PR target/104253
* gcc.target/powerpc/pr104253.c: New test.
---
gcc/config/rs6000/rs6000.cc | 6 +
gcc/testsuite/gcc.target/powerpc/pr104253.c | 154 ++++++++++++++++++++
2 files changed, 160 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/powerpc/pr104253.c
Comments
On Fri, Jan 28, 2022 at 10:47:06PM -0500, Michael Meissner via Gcc-patches wrote:
> 2022-01-28 Michael Meissner <meissner@the-meissners.org>
>
> gcc/
> PR target/104253
> * config/rs6000/rs6000.cc (init_float128_ibm): Use the TF names
> for builtin conversions between __ibm128 and DImode when long
> double uses the IEEE 128-bit format.
>
> gcc/testsuite/
> PR target/104253
> * gcc.target/powerpc/pr104253.c: New test.
FYI, this looks good to me, but I think Segher or David should give the
final ack.
Jakub
Ping patch to fix PR target/104253. This patch fixes using the wrong names for
conversions between __ibm128 and integer types if the long double default is
IEEE 128-bit.
| Date: Fri, 28 Jan 2022 22:47:06 -0500
| From: Michael Meissner <meissner@linux.ibm.com>
| Subject: [PATCH], PR 104253, Fix __ibm128 conversions on IEEE 128-bit system
| Message-ID: <YfS4uq8fR2oUJUl5@toto.the-meissners.org>
Hi!
On Fri, Jan 28, 2022 at 10:47:06PM -0500, Michael Meissner wrote:
> Use correct names for __ibm128 if long double is IEEE 128-bit.
>
> If you are on a PowerPC system where the default long double is IEEE
> 128-bit, GCC will use the wrong names for some of the conversion functions
> for the __ibm128 type.
It uses the wrong functions, yes.
> What is happening is when the defult long double is IEEE 128-bit, the
(default)
> various convert, truncation, and extend functions did not specify a
> default name for the conversion. The machine indepentent portions of the
(independent)
> compiler would construct a call with an 'if' name (IFmode being the mode
> for IBM 128-bit floating point). This patch specifies to use the
> tradiational 'tf' name for these conversion functions.
(traditional)
> 2022-01-28 Michael Meissner <meissner@the-meissners.org>
>
> gcc/
> PR target/104253
> * config/rs6000/rs6000.cc (init_float128_ibm): Use the TF names
> for builtin conversions between __ibm128 and DImode when long
> double uses the IEEE 128-bit format.
Please rephrase this? It should say "*also* when long double is IEEE
QP". And it is not about the names in the first place.
In the code it says
/* Add various conversions for IFmode to use the traditional TFmode
names. */
which is both clearer and more correct :-)
> gcc/testsuite/
> PR target/104253
> * gcc.target/powerpc/pr104253.c: New test.
> +/* { require-effective-target ppc_float128_sw } */
The documentation for this selector is wrong, btw: it says it tests
whether this is emulated in software! But instead it just tests if it
works, soft float, emulated, and hardware are all fine.
The patch is okay for trunk, and backports later. Thanks!
Segher
@@ -11113,6 +11113,12 @@ init_float128_ibm (machine_mode mode)
set_conv_libfunc (trunc_optab, DDmode, mode, "__dpd_trunctfdd");
set_conv_libfunc (sext_optab, TDmode, mode, "__dpd_extendtftd");
+ set_conv_libfunc (sfix_optab, DImode, mode, "__fixtfdi");
+ set_conv_libfunc (ufix_optab, DImode, mode, "__fixunstfdi");
+
+ set_conv_libfunc (sfloat_optab, mode, DImode, "__floatditf");
+ set_conv_libfunc (ufloat_optab, mode, DImode, "__floatunditf");
+
if (TARGET_POWERPC64)
{
set_conv_libfunc (sfix_optab, TImode, mode, "__fixtfti");
new file mode 100644
@@ -0,0 +1,154 @@
+/*
+ * Require float128 support because __ibm128 currently is not enabled unless we
+ * also have __float128 support.
+ */
+
+/* { dg-do run } */
+/* { require-effective-target ppc_float128_sw } */
+/* { dg-options "-O2 -mvsx -mfloat128" } */
+
+/*
+ * PR target/104253
+ *
+ * Verify that the various conversions to and from __ibm128 work. When the
+ * default for long double is changed to IEEE 128-bit, originally GCC would
+ * call the functions using an 'if' name instead of 'tf.
+ */
+
+#include <stdlib.h>
+
+extern float ibm128_to_sf (__ibm128) __attribute__((noinline));
+extern double ibm128_to_df (__ibm128) __attribute__((noinline));
+extern int ibm128_to_si (__ibm128) __attribute__((noinline));
+extern long long ibm128_to_di (__ibm128) __attribute__((noinline));
+extern unsigned int ibm128_to_usi (__ibm128) __attribute__((noinline));
+extern unsigned long long ibm128_to_udi (__ibm128) __attribute__((noinline));
+
+extern __ibm128 sf_to_ibm128 (float) __attribute__((noinline));
+extern __ibm128 df_to_ibm128 (double) __attribute__((noinline));
+extern __ibm128 si_to_ibm128 (int) __attribute__((noinline));
+extern __ibm128 di_to_ibm128 (long long) __attribute__((noinline));
+extern __ibm128 usi_to_ibm128 (unsigned int) __attribute__((noinline));
+extern __ibm128 udi_to_ibm128 (unsigned long long) __attribute__((noinline));
+
+float
+ibm128_to_sf (__ibm128 x)
+{
+ return x;
+}
+
+double
+ibm128_to_df (__ibm128 x)
+{
+ return x;
+}
+
+int
+ibm128_to_si (__ibm128 x)
+{
+ return x;
+}
+
+long long
+ibm128_to_di (__ibm128 x)
+{
+ return x;
+}
+
+unsigned int
+ibm128_to_usi (__ibm128 x)
+{
+ return x;
+}
+
+unsigned long long
+ibm128_to_udi (__ibm128 x)
+{
+ return x;
+}
+
+__ibm128
+sf_to_ibm128 (float x)
+{
+ return x;
+}
+
+__ibm128
+df_to_ibm128 (double x)
+{
+ return x;
+}
+
+__ibm128
+si_to_ibm128 (int x)
+{
+ return x;
+}
+
+__ibm128
+di_to_ibm128 (long long x)
+{
+ return x;
+}
+
+__ibm128
+usi_to_ibm128 (unsigned int x)
+{
+ return x;
+}
+
+__ibm128
+udi_to_ibm128 (unsigned long long x)
+{
+ return x;
+}
+
+volatile float seven_sf = 7.0f;
+volatile double seven_df = 7.0;
+volatile int seven_si = 7;
+volatile long long seven_di = 7LL;
+volatile unsigned int seven_usi = 7U;
+volatile unsigned long long seven_udi = 7ULL;
+volatile __ibm128 seven_ibm128 = 7.0;
+
+int
+main (void)
+{
+ if (seven_ibm128 != sf_to_ibm128 (seven_sf))
+ abort ();
+
+ if (seven_ibm128 != df_to_ibm128 (seven_df))
+ abort ();
+
+ if (seven_ibm128 != si_to_ibm128 (seven_si))
+ abort ();
+
+ if (seven_ibm128 != di_to_ibm128 (seven_di))
+ abort ();
+
+ if (seven_ibm128 != usi_to_ibm128 (seven_usi))
+ abort ();
+
+ if (seven_ibm128 != udi_to_ibm128 (seven_udi))
+ abort ();
+
+ if (seven_sf != ibm128_to_sf (seven_ibm128))
+ abort ();
+
+ if (seven_df != ibm128_to_df (seven_ibm128))
+ abort ();
+
+ if (seven_si != ibm128_to_si (seven_ibm128))
+ abort ();
+
+ if (seven_di != ibm128_to_di (seven_ibm128))
+ abort ();
+
+ if (seven_usi != ibm128_to_usi (seven_ibm128))
+ abort ();
+
+ if (seven_udi != ibm128_to_udi (seven_ibm128))
+ abort ();
+
+ return 0;
+}