[PATCH 1/2] s_sincosf.h: Change pio4 type to float [BZ #28713]

[PATCH 1/2] s_sincosf.h: Change pio4 type to float [BZ #28713]

[H.J. Lu]

s_cosf.c and s_sinf.c have

  if (abstop12 (y) < abstop12 (pio4))

where abstop12 takes a float argument, but pio4 is static const double.
pio4 is used only in calls to abstop12 and never in arithmetic.  Change
it to float to fix:

FAIL: math/test-float-cos
FAIL: math/test-float-sin
FAIL: math/test-float-sincos
FAIL: math/test-float32-cos
FAIL: math/test-float32-sin
FAIL: math/test-float32-sincos

when compiling with GCC 12.
---
 sysdeps/ieee754/flt-32/s_sincosf.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sysdeps/ieee754/flt-32/s_sincosf.h b/sysdeps/ieee754/flt-32/s_sincosf.h
index 125ab7f846..0dadd6374e 100644
--- a/sysdeps/ieee754/flt-32/s_sincosf.h
+++ b/sysdeps/ieee754/flt-32/s_sincosf.h
@@ -24,7 +24,7 @@
 /* 2PI * 2^-64.  */
 static const double pi63 = 0x1.921FB54442D18p-62;
 /* PI / 4.  */
-static const double pio4 = 0x1.921FB54442D18p-1;
+static const float pio4 = 0x1.921FB54442D18p-1;
 
 /* Polynomial data (the cosine polynomial is negated in the 2nd entry).  */
 extern const sincos_t __sincosf_table[2] attribute_hidden;
-- 
2.33.1

[PATCH 2/2] math: Add X_TLOSSf [BZ #28713]

[H.J. Lu]

Change X_TLOSS to the hexadecimal notation and add X_TLOSSf to define
the float verion of X_TLOSS to fix

FAIL: math/test-float-j0
FAIL: math/test-float-jn
FAIL: math/test-float-y0
FAIL: math/test-float-y1
FAIL: math/test-float-yn
FAIL: math/test-float32-j0
FAIL: math/test-float32-jn
FAIL: math/test-float32-y0
FAIL: math/test-float32-y1
FAIL: math/test-float32-yn

when compiling with GCC 12.
---
 math/math-svid-compat.h | 3 ++-
 math/w_j0f_compat.c     | 4 ++--
 math/w_j1f_compat.c     | 2 +-
 math/w_jnf_compat.c     | 4 ++--
 4 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/math/math-svid-compat.h b/math/math-svid-compat.h
index 5c18cb1b03..61d22ce461 100644
--- a/math/math-svid-compat.h
+++ b/math/math-svid-compat.h
@@ -48,7 +48,8 @@ struct exception
 extern int matherr (struct exception *__exc);
 extern int __matherr (struct exception *__exc);
 
-#define X_TLOSS	1.41484755040568800000e+16
+#define X_TLOSS		0x1.921fb54442d180000000p+53
+#define X_TLOSSf	0x1.921fb6p+53
 
 /* Types of exceptions in the `type' field.  */
 #define DOMAIN		1
diff --git a/math/w_j0f_compat.c b/math/w_j0f_compat.c
index d375f3bfb0..7e8be846cb 100644
--- a/math/w_j0f_compat.c
+++ b/math/w_j0f_compat.c
@@ -27,7 +27,7 @@
 float
 __j0f (float x)
 {
-  if (__builtin_expect (isgreater (fabsf (x), (float) X_TLOSS), 0)
+  if (__builtin_expect (isgreater (fabsf (x), X_TLOSSf), 0)
       && _LIB_VERSION != _IEEE_ && _LIB_VERSION != _POSIX_)
     /* j0(|x|>X_TLOSS) */
     return __kernel_standard_f (x, x, 134);
@@ -42,7 +42,7 @@ float
 __y0f (float x)
 {
   if (__builtin_expect (islessequal (x, 0.0f)
-                        || isgreater (x, (float) X_TLOSS), 0)
+                        || isgreater (x, X_TLOSSf), 0)
       && _LIB_VERSION != _IEEE_)
     {
       if (x < 0.0f)
diff --git a/math/w_j1f_compat.c b/math/w_j1f_compat.c
index 81e56b771e..471dd93fdd 100644
--- a/math/w_j1f_compat.c
+++ b/math/w_j1f_compat.c
@@ -42,7 +42,7 @@ float
 __y1f (float x)
 {
   if (__builtin_expect (islessequal (x, 0.0f)
-			|| isgreater (x, (float) X_TLOSS), 0)
+			|| isgreater (x, X_TLOSSf), 0)
       && _LIB_VERSION != _IEEE_)
     {
       if (x < 0.0f)
diff --git a/math/w_jnf_compat.c b/math/w_jnf_compat.c
index 296e631566..80af01385a 100644
--- a/math/w_jnf_compat.c
+++ b/math/w_jnf_compat.c
@@ -27,7 +27,7 @@
 float
 __jnf (int n, float x)
 {
-  if (__builtin_expect (isgreater (fabsf (x), (float) X_TLOSS), 0)
+  if (__builtin_expect (isgreater (fabsf (x), X_TLOSSf), 0)
       && _LIB_VERSION != _IEEE_ && _LIB_VERSION != _POSIX_)
     /* jn(n,|x|>X_TLOSS) */
     return __kernel_standard_f (n, x, 138);
@@ -42,7 +42,7 @@ float
 __ynf (int n, float x)
 {
   if (__builtin_expect (islessequal (x, 0.0f)
-			|| isgreater (x, (float) X_TLOSS), 0)
+			|| isgreater (x, X_TLOSSf), 0)
       && _LIB_VERSION != _IEEE_)
     {
       if (x < 0.0f)
-- 
2.33.1

Re: [PATCH 1/2] s_sincosf.h: Change pio4 type to float [BZ #28713]

[Paul Zimmermann]

       Dear H.J.,

I confirm the 6 failures with gcc12, and that they are fixed by that patch.

I suggest one small change: replace pio4 = 0x1.921FB54442D18p-1 by
pio4 = 0x1.921fb6p-1f, where 0x1.921fb6p-1 is pi/4 rounded to nearest
in binary32, and the 'f' suffix denotes a float constant.

LGTM with that small change.

Paul

> Date: Mon, 20 Dec 2021 15:18:16 -0800
> From: "H.J. Lu via Libc-alpha" <libc-alpha@sourceware.org>
> Cc: Joseph Myers <joseph@codesourcery.com>
> 
> s_cosf.c and s_sinf.c have
> 
>   if (abstop12 (y) < abstop12 (pio4))
> 
> where abstop12 takes a float argument, but pio4 is static const double.
> pio4 is used only in calls to abstop12 and never in arithmetic.  Change
> it to float to fix:
> 
> FAIL: math/test-float-cos
> FAIL: math/test-float-sin
> FAIL: math/test-float-sincos
> FAIL: math/test-float32-cos
> FAIL: math/test-float32-sin
> FAIL: math/test-float32-sincos
> 
> when compiling with GCC 12.
> ---
>  sysdeps/ieee754/flt-32/s_sincosf.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sysdeps/ieee754/flt-32/s_sincosf.h b/sysdeps/ieee754/flt-32/s_sincosf.h
> index 125ab7f846..0dadd6374e 100644
> --- a/sysdeps/ieee754/flt-32/s_sincosf.h
> +++ b/sysdeps/ieee754/flt-32/s_sincosf.h
> @@ -24,7 +24,7 @@
>  /* 2PI * 2^-64.  */
>  static const double pi63 = 0x1.921FB54442D18p-62;
>  /* PI / 4.  */
> -static const double pio4 = 0x1.921FB54442D18p-1;
> +static const float pio4 = 0x1.921FB54442D18p-1;
>  
>  /* Polynomial data (the cosine polynomial is negated in the 2nd entry).  */
>  extern const sincos_t __sincosf_table[2] attribute_hidden;
> -- 
> 2.33.1
> 
> 

Re: [PATCH 2/2] math: Add X_TLOSSf [BZ #28713]

[Paul Zimmermann]

       Dear HJ,

> Date: Mon, 20 Dec 2021 15:18:17 -0800
> From: "H.J. Lu via Libc-alpha" <libc-alpha@sourceware.org>
> Cc: Joseph Myers <joseph@codesourcery.com>
> 
> Change X_TLOSS to the hexadecimal notation and add X_TLOSSf to define
> the float verion of X_TLOSS to fix
> 
> FAIL: math/test-float-j0
> FAIL: math/test-float-jn
> FAIL: math/test-float-y0
> FAIL: math/test-float-y1
> FAIL: math/test-float-yn
> FAIL: math/test-float32-j0
> FAIL: math/test-float32-jn
> FAIL: math/test-float32-y0
> FAIL: math/test-float32-y1
> FAIL: math/test-float32-yn
> 
> when compiling with GCC 12.

I confirm these tests fail with gcc 12, and pass with the patch below
(tested on x86_64 linux).

>  math/math-svid-compat.h | 3 ++-
>  math/w_j0f_compat.c     | 4 ++--
>  math/w_j1f_compat.c     | 2 +-
>  math/w_jnf_compat.c     | 4 ++--
>  4 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/math/math-svid-compat.h b/math/math-svid-compat.h
> index 5c18cb1b03..61d22ce461 100644
> --- a/math/math-svid-compat.h
> +++ b/math/math-svid-compat.h
> @@ -48,7 +48,8 @@ struct exception
>  extern int matherr (struct exception *__exc);
>  extern int __matherr (struct exception *__exc);
>  
> -#define X_TLOSS	1.41484755040568800000e+16
> +#define X_TLOSS		0x1.921fb54442d180000000p+53

why did you change the alignement?

> +#define X_TLOSSf	0x1.921fb6p+53

I would add a suffix 'f'

>  /* Types of exceptions in the `type' field.  */
>  #define DOMAIN		1
> diff --git a/math/w_j0f_compat.c b/math/w_j0f_compat.c
> index d375f3bfb0..7e8be846cb 100644
> --- a/math/w_j0f_compat.c
> +++ b/math/w_j0f_compat.c
> @@ -27,7 +27,7 @@
>  float
>  __j0f (float x)
>  {
> -  if (__builtin_expect (isgreater (fabsf (x), (float) X_TLOSS), 0)
> +  if (__builtin_expect (isgreater (fabsf (x), X_TLOSSf), 0)
>        && _LIB_VERSION != _IEEE_ && _LIB_VERSION != _POSIX_)
>      /* j0(|x|>X_TLOSS) */
>      return __kernel_standard_f (x, x, 134);
> @@ -42,7 +42,7 @@ float
>  __y0f (float x)
>  {
>    if (__builtin_expect (islessequal (x, 0.0f)
> -                        || isgreater (x, (float) X_TLOSS), 0)
> +                        || isgreater (x, X_TLOSSf), 0)
>        && _LIB_VERSION != _IEEE_)
>      {
>        if (x < 0.0f)
> diff --git a/math/w_j1f_compat.c b/math/w_j1f_compat.c
> index 81e56b771e..471dd93fdd 100644
> --- a/math/w_j1f_compat.c
> +++ b/math/w_j1f_compat.c
> @@ -42,7 +42,7 @@ float
>  __y1f (float x)
>  {
>    if (__builtin_expect (islessequal (x, 0.0f)
> -			|| isgreater (x, (float) X_TLOSS), 0)
> +			|| isgreater (x, X_TLOSSf), 0)
>        && _LIB_VERSION != _IEEE_)
>      {
>        if (x < 0.0f)
> diff --git a/math/w_jnf_compat.c b/math/w_jnf_compat.c
> index 296e631566..80af01385a 100644
> --- a/math/w_jnf_compat.c
> +++ b/math/w_jnf_compat.c
> @@ -27,7 +27,7 @@
>  float
>  __jnf (int n, float x)
>  {
> -  if (__builtin_expect (isgreater (fabsf (x), (float) X_TLOSS), 0)
> +  if (__builtin_expect (isgreater (fabsf (x), X_TLOSSf), 0)
>        && _LIB_VERSION != _IEEE_ && _LIB_VERSION != _POSIX_)
>      /* jn(n,|x|>X_TLOSS) */
>      return __kernel_standard_f (n, x, 138);
> @@ -42,7 +42,7 @@ float
>  __ynf (int n, float x)
>  {
>    if (__builtin_expect (islessequal (x, 0.0f)
> -			|| isgreater (x, (float) X_TLOSS), 0)
> +			|| isgreater (x, X_TLOSSf), 0)
>        && _LIB_VERSION != _IEEE_)
>      {
>        if (x < 0.0f)
> -- 
> 2.33.1

looks good to me, thanks!

Paul

PS: after this patch, and the previous one, there is only one remaining
failure with gcc12:
FAIL: elf/tst-env-setuid

Re: [PATCH 2/2] math: Add X_TLOSSf [BZ #28713]

[Florian Weimer]

* H. J. Lu via Libc-alpha:

> diff --git a/math/math-svid-compat.h b/math/math-svid-compat.h
> index 5c18cb1b03..61d22ce461 100644
> --- a/math/math-svid-compat.h
> +++ b/math/math-svid-compat.h
> @@ -48,7 +48,8 @@ struct exception
>  extern int matherr (struct exception *__exc);
>  extern int __matherr (struct exception *__exc);
>  
> -#define X_TLOSS	1.41484755040568800000e+16
> +#define X_TLOSS		0x1.921fb54442d180000000p+53
> +#define X_TLOSSf	0x1.921fb6p+53

Is it possible to use a preprocessor macro with token pasting to create
a correctly rounded flaot constant from X_TLOSS?

#define AS_FLOAT_CONSTANT_1(x) x##f
#define AS_FLOAT_CONSTANT(x) AS_FLOAT_CONSTANT_1(x)

I expect that GCC will not raise any exceptions when such a constant is
evaluated at run time.

Thanks,
Florian

Re: [PATCH 2/2] math: Add X_TLOSSf [BZ #28713]

[Paul Zimmermann]

       Florian,

> Date: Tue, 21 Dec 2021 11:13:52 +0100
> From: Florian Weimer via Libc-alpha <libc-alpha@sourceware.org>
> Cc: Joseph Myers <joseph@codesourcery.com>
> 
> * H. J. Lu via Libc-alpha:
> 
> > diff --git a/math/math-svid-compat.h b/math/math-svid-compat.h
> > index 5c18cb1b03..61d22ce461 100644
> > --- a/math/math-svid-compat.h
> > +++ b/math/math-svid-compat.h
> > @@ -48,7 +48,8 @@ struct exception
> >  extern int matherr (struct exception *__exc);
> >  extern int __matherr (struct exception *__exc);
> >  
> > -#define X_TLOSS	1.41484755040568800000e+16
> > +#define X_TLOSS		0x1.921fb54442d180000000p+53
> > +#define X_TLOSSf	0x1.921fb6p+53
> 
> Is it possible to use a preprocessor macro with token pasting to create
> a correctly rounded flaot constant from X_TLOSS?
> 
> #define AS_FLOAT_CONSTANT_1(x) x##f
> #define AS_FLOAT_CONSTANT(x) AS_FLOAT_CONSTANT_1(x)

nice trick!

> I expect that GCC will not raise any exceptions when such a constant is
> evaluated at run time.

is there a chance that the constant is evaluated at run time? In such a case,
could it be that 0x1.921fb54442d180000000p+53f gets different values depending
on the rounding mode, in which case we would get back to the gcc12 failures.

Paul

Re: [PATCH 1/2] s_sincosf.h: Change pio4 type to float [BZ #28713]

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 1965 bytes --]

On Tue, Dec 21, 2021 at 12:52 AM Paul Zimmermann
<Paul.Zimmermann@inria.fr> wrote:
>
>        Dear H.J.,
>
> I confirm the 6 failures with gcc12, and that they are fixed by that patch.
>
> I suggest one small change: replace pio4 = 0x1.921FB54442D18p-1 by
> pio4 = 0x1.921fb6p-1f, where 0x1.921fb6p-1 is pi/4 rounded to nearest
> in binary32, and the 'f' suffix denotes a float constant.
>
> LGTM with that small change.

This is the patch I am checking in with

-static const double pio4 = 0x1.921FB54442D18p-1;
+static const float pio4 = 0x1.921FB6p-1f;

Thanks.

> Paul
>
> > Date: Mon, 20 Dec 2021 15:18:16 -0800
> > From: "H.J. Lu via Libc-alpha" <libc-alpha@sourceware.org>
> > Cc: Joseph Myers <joseph@codesourcery.com>
> >
> > s_cosf.c and s_sinf.c have
> >
> >   if (abstop12 (y) < abstop12 (pio4))
> >
> > where abstop12 takes a float argument, but pio4 is static const double.
> > pio4 is used only in calls to abstop12 and never in arithmetic.  Change
> > it to float to fix:
> >
> > FAIL: math/test-float-cos
> > FAIL: math/test-float-sin
> > FAIL: math/test-float-sincos
> > FAIL: math/test-float32-cos
> > FAIL: math/test-float32-sin
> > FAIL: math/test-float32-sincos
> >
> > when compiling with GCC 12.
> > ---
> >  sysdeps/ieee754/flt-32/s_sincosf.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/sysdeps/ieee754/flt-32/s_sincosf.h b/sysdeps/ieee754/flt-32/s_sincosf.h
> > index 125ab7f846..0dadd6374e 100644
> > --- a/sysdeps/ieee754/flt-32/s_sincosf.h
> > +++ b/sysdeps/ieee754/flt-32/s_sincosf.h
> > @@ -24,7 +24,7 @@
> >  /* 2PI * 2^-64.  */
> >  static const double pi63 = 0x1.921FB54442D18p-62;
> >  /* PI / 4.  */
> > -static const double pio4 = 0x1.921FB54442D18p-1;
> > +static const float pio4 = 0x1.921FB54442D18p-1;
> >
> >  /* Polynomial data (the cosine polynomial is negated in the 2nd entry).  */
> >  extern const sincos_t __sincosf_table[2] attribute_hidden;
> > --
> > 2.33.1
> >
> >



-- 
H.J.

[-- Attachment #2: 0001-s_sincosf.h-Change-pio4-type-to-float-BZ-28713.patch --]
[-- Type: text/x-patch, Size: 1432 bytes --]

From d3e4f5a1014db09ff1c62c6506f92cba469e193d Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Mon, 20 Dec 2021 14:37:26 -0800
Subject: [PATCH] s_sincosf.h: Change pio4 type to float [BZ #28713]

s_cosf.c and s_sinf.c have

  if (abstop12 (y) < abstop12 (pio4))

where abstop12 takes a float argument, but pio4 is static const double.
pio4 is used only in calls to abstop12 and never in arithmetic.  Apply

-static const double pio4 = 0x1.921FB54442D18p-1;
+static const float pio4 = 0x1.921FB6p-1f;

to fix:

FAIL: math/test-float-cos
FAIL: math/test-float-sin
FAIL: math/test-float-sincos
FAIL: math/test-float32-cos
FAIL: math/test-float32-sin
FAIL: math/test-float32-sincos

when compiling with GCC 12.

Reviewed-by: Paul Zimmermann <Paul.Zimmermann@inria.fr>
---
 sysdeps/ieee754/flt-32/s_sincosf.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sysdeps/ieee754/flt-32/s_sincosf.h b/sysdeps/ieee754/flt-32/s_sincosf.h
index 125ab7f846..372d8542c2 100644
--- a/sysdeps/ieee754/flt-32/s_sincosf.h
+++ b/sysdeps/ieee754/flt-32/s_sincosf.h
@@ -24,7 +24,7 @@
 /* 2PI * 2^-64.  */
 static const double pi63 = 0x1.921FB54442D18p-62;
 /* PI / 4.  */
-static const double pio4 = 0x1.921FB54442D18p-1;
+static const float pio4 = 0x1.921FB6p-1f;
 
 /* Polynomial data (the cosine polynomial is negated in the 2nd entry).  */
 extern const sincos_t __sincosf_table[2] attribute_hidden;
-- 
2.33.1

Re: [PATCH 2/2] math: Add X_TLOSSf [BZ #28713]

[H.J. Lu]

On Tue, Dec 21, 2021 at 2:34 AM Paul Zimmermann
<Paul.Zimmermann@inria.fr> wrote:
>
>        Florian,
>
> > Date: Tue, 21 Dec 2021 11:13:52 +0100
> > From: Florian Weimer via Libc-alpha <libc-alpha@sourceware.org>
> > Cc: Joseph Myers <joseph@codesourcery.com>
> >
> > * H. J. Lu via Libc-alpha:
> >
> > > diff --git a/math/math-svid-compat.h b/math/math-svid-compat.h
> > > index 5c18cb1b03..61d22ce461 100644
> > > --- a/math/math-svid-compat.h
> > > +++ b/math/math-svid-compat.h
> > > @@ -48,7 +48,8 @@ struct exception
> > >  extern int matherr (struct exception *__exc);
> > >  extern int __matherr (struct exception *__exc);
> > >
> > > -#define X_TLOSS    1.41484755040568800000e+16
> > > +#define X_TLOSS            0x1.921fb54442d180000000p+53
> > > +#define X_TLOSSf   0x1.921fb6p+53
> >
> > Is it possible to use a preprocessor macro with token pasting to create
> > a correctly rounded flaot constant from X_TLOSS?
> >
> > #define AS_FLOAT_CONSTANT_1(x) x##f
> > #define AS_FLOAT_CONSTANT(x) AS_FLOAT_CONSTANT_1(x)
>
> nice trick!

Fixed.

The v2 patch is at

https://sourceware.org/pipermail/libc-alpha/2021-December/134490.html

> > I expect that GCC will not raise any exceptions when such a constant is
> > evaluated at run time.
>
> is there a chance that the constant is evaluated at run time? In such a case,
> could it be that 0x1.921fb54442d180000000p+53f gets different values depending
> on the rounding mode, in which case we would get back to the gcc12 failures.
>
> Paul

Thanks.

-- 
H.J.

Re: [PATCH 2/2] math: Add X_TLOSSf [BZ #28713]

[Paul Zimmermann]

       Dear HJ,

> The v2 patch is at
> 
> https://sourceware.org/pipermail/libc-alpha/2021-December/134490.html

at first glance this looks ok to me, but I did not get an answer to that:

> > is there a chance that the constant is evaluated at run time? In such a case,
> > could it be that 0x1.921fb54442d180000000p+53f gets different values depending
> > on the rounding mode, in which case we would get back to the gcc12 failures.

Paul

Re: [PATCH 2/2] math: Add X_TLOSSf [BZ #28713]

[H.J. Lu]

On Wed, Dec 22, 2021 at 1:11 AM Paul Zimmermann
<Paul.Zimmermann@inria.fr> wrote:
>
>        Dear HJ,
>
> > The v2 patch is at
> >
> > https://sourceware.org/pipermail/libc-alpha/2021-December/134490.html
>
> at first glance this looks ok to me, but I did not get an answer to that:
>
> > > is there a chance that the constant is evaluated at run time? In such a case,
> > > could it be that 0x1.921fb54442d180000000p+53f gets different values depending
> > > on the rounding mode, in which case we would get back to the gcc12 failures.
>
> Paul

The C standard says that "An unsuffixed floating constant has type
double. If suffixed by
the letter f or F, it has type float."

-- 
H.J.

Re: [PATCH 2/2] math: Add X_TLOSSf [BZ #28713]

[Paul Zimmermann]

       Hi HJ,

> > > The v2 patch is at
> > >
> > > https://sourceware.org/pipermail/libc-alpha/2021-December/134490.html
> >
> > at first glance this looks ok to me, but I did not get an answer to that:
> >
> > > > is there a chance that the constant is evaluated at run time? In such a case,
> > > > could it be that 0x1.921fb54442d180000000p+53f gets different values depending
> > > > on the rounding mode, in which case we would get back to the gcc12 failures.
> >
> > Paul
> 
> The C standard says that "An unsuffixed floating constant has type
> double. If suffixed by
> the letter f or F, it has type float."

sorry this was not my question. My concern is whether
AS_FLOAT_CONSTANT(0x1.921fb54442d180000000p+53) is
evaluated at compile-time or run-time.

If at compile-time, the C standard says that it should be evaluated with
the default rounding mode (nearest ties to even), thus we always get
0x1.921fb6p1.

If at run-time, we could get different values depending on the current
rounding mode, and the bug you are trying to fix could come up again.

Can someone clarify this?

Paul

Re: [PATCH 2/2] math: Add X_TLOSSf [BZ #28713]

[H.J. Lu]

On Wed, Dec 22, 2021 at 5:35 AM Paul Zimmermann
<Paul.Zimmermann@inria.fr> wrote:
>
>        Hi HJ,
>
> > > > The v2 patch is at
> > > >
> > > > https://sourceware.org/pipermail/libc-alpha/2021-December/134490.html
> > >
> > > at first glance this looks ok to me, but I did not get an answer to that:
> > >
> > > > > is there a chance that the constant is evaluated at run time? In such a case,
> > > > > could it be that 0x1.921fb54442d180000000p+53f gets different values depending
> > > > > on the rounding mode, in which case we would get back to the gcc12 failures.
> > >
> > > Paul
> >
> > The C standard says that "An unsuffixed floating constant has type
> > double. If suffixed by
> > the letter f or F, it has type float."
>
> sorry this was not my question. My concern is whether
> AS_FLOAT_CONSTANT(0x1.921fb54442d180000000p+53) is
> evaluated at compile-time or run-time.
>
> If at compile-time, the C standard says that it should be evaluated with
> the default rounding mode (nearest ties to even), thus we always get
> 0x1.921fb6p1.

Since a floating constant is handled at compile-time by C frontend:

#0  real_from_string3 (r=0x7fffffffc4b0,
    s=0x7fffffffc420 "0x1.921fb54442d180000000p+53", fmt=...)
    at /export/gnu/import/git/gitlab/x86-gcc/gcc/real.c:2173
#1  0x0000000000ba9e78 in interpret_float (token=0x3a1add0, flags=530,
    suffix=0x0, overflow=0x7fffffffc6c4)
    at /export/gnu/import/git/gitlab/x86-gcc/gcc/c-family/c-lex.c:1035
#2  0x0000000000ba8cf3 in c_lex_with_flags (value=0x7ffff7fc7be0,
    loc=0x7ffff7fc7bdc, cpp_flags=0x7ffff7fc7be8 "\001", lex_flags=2)
    at /export/gnu/import/git/gitlab/x86-gcc/gcc/c-family/c-lex.c:517
#3  0x0000000000aec392 in c_lex_one_token (parser=0x7ffff7fc7bd0,
    token=0x7ffff7fc7bd8, raw=false)
    at /export/gnu/import/git/gitlab/x86-gcc/gcc/c/c-parser.c:279
#4  0x0000000000aec8cd in c_parser_peek_token (parser=0x7ffff7fc7bd0)
    at /export/gnu/import/git/gitlab/x86-gcc/gcc/c/c-parser.c:483
#5  0x0000000000aebf02 in c_parser_next_token_is (parser=0x7ffff7fc7bd0,
    type=CPP_CLOSE_PAREN)
    at /export/gnu/import/git/gitlab/x86-gcc/gcc/c/c-parser.h:167
#6  0x0000000000b0646d in c_parser_postfix_expression_after_primary (
    parser=0x7ffff7fc7bd0, expr_loc=20642, expr=...)
    at /export/gnu/import/git/gitlab/x86-gcc/gcc/c/c-parser.c:10529

> If at run-time, we could get different values depending on the current
> rounding mode, and the bug you are trying to fix could come up again.
>
> Can someone clarify this?
>
> Paul



-- 
H.J.

Re: [PATCH 2/2] math: Add X_TLOSSf [BZ #28713]

[Florian Weimer]

* H. J. Lu:

> On Wed, Dec 22, 2021 at 5:35 AM Paul Zimmermann
> <Paul.Zimmermann@inria.fr> wrote:
>>
>>        Hi HJ,
>>
>> > > > The v2 patch is at
>> > > >
>> > > > https://sourceware.org/pipermail/libc-alpha/2021-December/134490.html
>> > >
>> > > at first glance this looks ok to me, but I did not get an answer to that:
>> > >
>> > > > > is there a chance that the constant is evaluated at run time? In such a case,
>> > > > > could it be that 0x1.921fb54442d180000000p+53f gets different values depending
>> > > > > on the rounding mode, in which case we would get back to the gcc12 failures.
>> > >
>> > > Paul
>> >
>> > The C standard says that "An unsuffixed floating constant has type
>> > double. If suffixed by
>> > the letter f or F, it has type float."
>>
>> sorry this was not my question. My concern is whether
>> AS_FLOAT_CONSTANT(0x1.921fb54442d180000000p+53) is
>> evaluated at compile-time or run-time.
>>
>> If at compile-time, the C standard says that it should be evaluated with
>> the default rounding mode (nearest ties to even), thus we always get
>> 0x1.921fb6p1.
>
> Since a floating constant is handled at compile-time by C frontend:

But maybe this is a bug?  It's a bit counter-intuitive that

  0x1.921fb54442d180000000p+53f

and

  (float) 0x1.921fb54442d180000000p+53

give different results.

Thanks,
Florian

Re: [PATCH 2/2] math: Add X_TLOSSf [BZ #28713]

[H.J. Lu]

On Wed, Dec 22, 2021 at 6:04 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > On Wed, Dec 22, 2021 at 5:35 AM Paul Zimmermann
> > <Paul.Zimmermann@inria.fr> wrote:
> >>
> >>        Hi HJ,
> >>
> >> > > > The v2 patch is at
> >> > > >
> >> > > > https://sourceware.org/pipermail/libc-alpha/2021-December/134490.html
> >> > >
> >> > > at first glance this looks ok to me, but I did not get an answer to that:
> >> > >
> >> > > > > is there a chance that the constant is evaluated at run time? In such a case,
> >> > > > > could it be that 0x1.921fb54442d180000000p+53f gets different values depending
> >> > > > > on the rounding mode, in which case we would get back to the gcc12 failures.
> >> > >
> >> > > Paul
> >> >
> >> > The C standard says that "An unsuffixed floating constant has type
> >> > double. If suffixed by
> >> > the letter f or F, it has type float."
> >>
> >> sorry this was not my question. My concern is whether
> >> AS_FLOAT_CONSTANT(0x1.921fb54442d180000000p+53) is
> >> evaluated at compile-time or run-time.
> >>
> >> If at compile-time, the C standard says that it should be evaluated with
> >> the default rounding mode (nearest ties to even), thus we always get
> >> 0x1.921fb6p1.
> >
> > Since a floating constant is handled at compile-time by C frontend:
>
> But maybe this is a bug?  It's a bit counter-intuitive that
>
>   0x1.921fb54442d180000000p+53f
>
> and
>
>   (float) 0x1.921fb54442d180000000p+53
>
> give different results.

It is the same as

static float f = 0x1.921fb54442d180000000p+53f;

-- 
H.J.

Re: [PATCH 2/2] math: Add X_TLOSSf [BZ #28713]

[Joseph Myers]

On Wed, 22 Dec 2021, Paul Zimmermann wrote:

>        Dear HJ,
> 
> > The v2 patch is at
> > 
> > https://sourceware.org/pipermail/libc-alpha/2021-December/134490.html
> 
> at first glance this looks ok to me, but I did not get an answer to that:
> 
> > > is there a chance that the constant is evaluated at run time? In such a case,
> > > could it be that 0x1.921fb54442d180000000p+53f gets different values depending
> > > on the rounding mode, in which case we would get back to the gcc12 failures.

Floating constants are evaluated in the translation-time rounding mode, 
which is round-to-nearest unless the FENV_ROUND pragma is used.  They are 
never affected by the dynamic rounding mode.

Excess precision applies to floating constants (so when a floating 
constant is of a type with excess precision, it's evaluated - using the 
translation-time rounding mode - to a wider evaluation format).  
Depending on how the constant gets used, it's possible a narrowing to the 
range and precision of the type could occur at run time using the dynamic 
rounding mode.  This isn't relevant to glibc, given that we don't build 
with -fexcess-precision=standard and don't currently support _Float16 
functions (the case where excess precision is relevant without 
-fexcess-precision=standard).  Even with -fexcess precision=standard, it 
would only be relevant in cases where a runtime narrowing occurs (constant 
used for assignment / initialization / argument passing / return).

-- 
Joseph S. Myers
joseph@codesourcery.com