diff mbox series

, PowerPC long double transistion, patch #2

Message ID 20180613211637.GA5861@ibm-toto.the-meissners.org
State New
Headers show
Series , PowerPC long double transistion, patch #2 | expand

Commit Message

Michael Meissner June 13, 2018, 9:16 p.m. UTC 
In addition to the previous patch to aid in transitioning the PowerPC long
double format to IEEE 128-bit, I have some additional patches that are needed.
The previous patch is:
https://gcc.gnu.org/ml/gcc-patches/2018-06/msg00634.html

This fixes some of the PowerPC tests that had implicit assumptions about the
long double format.  The fixes involved:

    1)	Using long double __complex instead of using a KC attribute;
    2)	Explicitly adding -mabi=ibmlongdouble; (or)
    3)	Using __ibm128 instead of long double if long double is IEEE 128-bit.

I have done separate bootstraps on a little endian power8 system with the long
double type set to IBM extended and IEEE 128-bit extended.  There were no
regressions in using this patch, and it fixes several of the errors when you
use a compiler with long double defaulting to IEEE 128-bit floating point.  Can
I check it in, and eventually back port it to GCC 8.2 with the other long
double transition patches.

2018-06-13  Michael Meissner  <meissner@linux.ibm.com>

	* gcc.target/powerpc/divkc3-1.c: If long double is IEEE 128-bit,
	use __complex long double instead of KC attribute.
	* gcc.target/powerpc/mulkc3-1.c: Likewise.
	* gcc.target/powerpc/float128-3.c: Add -mabi=ibmlongdouble to
	tests that require long double to be IBM extended double.
	* gcc.target/powerpc/float128-5.c: Likewise.
	* gcc.target/powerpc/float128-mix.c: Likewise.
	* gcc.target/powerpc/pack02.c: On systems where long double is
	IEEE 128-bit, use __ibm128 instead of long double.  Use the ibm128
	pack/unpack functions instead of the long double pack/unpack
	functions.
	* gcc.target/powerpc/pr57150.c: Likewise.
	* gcc.target/powerpc/pr60203.c: Likewise.
	* gcc.target/powerpc/pr67808.c: Likewise.
	* gcc.target/powerpc/pr70117.c: Likewise.
	* gcc.target/powerpc/tfmode_off.c: Likewise.

Comments

Segher Boessenkool June 15, 2018, midnight UTC | #1 
Hi Mike,

On Wed, Jun 13, 2018 at 05:16:37PM -0400, Michael Meissner wrote:
> This fixes some of the PowerPC tests that had implicit assumptions about the
> long double format.  The fixes involved:
> 
>     1)	Using long double __complex instead of using a KC attribute;

Why?  Does KC not work?  It should work.

>     2)	Explicitly adding -mabi=ibmlongdouble; (or)

What in these tests requires IBM long double?  It is not clear.

>     3)	Using __ibm128 instead of long double if long double is IEEE 128-bit.

Can't you do that *always*?  Why not?


Segher
Michael Meissner June 15, 2018, 12:15 a.m. UTC | #2 
On Thu, Jun 14, 2018 at 07:00:52PM -0500, Segher Boessenkool wrote:
> Hi Mike,
> 
> On Wed, Jun 13, 2018 at 05:16:37PM -0400, Michael Meissner wrote:
> > This fixes some of the PowerPC tests that had implicit assumptions about the
> > long double format.  The fixes involved:
> > 
> >     1)	Using long double __complex instead of using a KC attribute;
> 
> Why?  Does KC not work?  It should work.

No KC does not work if long double is IEEE, just like IC does not work if long
double is IBM extended double.  This is part of the __float128/__ibm128 changes
that eliminated using separate types all of the time.

> >     2)	Explicitly adding -mabi=ibmlongdouble; (or)
> 
> What in these tests requires IBM long double?  It is not clear.

These are tests that test things like the format of IBM extended double (things
like pack, unpack, making sure the various __builtin_is<xxx> work for carefully
crafted numbers).

> 
> >     3)	Using __ibm128 instead of long double if long double is IEEE 128-bit.
> 
> Can't you do that *always*?  Why not?

Because __ibm128 is only defined if we have __float128 support.  It is not
defined on non-VSX Linux systems.
Segher Boessenkool June 15, 2018, 9 p.m. UTC | #3 
On Thu, Jun 14, 2018 at 08:15:51PM -0400, Michael Meissner wrote:
> On Thu, Jun 14, 2018 at 07:00:52PM -0500, Segher Boessenkool wrote:
> > Hi Mike,
> > 
> > On Wed, Jun 13, 2018 at 05:16:37PM -0400, Michael Meissner wrote:
> > > This fixes some of the PowerPC tests that had implicit assumptions about the
> > > long double format.  The fixes involved:
> > > 
> > >     1)	Using long double __complex instead of using a KC attribute;
> > 
> > Why?  Does KC not work?  It should work.
> 
> No KC does not work if long double is IEEE, just like IC does not work if long
> double is IBM extended double.  This is part of the __float128/__ibm128 changes
> that eliminated using separate types all of the time.

But those aren't types, those are modes.  Why would the modes not work?
That makes no sense.  Just like the __ieee128 and __ibm128 types work
always, so should the IF, KF, IC and KC modes.

So what happens if you try to use KC?

> > >     2)	Explicitly adding -mabi=ibmlongdouble; (or)
> > 
> > What in these tests requires IBM long double?  It is not clear.
> 
> These are tests that test things like the format of IBM extended double (things
> like pack, unpack, making sure the various __builtin_is<xxx> work for carefully
> crafted numbers).

Aha.

> > >     3)	Using __ibm128 instead of long double if long double is IEEE 128-bit.
> > 
> > Can't you do that *always*?  Why not?
> 
> Because __ibm128 is only defined if we have __float128 support.  It is not
> defined on non-VSX Linux systems.

Please document this in the test, then.


Segher
Michael Meissner June 18, 2018, 7:06 p.m. UTC | #4 
On Fri, Jun 15, 2018 at 04:00:42PM -0500, Segher Boessenkool wrote:
> On Thu, Jun 14, 2018 at 08:15:51PM -0400, Michael Meissner wrote:
> > On Thu, Jun 14, 2018 at 07:00:52PM -0500, Segher Boessenkool wrote:
> > > Hi Mike,
> > > 
> > > On Wed, Jun 13, 2018 at 05:16:37PM -0400, Michael Meissner wrote:
> > > > This fixes some of the PowerPC tests that had implicit assumptions about the
> > > > long double format.  The fixes involved:
> > > > 
> > > >     1)	Using long double __complex instead of using a KC attribute;
> > > 
> > > Why?  Does KC not work?  It should work.
> > 
> > No KC does not work if long double is IEEE, just like IC does not work if long
> > double is IBM extended double.  This is part of the __float128/__ibm128 changes
> > that eliminated using separate types all of the time.
> 
> But those aren't types, those are modes.  Why would the modes not work?
> That makes no sense.  Just like the __ieee128 and __ibm128 types work
> always, so should the IF, KF, IC and KC modes.
> 
> So what happens if you try to use KC?

The problem is the infrastructure of the compiler.  The type and builtin system
really, really does not like having the same external name for a library
function.  So if you have a type that uses KFmode and the function resolved to
"__addkf3", and you have another type that uses TFmode, and it too resolves to
"__addkf3" you get an internal error.  Ditto for the name mangling.

So we have to make sure that there are really only two modes being used.
Segher Boessenkool June 19, 2018, 8:39 p.m. UTC | #5 
On Mon, Jun 18, 2018 at 03:06:18PM -0400, Michael Meissner wrote:
> On Fri, Jun 15, 2018 at 04:00:42PM -0500, Segher Boessenkool wrote:
> > But those aren't types, those are modes.  Why would the modes not work?
> > That makes no sense.  Just like the __ieee128 and __ibm128 types work
> > always, so should the IF, KF, IC and KC modes.
> > 
> > So what happens if you try to use KC?
> 
> The problem is the infrastructure of the compiler.  The type and builtin system
> really, really does not like having the same external name for a library
> function.  So if you have a type that uses KFmode and the function resolved to
> "__addkf3", and you have another type that uses TFmode, and it too resolves to
> "__addkf3" you get an internal error.  Ditto for the name mangling.
> 
> So we have to make sure that there are really only two modes being used.

Or just #define TFmode to KFmode (if that is the type being used), and
something similar for the types.  Similar to how Pmode works.

This might not be a simple change, but we'll need to get there eventually.
The way TFmode works currently is no end of problems.


Segher
diff mbox series

Patch

Index: gcc/testsuite/gcc.target/powerpc/divkc3-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/divkc3-1.c	(revision 261511)
+++ gcc/testsuite/gcc.target/powerpc/divkc3-1.c	(working copy)
@@ -3,7 +3,11 @@ 
 
 void abort ();
 
-typedef __complex float __cfloat128 __attribute__((mode(KC)));
+#ifdef __LONG_DOUBLE_IEEE128__
+typedef __complex long double __cfloat128;
+#else
+typedef __complex float __cfloat128 __attribute__((mode(__KC__)));
+#endif
 
 __cfloat128 divide (__cfloat128 x, __cfloat128 y)
 {
Index: gcc/testsuite/gcc.target/powerpc/float128-3.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/float128-3.c	(revision 261511)
+++ gcc/testsuite/gcc.target/powerpc/float128-3.c	(working copy)
@@ -1,6 +1,6 @@ 
 /* { dg-do compile { target { powerpc*-*-linux* } } } */
 /* { dg-require-effective-target powerpc_vsx_ok } */
-/* { dg-options "-O2 -mvsx -mno-float128" } */
+/* { dg-options "-O2 -mvsx -mno-float128 -mabi=ibmlongdouble -Wno-psabi" } */
 
 /* Test that we can use #pragma GCC target to enable -mfloat128.  */
 
Index: gcc/testsuite/gcc.target/powerpc/float128-5.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/float128-5.c	(revision 261511)
+++ gcc/testsuite/gcc.target/powerpc/float128-5.c	(working copy)
@@ -1,6 +1,6 @@ 
 /* { dg-do compile { target { powerpc*-*-linux* && lp64 } } } */
 /* { dg-require-effective-target powerpc_p9vector_ok } */
-/* { dg-options "-O2 -mpower9-vector -mno-float128" } */
+/* { dg-options "-O2 -mpower9-vector -mno-float128 -mabi=ibmlongdouble -Wno-psabi" } */
 
 /* Test that we can use #pragma GCC target to enable -mfloat128 and generate
    code on ISA 3.0 for the float128 built-in functions.  Lp64 is required
Index: gcc/testsuite/gcc.target/powerpc/float128-mix.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/float128-mix.c	(revision 261511)
+++ gcc/testsuite/gcc.target/powerpc/float128-mix.c	(working copy)
@@ -1,6 +1,6 @@ 
 /* { dg-do compile { target { powerpc*-*-linux* } } } */
 /* { dg-require-effective-target powerpc_vsx_ok } */
-/* { dg-options "-O2 -mvsx" } */
+/* { dg-options "-O2 -mvsx -mabi=ibmlongdouble -Wno-psabi" } */
 
 
 /* Test to make sure that __float128 and long double cannot be combined together.  */
Index: gcc/testsuite/gcc.target/powerpc/mulkc3-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/mulkc3-1.c	(revision 261511)
+++ gcc/testsuite/gcc.target/powerpc/mulkc3-1.c	(working copy)
@@ -3,7 +3,11 @@ 
 
 void abort ();
 
-typedef __complex float __cfloat128 __attribute__((mode(KC)));
+#ifdef __LONG_DOUBLE_IEEE128__
+typedef __complex long double __cfloat128;
+#else
+typedef __complex float __cfloat128 __attribute__((mode(__KC__)));
+#endif
 
 __cfloat128 multiply (__cfloat128 x, __cfloat128 y)
 {
Index: gcc/testsuite/gcc.target/powerpc/pack02.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pack02.c	(revision 261511)
+++ gcc/testsuite/gcc.target/powerpc/pack02.c	(working copy)
@@ -9,6 +9,20 @@ 
 #include <stdlib.h>
 #include <math.h>
 
+#if defined(__LONG_DOUBLE_IEEE128__)
+#define PACK __builtin_pack_ibm128
+#define UNPACK __builtin_unpack_ibm128
+#define LDOUBLE __ibm128
+
+#elif defined(__LONG_DOUBLE_IBM128__)
+#define PACK __builtin_pack_longdouble
+#define UNPACK __builtin_unpack_longdouble
+#define LDOUBLE __ibm128
+
+#else
+#error "long double must be either IBM 128-bit or IEEE 128-bit"
+#endif
+
 #ifdef DEBUG
 #include <stdio.h>
 #endif
@@ -18,31 +32,31 @@  main (void)
 {
   double high = pow (2.0, 60);
   double low  = 2.0;
-  long double a = ((long double)high) + ((long double)low);
-  double x0 = __builtin_unpack_longdouble (a, 0);
-  double x1 = __builtin_unpack_longdouble (a, 1);
-  long double b = __builtin_pack_longdouble (x0, x1);
+  LDOUBLE a = ((LDOUBLE)high) + ((LDOUBLE)low);
+  double x0 = UNPACK (a, 0);
+  double x1 = UNPACK (a, 1);
+  LDOUBLE b = PACK (x0, x1);
 
 #ifdef DEBUG
   {
     size_t i;
     union {
-      long double ld;
+      LDOUBLE ld;
       double d;
-      unsigned char uc[sizeof (long double)];
-      char c[sizeof (long double)];
+      unsigned char uc[sizeof (LDOUBLE)];
+      char c[sizeof (LDOUBLE)];
     } u;
 
     printf ("a  = 0x");
     u.ld = a;
-    for (i = 0; i < sizeof (long double); i++)
+    for (i = 0; i < sizeof (LDOUBLE); i++)
       printf ("%.2x", u.uc[i]);
 
     printf (", %Lg\n", a);
 
     printf ("b  = 0x");
     u.ld = b;
-    for (i = 0; i < sizeof (long double); i++)
+    for (i = 0; i < sizeof (LDOUBLE); i++)
       printf ("%.2x", u.uc[i]);
 
     printf (", %Lg\n", b);
@@ -52,28 +66,28 @@  main (void)
     for (i = 0; i < sizeof (double); i++)
       printf ("%.2x", u.uc[i]);
 
-    printf (",%*s %g\n", (int)(2 * (sizeof (long double) - sizeof (double))), "", high);
+    printf (",%*s %g\n", (int)(2 * (sizeof (LDOUBLE) - sizeof (double))), "", high);
 
     printf ("lo = 0x");
     u.d = low;
     for (i = 0; i < sizeof (double); i++)
       printf ("%.2x", u.uc[i]);
 
-    printf (",%*s %g\n", (int)(2 * (sizeof (long double) - sizeof (double))), "", low);
+    printf (",%*s %g\n", (int)(2 * (sizeof (LDOUBLE) - sizeof (double))), "", low);
 
     printf ("x0 = 0x");
     u.d = x0;
     for (i = 0; i < sizeof (double); i++)
       printf ("%.2x", u.uc[i]);
 
-    printf (",%*s %g\n", (int)(2 * (sizeof (long double) - sizeof (double))), "", x0);
+    printf (",%*s %g\n", (int)(2 * (sizeof (LDOUBLE) - sizeof (double))), "", x0);
 
     printf ("x1 = 0x");
     u.d = x1;
     for (i = 0; i < sizeof (double); i++)
       printf ("%.2x", u.uc[i]);
 
-    printf (",%*s %g\n", (int)(2 * (sizeof (long double) - sizeof (double))), "", x1);
+    printf (",%*s %g\n", (int)(2 * (sizeof (LDOUBLE) - sizeof (double))), "", x1);
   }
 #endif
 
Index: gcc/testsuite/gcc.target/powerpc/pr57150.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr57150.c	(revision 261511)
+++ gcc/testsuite/gcc.target/powerpc/pr57150.c	(working copy)
@@ -10,12 +10,22 @@ 
 /* { dg-final { scan-assembler-not "stxvw4x" } } */
 /* { dg-final { scan-assembler-not "stvx" } } */
 
-/* Insure caller save on long double does not use VSX instructions.  */
+/* Insure caller save on long double/ibm128 does not use VSX instructions.  */
 
-extern long double modify (long double);
+#if defined(__LONG_DOUBLE_IEEE128__)
+#define LDOUBLE __ibm128
+
+#elif defined(__LONG_DOUBLE_IBM128__)
+#define LDOUBLE long double
+
+#else
+#error "long double must be IBM 128-bit or IEEE 128-bit"
+#endif
+
+extern LDOUBLE modify (LDOUBLE);
 
 void
-sum (long double *ptr, long double value, unsigned long n)
+sum (LDOUBLE *ptr, LDOUBLE value, unsigned long n)
 {
   unsigned long i;
 
Index: gcc/testsuite/gcc.target/powerpc/pr60203.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr60203.c	(revision 261511)
+++ gcc/testsuite/gcc.target/powerpc/pr60203.c	(working copy)
@@ -4,9 +4,19 @@ 
 /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power8" } } */
 /* { dg-options "-mcpu=power8 -O3" } */
 
-union u_ld { long double ld; double d[2]; };
+#if defined(__LONG_DOUBLE_IEEE128__)
+#define LDOUBLE __ibm128
 
-long double
+#elif defined(__LONG_DOUBLE_IBM128__)
+#define LDOUBLE long double
+
+#else
+#error "long double must be IBM 128-bit or IEEE 128-bit"
+#endif
+
+union u_ld { LDOUBLE ld; double d[2]; };
+
+LDOUBLE
 pack (double a, double aa)
 {
   union u_ld u;
@@ -16,7 +26,7 @@  pack (double a, double aa)
 }
 
 double
-unpack_0 (long double x)
+unpack_0 (LDOUBLE x)
 {
   union u_ld u;
   u.ld = x;
@@ -24,7 +34,7 @@  unpack_0 (long double x)
 }
 
 double
-unpack_1 (long double x)
+unpack_1 (LDOUBLE x)
 {
   union u_ld u;
   u.ld = x;
Index: gcc/testsuite/gcc.target/powerpc/pr67808.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr67808.c	(revision 261511)
+++ gcc/testsuite/gcc.target/powerpc/pr67808.c	(working copy)
@@ -6,37 +6,47 @@ 
 
 /* PR 67808: LRA ICEs on simple double to long double conversion test case */
 
+#if defined(__LONG_DOUBLE_IEEE128__)
+#define LDOUBLE __ibm128
+
+#elif defined(__LONG_DOUBLE_IBM128__)
+#define LDOUBLE long double
+
+#else
+#error "long double must be IBM 128-bit or IEEE 128-bit"
+#endif
+
 void
-dfoo (long double *ldb1, double *db1)
+dfoo (LDOUBLE *ldb1, double *db1)
 {
   *ldb1 = *db1;
 }
 
-long double
+LDOUBLE
 dfoo2 (double *db1)
 {
   return *db1;
 }
 
-long double
+LDOUBLE
 dfoo3 (double x)
 {
   return x;
 }
 
 void
-ffoo (long double *ldb1, float *db1)
+ffoo (LDOUBLE *ldb1, float *db1)
 {
   *ldb1 = *db1;
 }
 
-long double
+LDOUBLE
 ffoo2 (float *db1)
 {
   return *db1;
 }
 
-long double
+LDOUBLE
 ffoo3 (float x)
 {
   return x;
Index: gcc/testsuite/gcc.target/powerpc/pr70117.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr70117.c	(revision 261511)
+++ gcc/testsuite/gcc.target/powerpc/pr70117.c	(working copy)
@@ -3,10 +3,20 @@ 
 
 #include <float.h>
 
+#if defined(__LONG_DOUBLE_IEEE128__)
+#define LDOUBLE __ibm128
+
+#elif defined(__LONG_DOUBLE_IBM128__)
+#define LDOUBLE long double
+
+#else
+#error "long double must be IBM 128-bit or IEEE 128-bit"
+#endif
+
 union gl_long_double_union
 {
   struct { double hi; double lo; } dd;
-  long double ld;
+  LDOUBLE ld;
 };
 
 /* This is gnulib's LDBL_MAX which, being 107 bits in precision, is
@@ -22,13 +32,13 @@  volatile double dnan = 0.0/0.0;
 int
 main (void)
 {
-  long double ld;
+  LDOUBLE ld;
 
   ld = gl_LDBL_MAX.ld;
   if (__builtin_isinfl (ld))
     __builtin_abort ();
   ld = -gl_LDBL_MAX.ld;
-  if (__builtin_isinfl (ld))
+  if (__builtin_isinf (ld))
     __builtin_abort ();
 
   ld = gl_LDBL_MAX.ld;
Index: gcc/testsuite/gcc.target/powerpc/tfmode_off.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/tfmode_off.c	(revision 261511)
+++ gcc/testsuite/gcc.target/powerpc/tfmode_off.c	(working copy)
@@ -4,7 +4,15 @@ 
 /* { dg-require-effective-target longdouble128 } */
 /* { dg-options "-O2 -fno-align-functions -fno-asynchronous-unwind-tables -mtraceback=no -save-temps" } */
 
-typedef float TFmode __attribute__ ((mode (TF)));
+#if defined(__LONG_DOUBLE_IEEE128__)
+#define TFmode __ibm128
+
+#elif defined(__LONG_DOUBLE_IBM128__)
+#define TFmode long double
+
+#else
+#error "long double must be IBM 128-bit or IEEE 128-bit"
+#endif
 
 void w1 (void *x, TFmode y) { *(TFmode *) (x + 32767) = y; }
 void w2 (void *x, TFmode y) { *(TFmode *) (x + 32766) = y; }