[PATCH] PowerPC Fix ibm128 defaults for pr70117.c test.

[PATCH] PowerPC Fix ibm128 defaults for pr70117.c test.

[Michael Meissner]

From 698d9fd8a5701fa4ed9690ddf71d57765921778c Mon Sep 17 00:00:00 2001
From: Michael Meissner <meissner@linux.ibm.com>
Date: Sun, 15 Nov 2020 00:48:23 -0500
Subject: [PATCH] PowerPC Fix ibm128 defaults for pr70117.c test.

This patch was previously posted as a combined patch with 2 other testsuite
patches.  I moved it to a separate patch.

This patch fixes up a failure that I saw when I built a compiler with the long
double default set to IEEE 128-bit instead of IBM 128-bit.  Now compilers with
either 128-bit long double default pass this test.  Can I check this into the
master branch?

gcc/testsuite/
2020-11-15  Michael Meissner  <meissner@linux.ibm.com>

	PR target/70117
	* gcc.target/powerpc/pr70117.c: Add support for long double being
	IEEE 128-bit.
---
 gcc/testsuite/gcc.target/powerpc/pr70117.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

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: meissner@linux.ibm.com, phone: +1 (978) 899-4797

Re: [PATCH] PowerPC Fix ibm128 defaults for pr70117.c test.

[will schmidt]

On Sun, 2020-11-15 at 12:17 -0500, Michael Meissner via Gcc-patches wrote:
> From 698d9fd8a5701fa4ed9690ddf71d57765921778c Mon Sep 17 00:00:00 2001
> From: Michael Meissner <meissner@linux.ibm.com>
> Date: Sun, 15 Nov 2020 00:48:23 -0500
> Subject: [PATCH] PowerPC Fix ibm128 defaults for pr70117.c test.
> 
> This patch was previously posted as a combined patch with 2 other testsuite
> patches.  I moved it to a separate patch.

I don't see that thread.  Either really old or differently named ?

> 
> This patch fixes up a failure that I saw when I built a compiler with the long
> double default set to IEEE 128-bit instead of IBM 128-bit.  Now compilers with
> either 128-bit long double default pass this test.  Can I check this into the
> master branch?

sandbox or upstream failure?
Perhaps stands alone better as 
" This patch updates the pr70177.c testcase to define IBM128_MAX as
appropriate for the IBM 128 or IEEE 128 type that is currently in use."
?


> 
> gcc/testsuite/
> 2020-11-15  Michael Meissner  <meissner@linux.ibm.com>
> 
> 	PR target/70117
> 	* gcc.target/powerpc/pr70117.c: Add support for long double being
> 	IEEE 128-bit.
> ---
>  gcc/testsuite/gcc.target/powerpc/pr70117.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> 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
> 
> 

Re: [PATCH] PowerPC Fix ibm128 defaults for pr70117.c test.

[Segher Boessenkool]

Hi!

On Sun, Nov 15, 2020 at 12:17:47PM -0500, Michael Meissner wrote:
> --- 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)

This is the IEEE QP float number 43fefffffffffffff7ffffffffffff80 which
I very much doubt is the maximum finite double-double?  See the 0 in the
middle of the mantissa...  43feffffffffffffffffffffffffff00 is bigger,
and representable as double-double just as well?  Or even
43feffffffffffffffffffffffffff80 should be.


Segher

Re: [PATCH] PowerPC Fix ibm128 defaults for pr70117.c test.

[Jakub Jelinek]

On Wed, Nov 18, 2020 at 03:43:20PM -0600, Segher Boessenkool wrote:
> Hi!
> 
> On Sun, Nov 15, 2020 at 12:17:47PM -0500, Michael Meissner wrote:
> > --- 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)
> 
> This is the IEEE QP float number 43fefffffffffffff7ffffffffffff80 which
> I very much doubt is the maximum finite double-double?  See the 0 in the

Numbers without the 0 in the middle-end aren't valid, see
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95450#c6
for more details.  Without the 0 in the middle the double double number
rounded to double would require increasing the higher double, and as it is
the largest representable finite number, that is not possible.

	Jakub

Re: [PATCH] PowerPC Fix ibm128 defaults for pr70117.c test.

[Segher Boessenkool]

On Wed, Nov 18, 2020 at 10:53:49PM +0100, Jakub Jelinek wrote:
> On Wed, Nov 18, 2020 at 03:43:20PM -0600, Segher Boessenkool wrote:
> > Hi!
> > 
> > On Sun, Nov 15, 2020 at 12:17:47PM -0500, Michael Meissner wrote:
> > > --- 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)
> > 
> > This is the IEEE QP float number 43fefffffffffffff7ffffffffffff80 which
> > I very much doubt is the maximum finite double-double?  See the 0 in the
> 
> Numbers without the 0 in the middle-end aren't valid, see
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95450#c6
> for more details.  Without the 0 in the middle the double double number
> rounded to double would require increasing the higher double, and as it is
> the largest representable finite number, that is not possible.

Ah, in that way.  Tricky.

Mike, please add a comment, what number it represents?  Okay for trunk
with that, thanks.

(Should those not be define in some header though?)


Segher

Re: [PATCH] PowerPC Fix ibm128 defaults for pr70117.c test.

[Paul A. Clarke]

On Wed, Nov 18, 2020 at 04:29:09PM -0600, Segher Boessenkool wrote:
> On Wed, Nov 18, 2020 at 10:53:49PM +0100, Jakub Jelinek wrote:
> > On Wed, Nov 18, 2020 at 03:43:20PM -0600, Segher Boessenkool wrote:
> > > On Sun, Nov 15, 2020 at 12:17:47PM -0500, Michael Meissner wrote:
> > > > --- 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)
> > > 
> > > This is the IEEE QP float number 43fefffffffffffff7ffffffffffff80 which
> > > I very much doubt is the maximum finite double-double?  See the 0 in the
> > 
> > Numbers without the 0 in the middle-end aren't valid, see
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95450#c6
> > for more details.  Without the 0 in the middle the double double number
> > rounded to double would require increasing the higher double, and as it is
> > the largest representable finite number, that is not possible.
> 
> Ah, in that way.  Tricky.
> 
> Mike, please add a comment, what number it represents?  Okay for trunk
> with that, thanks.
> 
> (Should those not be define in some header though?)

Would it be better to represent the number in hex, like with printf's '%a'
formatting (e.g. "0x1.921fb54442d18p+0"...this is NOT the same value)?

(I always get nervous when I see a long float hardcoded in decimal.)

PC

Re: [PATCH] PowerPC Fix ibm128 defaults for pr70117.c test.

[Segher Boessenkool]

On Wed, Nov 18, 2020 at 07:13:04PM -0600, Paul A. Clarke wrote:
> On Wed, Nov 18, 2020 at 04:29:09PM -0600, Segher Boessenkool wrote:
> > Mike, please add a comment, what number it represents?  Okay for trunk
> > with that, thanks.
> > 
> > (Should those not be define in some header though?)
> 
> Would it be better to represent the number in hex, like with printf's '%a'
> formatting (e.g. "0x1.921fb54442d18p+0"...this is NOT the same value)?
> 
> (I always get nervous when I see a long float hardcoded in decimal.)

Yeah, good point, thanks.

That's indeed why I converted it to hex, to see what it *is* :-)

Hexadecimal float is C99 so we cannot use it in the installed headers,
but it should be fine in most testcases now.  And even in the headers
we can put it in a comment of course.


Segher

Re: [PATCH] PowerPC Fix ibm128 defaults for pr70117.c test.

[Michael Meissner]

On Wed, Nov 18, 2020 at 04:29:09PM -0600, Segher Boessenkool wrote:
> On Wed, Nov 18, 2020 at 10:53:49PM +0100, Jakub Jelinek wrote:
> > On Wed, Nov 18, 2020 at 03:43:20PM -0600, Segher Boessenkool wrote:
> > > Hi!
> > > 
> > > On Sun, Nov 15, 2020 at 12:17:47PM -0500, Michael Meissner wrote:
> > > > --- 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)
> > > 
> > > This is the IEEE QP float number 43fefffffffffffff7ffffffffffff80 which
> > > I very much doubt is the maximum finite double-double?  See the 0 in the
> > 
> > Numbers without the 0 in the middle-end aren't valid, see
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95450#c6
> > for more details.  Without the 0 in the middle the double double number
> > rounded to double would require increasing the higher double, and as it is
> > the largest representable finite number, that is not possible.
> 
> Ah, in that way.  Tricky.
> 
> Mike, please add a comment, what number it represents?  Okay for trunk
> with that, thanks.
> 
> (Should those not be define in some header though?)

When long double is IBM extended double, then LDBL_MAX, etc. is set with math.h
(and the __ version created by the compiler).  We don't have min/max for the
funky MD only floating point numbers defined.  I got the number by printing
LDBL_MAX in fact and just pasting that in.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.ibm.com, phone: +1 (978) 899-4797

Re: [PATCH] PowerPC Fix ibm128 defaults for pr70117.c test.

[Segher Boessenkool]

On Thu, Nov 19, 2020 at 03:08:05AM -0500, Michael Meissner wrote:
> On Wed, Nov 18, 2020 at 04:29:09PM -0600, Segher Boessenkool wrote:
> > Mike, please add a comment, what number it represents?  Okay for trunk
> > with that, thanks.
> > 
> > (Should those not be define in some header though?)
> 
> When long double is IBM extended double, then LDBL_MAX, etc. is set with math.h
> (and the __ version created by the compiler).  We don't have min/max for the
> funky MD only floating point numbers defined.  I got the number by printing
> LDBL_MAX in fact and just pasting that in.

Sure -- I am suggesting to always define __IBM128_MAX__ and the like,
which then can be used to define LDBL_MAX, but also can be used
directly.


Segher

Re: [PATCH] PowerPC Fix ibm128 defaults for pr70117.c test.

[Michael Meissner]

On Thu, Nov 19, 2020 at 08:03:02AM -0600, Segher Boessenkool wrote:
> On Thu, Nov 19, 2020 at 03:08:05AM -0500, Michael Meissner wrote:
> > On Wed, Nov 18, 2020 at 04:29:09PM -0600, Segher Boessenkool wrote:
> > > Mike, please add a comment, what number it represents?  Okay for trunk
> > > with that, thanks.
> > > 
> > > (Should those not be define in some header though?)
> > 
> > When long double is IBM extended double, then LDBL_MAX, etc. is set with math.h
> > (and the __ version created by the compiler).  We don't have min/max for the
> > funky MD only floating point numbers defined.  I got the number by printing
> > LDBL_MAX in fact and just pasting that in.
> 
> Sure -- I am suggesting to always define __IBM128_MAX__ and the like,
> which then can be used to define LDBL_MAX, but also can be used
> directly.

I have posted patches for this as a new set of patches.  Rather than trying to
create IBM 128-bit long double min/max/etc. defines, I just marked the test as
needing IBM 128-bit long double.

I did look into providing defines for these.  Unfortunately the function that
creates these (builtin_define_float_constants) is static.  And the caller of
that function (c_cpp_builtins) does not have a target hook or other method to
provide for these defines for MD specific floating point types.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.ibm.com, phone: +1 (978) 899-4797

Re: [PATCH] PowerPC Fix ibm128 defaults for pr70117.c test.

[Segher Boessenkool]

On Sat, Nov 21, 2020 at 12:27:30AM -0500, Michael Meissner wrote:
> On Thu, Nov 19, 2020 at 08:03:02AM -0600, Segher Boessenkool wrote:
> > Sure -- I am suggesting to always define __IBM128_MAX__ and the like,
> > which then can be used to define LDBL_MAX, but also can be used
> > directly.
> 
> I have posted patches for this as a new set of patches.  Rather than trying to
> create IBM 128-bit long double min/max/etc. defines, I just marked the test as
> needing IBM 128-bit long double.
> 
> I did look into providing defines for these.  Unfortunately the function that
> creates these (builtin_define_float_constants) is static.  And the caller of
> that function (c_cpp_builtins) does not have a target hook or other method to
> provide for these defines for MD specific floating point types.

So create the defines from somewhere in the backend, instead?  This
isn't rocket science.

You can even leave the existing LDBL defines alone if you just override
them later.

And there is nothing against adding new hooks or whatever!


Segher

Re: [PATCH] PowerPC Fix ibm128 defaults for pr70117.c test.

[Jakub Jelinek]

On Mon, Nov 23, 2020 at 12:59:29PM -0600, Segher Boessenkool wrote:
> On Sat, Nov 21, 2020 at 12:27:30AM -0500, Michael Meissner wrote:
> > On Thu, Nov 19, 2020 at 08:03:02AM -0600, Segher Boessenkool wrote:
> > > Sure -- I am suggesting to always define __IBM128_MAX__ and the like,
> > > which then can be used to define LDBL_MAX, but also can be used
> > > directly.
> > 
> > I have posted patches for this as a new set of patches.  Rather than trying to
> > create IBM 128-bit long double min/max/etc. defines, I just marked the test as
> > needing IBM 128-bit long double.
> > 
> > I did look into providing defines for these.  Unfortunately the function that
> > creates these (builtin_define_float_constants) is static.  And the caller of
> > that function (c_cpp_builtins) does not have a target hook or other method to
> > provide for these defines for MD specific floating point types.
> 
> So create the defines from somewhere in the backend, instead?  This
> isn't rocket science.
> 
> You can even leave the existing LDBL defines alone if you just override
> them later.

The current defines are quite complex code, because defining them is very
expensive and we don't want to pay that price at every compilation start
when most of the compilations never use those macros.
So they are magic deferred macros.

	Jakub

Re: [PATCH] PowerPC Fix ibm128 defaults for pr70117.c test.

[Segher Boessenkool]

On Mon, Nov 23, 2020 at 08:10:49PM +0100, Jakub Jelinek wrote:
> On Mon, Nov 23, 2020 at 12:59:29PM -0600, Segher Boessenkool wrote:
> > On Sat, Nov 21, 2020 at 12:27:30AM -0500, Michael Meissner wrote:
> > > On Thu, Nov 19, 2020 at 08:03:02AM -0600, Segher Boessenkool wrote:
> > > > Sure -- I am suggesting to always define __IBM128_MAX__ and the like,
> > > > which then can be used to define LDBL_MAX, but also can be used
> > > > directly.
> > > 
> > > I have posted patches for this as a new set of patches.  Rather than trying to
> > > create IBM 128-bit long double min/max/etc. defines, I just marked the test as
> > > needing IBM 128-bit long double.
> > > 
> > > I did look into providing defines for these.  Unfortunately the function that
> > > creates these (builtin_define_float_constants) is static.  And the caller of
> > > that function (c_cpp_builtins) does not have a target hook or other method to
> > > provide for these defines for MD specific floating point types.
> > 
> > So create the defines from somewhere in the backend, instead?  This
> > isn't rocket science.
> > 
> > You can even leave the existing LDBL defines alone if you just override
> > them later.
> 
> The current defines are quite complex code, because defining them is very
> expensive and we don't want to pay that price at every compilation start
> when most of the compilations never use those macros.
> So they are magic deferred macros.

Ah, so defining later will not work, or it will be quite fragile at
least.  So that won't fly :-/

We can provide IBM128 etc. macros for this just fine of course, but
using that for the LDBL is hard then, so perhaps we should not do that
second step, certainly not in stage > 1.

Thanks,


Segher