* [PATCH 1/2] s_sincosf.h: Change pio4 type to float [BZ #28713] @ 2021-12-20 23:18 H.J. Lu 2021-12-20 23:18 ` [PATCH 2/2] math: Add X_TLOSSf " H.J. Lu 2021-12-21 8:52 ` [PATCH 1/2] s_sincosf.h: Change pio4 type to float " Paul Zimmermann 0 siblings, 2 replies; 15+ messages in thread From: H.J. Lu @ 2021-12-20 23:18 UTC (permalink / raw) To: libc-alpha; +Cc: Joseph Myers 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/2] math: Add X_TLOSSf [BZ #28713] 2021-12-20 23:18 [PATCH 1/2] s_sincosf.h: Change pio4 type to float [BZ #28713] H.J. Lu @ 2021-12-20 23:18 ` H.J. Lu 2021-12-21 9:14 ` Paul Zimmermann 2021-12-21 10:13 ` Florian Weimer 2021-12-21 8:52 ` [PATCH 1/2] s_sincosf.h: Change pio4 type to float " Paul Zimmermann 1 sibling, 2 replies; 15+ messages in thread From: H.J. Lu @ 2021-12-20 23:18 UTC (permalink / raw) To: libc-alpha; +Cc: Joseph Myers 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] math: Add X_TLOSSf [BZ #28713] 2021-12-20 23:18 ` [PATCH 2/2] math: Add X_TLOSSf " H.J. Lu @ 2021-12-21 9:14 ` Paul Zimmermann 2021-12-21 10:13 ` Florian Weimer 1 sibling, 0 replies; 15+ messages in thread From: Paul Zimmermann @ 2021-12-21 9:14 UTC (permalink / raw) To: H.J. Lu; +Cc: libc-alpha, joseph 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] math: Add X_TLOSSf [BZ #28713] 2021-12-20 23:18 ` [PATCH 2/2] math: Add X_TLOSSf " H.J. Lu 2021-12-21 9:14 ` Paul Zimmermann @ 2021-12-21 10:13 ` Florian Weimer 2021-12-21 10:34 ` Paul Zimmermann 1 sibling, 1 reply; 15+ messages in thread From: Florian Weimer @ 2021-12-21 10:13 UTC (permalink / raw) To: H.J. Lu via Libc-alpha; +Cc: H.J. Lu, Joseph Myers * 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] math: Add X_TLOSSf [BZ #28713] 2021-12-21 10:13 ` Florian Weimer @ 2021-12-21 10:34 ` Paul Zimmermann 2021-12-21 17:01 ` H.J. Lu 0 siblings, 1 reply; 15+ messages in thread From: Paul Zimmermann @ 2021-12-21 10:34 UTC (permalink / raw) To: Florian Weimer; +Cc: libc-alpha, joseph 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] math: Add X_TLOSSf [BZ #28713] 2021-12-21 10:34 ` Paul Zimmermann @ 2021-12-21 17:01 ` H.J. Lu 2021-12-22 9:11 ` Paul Zimmermann 0 siblings, 1 reply; 15+ messages in thread From: H.J. Lu @ 2021-12-21 17:01 UTC (permalink / raw) To: Paul Zimmermann; +Cc: Florian Weimer, GNU C Library, Joseph S. Myers 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] math: Add X_TLOSSf [BZ #28713] 2021-12-21 17:01 ` H.J. Lu @ 2021-12-22 9:11 ` Paul Zimmermann 2021-12-22 13:21 ` H.J. Lu 2021-12-30 21:45 ` Joseph Myers 0 siblings, 2 replies; 15+ messages in thread From: Paul Zimmermann @ 2021-12-22 9:11 UTC (permalink / raw) To: H.J. Lu; +Cc: fweimer, libc-alpha, joseph 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] math: Add X_TLOSSf [BZ #28713] 2021-12-22 9:11 ` Paul Zimmermann @ 2021-12-22 13:21 ` H.J. Lu 2021-12-22 13:35 ` Paul Zimmermann 2021-12-30 21:45 ` Joseph Myers 1 sibling, 1 reply; 15+ messages in thread From: H.J. Lu @ 2021-12-22 13:21 UTC (permalink / raw) To: Paul Zimmermann; +Cc: Florian Weimer, GNU C Library, Joseph S. Myers 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] math: Add X_TLOSSf [BZ #28713] 2021-12-22 13:21 ` H.J. Lu @ 2021-12-22 13:35 ` Paul Zimmermann 2021-12-22 13:50 ` H.J. Lu 0 siblings, 1 reply; 15+ messages in thread From: Paul Zimmermann @ 2021-12-22 13:35 UTC (permalink / raw) To: H.J. Lu; +Cc: fweimer, libc-alpha, joseph 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] math: Add X_TLOSSf [BZ #28713] 2021-12-22 13:35 ` Paul Zimmermann @ 2021-12-22 13:50 ` H.J. Lu 2021-12-22 14:03 ` Florian Weimer 0 siblings, 1 reply; 15+ messages in thread From: H.J. Lu @ 2021-12-22 13:50 UTC (permalink / raw) To: Paul Zimmermann; +Cc: Florian Weimer, GNU C Library, Joseph S. Myers 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] math: Add X_TLOSSf [BZ #28713] 2021-12-22 13:50 ` H.J. Lu @ 2021-12-22 14:03 ` Florian Weimer 2021-12-22 14:30 ` H.J. Lu 0 siblings, 1 reply; 15+ messages in thread From: Florian Weimer @ 2021-12-22 14:03 UTC (permalink / raw) To: H.J. Lu; +Cc: Paul Zimmermann, GNU C Library, Joseph S. Myers * 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] math: Add X_TLOSSf [BZ #28713] 2021-12-22 14:03 ` Florian Weimer @ 2021-12-22 14:30 ` H.J. Lu 0 siblings, 0 replies; 15+ messages in thread From: H.J. Lu @ 2021-12-22 14:30 UTC (permalink / raw) To: Florian Weimer; +Cc: Paul Zimmermann, GNU C Library, Joseph S. Myers 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] math: Add X_TLOSSf [BZ #28713] 2021-12-22 9:11 ` Paul Zimmermann 2021-12-22 13:21 ` H.J. Lu @ 2021-12-30 21:45 ` Joseph Myers 1 sibling, 0 replies; 15+ messages in thread From: Joseph Myers @ 2021-12-30 21:45 UTC (permalink / raw) To: Paul Zimmermann; +Cc: H.J. Lu, fweimer, libc-alpha 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] s_sincosf.h: Change pio4 type to float [BZ #28713] 2021-12-20 23:18 [PATCH 1/2] s_sincosf.h: Change pio4 type to float [BZ #28713] H.J. Lu 2021-12-20 23:18 ` [PATCH 2/2] math: Add X_TLOSSf " H.J. Lu @ 2021-12-21 8:52 ` Paul Zimmermann 2021-12-21 16:58 ` H.J. Lu 1 sibling, 1 reply; 15+ messages in thread From: Paul Zimmermann @ 2021-12-21 8:52 UTC (permalink / raw) To: H.J. Lu; +Cc: libc-alpha, joseph 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 > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] s_sincosf.h: Change pio4 type to float [BZ #28713] 2021-12-21 8:52 ` [PATCH 1/2] s_sincosf.h: Change pio4 type to float " Paul Zimmermann @ 2021-12-21 16:58 ` H.J. Lu 0 siblings, 0 replies; 15+ messages in thread From: H.J. Lu @ 2021-12-21 16:58 UTC (permalink / raw) To: Paul Zimmermann; +Cc: GNU C Library, Joseph S. Myers [-- 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-12-30 21:45 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-20 23:18 [PATCH 1/2] s_sincosf.h: Change pio4 type to float [BZ #28713] H.J. Lu 2021-12-20 23:18 ` [PATCH 2/2] math: Add X_TLOSSf " H.J. Lu 2021-12-21 9:14 ` Paul Zimmermann 2021-12-21 10:13 ` Florian Weimer 2021-12-21 10:34 ` Paul Zimmermann 2021-12-21 17:01 ` H.J. Lu 2021-12-22 9:11 ` Paul Zimmermann 2021-12-22 13:21 ` H.J. Lu 2021-12-22 13:35 ` Paul Zimmermann 2021-12-22 13:50 ` H.J. Lu 2021-12-22 14:03 ` Florian Weimer 2021-12-22 14:30 ` H.J. Lu 2021-12-30 21:45 ` Joseph Myers 2021-12-21 8:52 ` [PATCH 1/2] s_sincosf.h: Change pio4 type to float " Paul Zimmermann 2021-12-21 16:58 ` H.J. Lu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).