math: Fix float conversion regressions with gcc-12 [BZ #28713]
Checks
| Context |
Check |
Description |
| dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
| dj/TryBot-32bit |
success
|
Build for i686
|
Commit Message
Converting double precision constants to float is now affected by the
runtime dynamic rounding mode instead of being evaluated at compile
time with default rounding mode (except static object initializers).
This can change the computed result and cause performance regression.
The known correctness issues (increased ulp errors) are already fixed,
this patch fixes remaining cases of unnecessary runtime conversions.
Add float M_* macros to math.h as new GNU extension API. To avoid
conversions the new M_* macros are used and instead of casting double
literals to float, use float literals (only required if the conversion
is inexact).
The patch was tested on aarch64 where the following symbols had new
spurious conversion instructions that got fixed:
__clog10f
__gammaf_r_finite@GLIBC_2.17
__j0f_finite@GLIBC_2.17
__j1f_finite@GLIBC_2.17
__jnf_finite@GLIBC_2.17
__kernel_casinhf
__lgamma_negf
__log1pf
__y0f_finite@GLIBC_2.17
__y1f_finite@GLIBC_2.17
cacosf
cacoshf
casinhf
catanf
catanhf
clogf
gammaf_positive
Fixes bug 28713.
---
manual/math.texi | 7 ++++---
math/k_casinh_template.c | 2 +-
math/math.h | 17 +++++++++++++++++
math/s_cacos_template.c | 2 +-
math/s_catan_template.c | 2 +-
math/s_catanh_template.c | 2 +-
math/s_clog10_template.c | 8 ++++----
math/s_clog_template.c | 4 ++--
sysdeps/generic/math-type-macros-float.h | 5 ++---
sysdeps/ieee754/flt-32/e_gammaf_r.c | 12 ++++++------
sysdeps/ieee754/flt-32/e_j0f.c | 4 ++--
sysdeps/ieee754/flt-32/e_j1f.c | 4 ++--
sysdeps/ieee754/flt-32/e_jnf.c | 2 +-
sysdeps/ieee754/flt-32/lgamma_negf.c | 8 ++++----
sysdeps/ieee754/flt-32/s_log1pf.c | 2 +-
15 files changed, 49 insertions(+), 32 deletions(-)
Comments
On Fri, 31 Dec 2021, Szabolcs Nagy via Libc-alpha wrote:
> Add float M_* macros to math.h as new GNU extension API. To avoid
> conversions the new M_* macros are used and instead of casting double
> literals to float, use float literals (only required if the conversion
> is inexact).
As a new user-visible feature, I think the new macros should get a NEWS
entry.
Dear Szabolcs,
+# define M_PI_2f 1.57079632679489661923f /* pi/2 */
- __real__ res = (FLOAT) M_MLIT (M_PI_2) - __real__ y;
+ __real__ res = M_MLIT (M_PI_2) - __real__ y;
I wonder why your patch never uses M_PI_2f. Should it be M_PI_2f above?
Best regards,
Paul
The 01/01/2022 07:45, Paul Zimmermann wrote:
> Dear Szabolcs,
>
> +# define M_PI_2f 1.57079632679489661923f /* pi/2 */
>
> - __real__ res = (FLOAT) M_MLIT (M_PI_2) - __real__ y;
> + __real__ res = M_MLIT (M_PI_2) - __real__ y;
>
> I wonder why your patch never uses M_PI_2f. Should it be M_PI_2f above?
this is template code under math/ and M_MLIT (M_PI_2) should
expand to M_PI_2f when it is used for the float variant.
see the change in
sysdeps/generic/math-type-macros-float.h
Dear Szabolcs,
> > +# define M_PI_2f 1.57079632679489661923f /* pi/2 */
> >
> > - __real__ res = (FLOAT) M_MLIT (M_PI_2) - __real__ y;
> > + __real__ res = M_MLIT (M_PI_2) - __real__ y;
> >
> > I wonder why your patch never uses M_PI_2f. Should it be M_PI_2f above?
>
> this is template code under math/ and M_MLIT (M_PI_2) should
> expand to M_PI_2f when it is used for the float variant.
>
> see the change in
> sysdeps/generic/math-type-macros-float.h
ok.
While looking at that patch, in math/math.h, I thought new constants should
be declared as hexadecimal values?
+# define M_Ef 2.7182818284590452354f /* e */
Otherwise this patch seems good to me.
Paul
The 01/03/2022 11:24, Paul Zimmermann wrote:
> Dear Szabolcs,
>
> > > +# define M_PI_2f 1.57079632679489661923f /* pi/2 */
> > >
> > > - __real__ res = (FLOAT) M_MLIT (M_PI_2) - __real__ y;
> > > + __real__ res = M_MLIT (M_PI_2) - __real__ y;
> > >
> > > I wonder why your patch never uses M_PI_2f. Should it be M_PI_2f above?
> >
> > this is template code under math/ and M_MLIT (M_PI_2) should
> > expand to M_PI_2f when it is used for the float variant.
> >
> > see the change in
> > sysdeps/generic/math-type-macros-float.h
>
> ok.
>
> While looking at that patch, in math/math.h, I thought new constants should
> be declared as hexadecimal values?
>
> +# define M_Ef 2.7182818284590452354f /* e */
>
> Otherwise this patch seems good to me.
this is a public header, i think decimal consts
are used because hexadecimal is >=c99 only and
not c++ compatible (at least before c++17).
Dear Szabolcs,
> > While looking at that patch, in math/math.h, I thought new constants should
> > be declared as hexadecimal values?
> >
> > +# define M_Ef 2.7182818284590452354f /* e */
> >
> > Otherwise this patch seems good to me.
>
> this is a public header, i think decimal consts
> are used because hexadecimal is >=c99 only and
> not c++ compatible (at least before c++17).
ok, this looks good to me, all test still pass with gcc 12 (on x86_64 linux),
except elf/tst-env-setuid, but I believe this is unrelated.
Reviewed-by: Paul Zimmermann <Paul.Zimmermann@inria.fr>
Thanks!
Paul
@@ -131,9 +131,10 @@ defined. The default set of features includes these constants.
@xref{Feature Test Macros}.
All values are of type @code{double}. As an extension, @theglibc{}
-also defines these constants with type @code{long double}. The
-@code{long double} macros have a lowercase @samp{l} appended to their
-names: @code{M_El}, @code{M_PIl}, and so forth. These are only
+also defines these constants with type @code{long double} and
+@code{float}. The @code{long double} macros have a lowercase @samp{l}
+while the @code{float} macros have a lowercase @samp{f} appended to
+their names: @code{M_El}, @code{M_PIl}, and so forth. These are only
available if @code{_GNU_SOURCE} is defined.
Likewise, @theglibc{} also defines these constants with the types
@@ -56,7 +56,7 @@ M_DECL_FUNC (__kernel_casinh) (CFLOAT x, int adj)
}
res = M_SUF (__clog) (y);
- __real__ res += (FLOAT) M_MLIT (M_LN2);
+ __real__ res += M_MLIT (M_LN2);
}
else if (rx >= M_LIT (0.5) && ix < M_EPSILON / 8)
{
@@ -1158,6 +1158,23 @@ iszero (__T __val)
# define M_SQRT1_2 0.70710678118654752440 /* 1/sqrt(2) */
#endif
+/* GNU extension to provide float constants with similar names. */
+#ifdef __USE_GNU
+# define M_Ef 2.7182818284590452354f /* e */
+# define M_LOG2Ef 1.4426950408889634074f /* log_2 e */
+# define M_LOG10Ef 0.43429448190325182765f /* log_10 e */
+# define M_LN2f 0.69314718055994530942f /* log_e 2 */
+# define M_LN10f 2.30258509299404568402f /* log_e 10 */
+# define M_PIf 3.14159265358979323846f /* pi */
+# define M_PI_2f 1.57079632679489661923f /* pi/2 */
+# define M_PI_4f 0.78539816339744830962f /* pi/4 */
+# define M_1_PIf 0.31830988618379067154f /* 1/pi */
+# define M_2_PIf 0.63661977236758134308f /* 2/pi */
+# define M_2_SQRTPIf 1.12837916709551257390f /* 2/sqrt(pi) */
+# define M_SQRT2f 1.41421356237309504880f /* sqrt(2) */
+# define M_SQRT1_2f 0.70710678118654752440f /* 1/sqrt(2) */
+#endif
+
/* The above constants are not adequate for computation using `long double's.
Therefore we provide as an extension constants with similar names as a
GNU extension. Provide enough digits for the 128-bit IEEE quad. */
@@ -32,7 +32,7 @@ M_DECL_FUNC (__cacos) (CFLOAT x)
{
y = M_SUF (__casin) (x);
- __real__ res = (FLOAT) M_MLIT (M_PI_2) - __real__ y;
+ __real__ res = M_MLIT (M_PI_2) - __real__ y;
if (__real__ res == 0)
__real__ res = 0;
__imag__ res = -__imag__ y;
@@ -106,7 +106,7 @@ M_DECL_FUNC (__catan) (CFLOAT x)
if (M_FABS (__imag__ x) == 1
&& M_FABS (__real__ x) < M_EPSILON * M_EPSILON)
__imag__ res = (M_COPYSIGN (M_LIT (0.5), __imag__ x)
- * ((FLOAT) M_MLIT (M_LN2)
+ * (M_MLIT (M_LN2)
- M_LOG (M_FABS (__real__ x))));
else
{
@@ -75,7 +75,7 @@ M_DECL_FUNC (__catanh) (CFLOAT x)
if (M_FABS (__real__ x) == 1
&& M_FABS (__imag__ x) < M_EPSILON * M_EPSILON)
__real__ res = (M_COPYSIGN (M_LIT (0.5), __real__ x)
- * ((FLOAT) M_MLIT (M_LN2)
+ * (M_MLIT (M_LN2)
- M_LOG (M_FABS (__imag__ x))));
else
{
@@ -72,7 +72,7 @@ M_DECL_FUNC (__clog10) (CFLOAT x)
if (absx == 1 && scale == 0)
{
__real__ result = (M_LOG1P (absy * absy)
- * ((FLOAT) M_MLIT (M_LOG10E) / 2));
+ * (M_MLIT (M_LOG10E) / 2));
math_check_force_underflow_nonneg (__real__ result);
}
else if (absx > 1 && absx < 2 && absy < 1 && scale == 0)
@@ -80,7 +80,7 @@ M_DECL_FUNC (__clog10) (CFLOAT x)
FLOAT d2m1 = (absx - 1) * (absx + 1);
if (absy >= M_EPSILON)
d2m1 += absy * absy;
- __real__ result = M_LOG1P (d2m1) * ((FLOAT) M_MLIT (M_LOG10E) / 2);
+ __real__ result = M_LOG1P (d2m1) * (M_MLIT (M_LOG10E) / 2);
}
else if (absx < 1
&& absx >= M_LIT (0.5)
@@ -88,7 +88,7 @@ M_DECL_FUNC (__clog10) (CFLOAT x)
&& scale == 0)
{
FLOAT d2m1 = (absx - 1) * (absx + 1);
- __real__ result = M_LOG1P (d2m1) * ((FLOAT) M_MLIT (M_LOG10E) / 2);
+ __real__ result = M_LOG1P (d2m1) * (M_MLIT (M_LOG10E) / 2);
}
else if (absx < 1
&& absx >= M_LIT (0.5)
@@ -96,7 +96,7 @@ M_DECL_FUNC (__clog10) (CFLOAT x)
&& absx * absx + absy * absy >= M_LIT (0.5))
{
FLOAT d2m1 = M_SUF (__x2y2m1) (absx, absy);
- __real__ result = M_LOG1P (d2m1) * ((FLOAT) M_MLIT (M_LOG10E) / 2);
+ __real__ result = M_LOG1P (d2m1) * (M_MLIT (M_LOG10E) / 2);
}
else
{
@@ -32,7 +32,7 @@ M_DECL_FUNC (__clog) (CFLOAT x)
if (__glibc_unlikely (rcls == FP_ZERO && icls == FP_ZERO))
{
/* Real and imaginary part are 0.0. */
- __imag__ result = signbit (__real__ x) ? (FLOAT) M_MLIT (M_PI) : 0;
+ __imag__ result = signbit (__real__ x) ? M_MLIT (M_PI) : 0;
__imag__ result = M_COPYSIGN (__imag__ result, __imag__ x);
/* Yes, the following line raises an exception. */
__real__ result = -1 / M_FABS (__real__ x);
@@ -94,7 +94,7 @@ M_DECL_FUNC (__clog) (CFLOAT x)
else
{
FLOAT d = M_HYPOT (absx, absy);
- __real__ result = M_LOG (d) - scale * (FLOAT) M_MLIT (M_LN2);
+ __real__ result = M_LOG (d) - scale * M_MLIT (M_LN2);
}
__imag__ result = M_ATAN2 (__imag__ x, __real__ x);
@@ -27,9 +27,8 @@
#define M_STRTO_NAN __strtof_nan
#define M_USE_BUILTIN(c) USE_ ##c ##F_BUILTIN
-/* Standard/GNU macro literals do not exist for the float type. Use
- the double macro constants. */
-#define M_MLIT(c) c
+/* GNU extension float constant macros. */
+#define M_MLIT(c) c ## f
#include <libm-alias-float.h>
#include <math-nan-payload-float.h>
@@ -85,7 +85,7 @@ gammaf_positive (float x, int *exp2_adj)
float x_adj_frac = x_adj - x_adj_int;
int x_adj_log2;
float x_adj_mant = __frexpf (x_adj, &x_adj_log2);
- if (x_adj_mant < (float) M_SQRT1_2)
+ if (x_adj_mant < M_SQRT1_2f)
{
x_adj_log2--;
x_adj_mant *= 2.0f;
@@ -94,7 +94,7 @@ gammaf_positive (float x, int *exp2_adj)
float ret = (__ieee754_powf (x_adj_mant, x_adj)
* __ieee754_exp2f (x_adj_log2 * x_adj_frac)
* __ieee754_expf (-x_adj)
- * sqrtf (2 * (float) M_PI / x_adj)
+ * sqrtf (2 * M_PIf / x_adj)
/ prod);
exp_adj += x_eps * __ieee754_logf (x_adj);
float bsum = gamma_coeff[NCOEFF - 1];
@@ -176,11 +176,11 @@ __ieee754_gammaf_r (float x, int *signgamp)
if (frac > 0.5f)
frac = 1.0f - frac;
float sinpix = (frac <= 0.25f
- ? __sinf ((float) M_PI * frac)
- : __cosf ((float) M_PI * (0.5f - frac)));
+ ? __sinf (M_PIf * frac)
+ : __cosf (M_PIf * (0.5f - frac)));
int exp2_adj;
- float tret = (float) M_PI / (-x * sinpix
- * gammaf_positive (-x, &exp2_adj));
+ float tret = M_PIf / (-x * sinpix
+ * gammaf_positive (-x, &exp2_adj));
ret = __scalbnf (tret, -exp2_adj);
math_check_force_underflow_nonneg (ret);
}
@@ -233,7 +233,7 @@ j0f_near_root (float x, float z)
float index_f;
int index;
- index_f = roundf ((x - FIRST_ZERO_J0) / (float) M_PI);
+ index_f = roundf ((x - FIRST_ZERO_J0) / M_PIf);
/* j0f_asympt fails to give an error <= 9 ulps for x=0x1.324e92p+7
(index 48) thus we can't reduce SMALL_SIZE below 49. */
if (index_f >= SMALL_SIZE)
@@ -514,7 +514,7 @@ y0f_near_root (float x, float z)
float index_f;
int index;
- index_f = roundf ((x - FIRST_ZERO_Y0) / (float) M_PI);
+ index_f = roundf ((x - FIRST_ZERO_Y0) / M_PIf);
if (index_f >= SMALL_SIZE)
return y0f_asympt (x);
index = (int) index_f;
@@ -243,7 +243,7 @@ j1f_near_root (float x, float z)
x = -x;
sign = -1.0f;
}
- index_f = roundf ((x - FIRST_ZERO_J1) / (float) M_PI);
+ index_f = roundf ((x - FIRST_ZERO_J1) / M_PIf);
if (index_f >= SMALL_SIZE)
return sign * j1f_asympt (x);
index = (int) index_f;
@@ -525,7 +525,7 @@ y1f_near_root (float x, float z)
float index_f;
int index;
- index_f = roundf ((x - FIRST_ZERO_Y1) / (float) M_PI);
+ index_f = roundf ((x - FIRST_ZERO_Y1) / M_PIf);
if (index_f >= SMALL_SIZE)
return y1f_asympt (x);
index = (int) index_f;
@@ -134,7 +134,7 @@ __ieee754_jnf(int n, float x)
tmp = n;
v = two/x;
tmp = tmp*__ieee754_logf(fabsf(v*tmp));
- if(tmp<(float)8.8721679688e+01) {
+ if(tmp<8.8721679688e+01f) {
for(i=n-1,di=(float)(i+i);i>0;i--){
temp = b;
b *= di;
@@ -165,9 +165,9 @@ static float
lg_sinpi (float x)
{
if (x <= 0.25f)
- return __sinf ((float) M_PI * x);
+ return __sinf (M_PIf * x);
else
- return __cosf ((float) M_PI * (0.5f - x));
+ return __cosf (M_PIf * (0.5f - x));
}
/* Compute cos (pi * X) for -0.25 <= X <= 0.5. */
@@ -176,9 +176,9 @@ static float
lg_cospi (float x)
{
if (x <= 0.25f)
- return __cosf ((float) M_PI * x);
+ return __cosf (M_PIf * x);
else
- return __sinf ((float) M_PI * (0.5f - x));
+ return __sinf (M_PIf * (0.5f - x));
}
/* Compute cot (pi * X) for -0.25 <= X <= 0.5. */
@@ -92,7 +92,7 @@ __log1pf(float x)
if(k==0) return zero;
else {c += k*ln2_lo; return k*ln2_hi+c;}
}
- R = hfsq*((float)1.0-(float)0.66666666666666666*f);
+ R = hfsq*(1.0f-0.66666666666666666f*f);
if(k==0) return f-R; else
return k*ln2_hi-((R-(k*ln2_lo+c))-f);
}