This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v4 1/5] float128: Add public _Float128 declarations to libm.
On Thu, 11 May 2017, Gabriel F. T. Gomes wrote:
> * math/mathcalls.h (drem): Only define if __MATH_DECLARING_FLOATN.
> (gamma): Likewise.
> (nexttoward): Likewise.
> (significand): Likewise.
> (pow10): Likewise.
> (scalb): Likewise.
> (finite): Likewise.
> (isinf): Likewise.
> (isnan): Likewise.
That's if *not* __MATH_DECLARING_FLOATN.
> * math/math.h (__MATH_DECLARING_FLOATN): New macro to control
Your ChangeLog entry for math.h doesn't seem to mention e.g. the inclusion
of <bits/floatn.h> or <bits/huge_val_flt128.h>. Please review all the
ChangeLog entries against the actual patch (once the code fixes listed
here are done).
> [__HAVE_DISTINCT_FLOAT128 && __GLIBC_USE (IEC_60559_TYPES_EXT)]
> (M_Ef128): New _GNU_SOURCE enabled macro.
No, the code is correct here and the ChangeLog entry is wrong; the
condition #if __HAVE_FLOAT128 && defined __USE_GNU in the code on these
macros is the correct one for them.
> +#if __HAVE_FLOAT128 && __GLIBC_USE (IEC_60559_TYPES_EXT)
> +# ifndef _Mfloat128_
> +# define _Mfloat128_ _Float128
> +# endif
> +/* GCC < 7 requires extra convincing to expose a complex float128 type. */
> +# ifdef __CFLOAT128
> +# undef _Mdouble_complex_
> +# define _Mdouble_complex_ __CFLOAT128
> +# endif
> +# define _Mdouble_ _Mfloat128_
> +# define __MATH_PRECNAME(name) name##f128
> +# include <bits/cmathcalls.h>
The condition for including bits/cmathcalls.h should match that for
including bits/mathcalls.h. That is, __GLIBC_USE (IEC_60559_TYPES_EXT) &&
(__HAVE_DISTINCT_FLOAT128 || (__HAVE_FLOAT128 && !defined _LIBC)) (or
something logically equivalent).
> /* Return nonzero value if X is positive or negative infinity. */
> -# if __GNUC_PREREQ (4,4) && !defined __SUPPORT_SNAN__
> +# if __HAVE_DISTINCT_FLOAT128 && !__GNUC_PREREQ (7,0) \
> + && !defined __SUPPORT_SNAN__
> + /* __builtin_isinf_sign is broken for float128 only before GCC 7.0. */
> +# define isinf(x) \
> + (__builtin_types_compatible_p (typeof (x), _Float128) \
> + ? __isinff128 (x) : __builtin_isinf_sign (x))
> +# elif __GNUC_PREREQ (4,4) && !defined __SUPPORT_SNAN__
You need to use __typeof or __typeof__ here, not typeof, to be
namespace-clean.
OK with those changes.
Note on libm header omissions from this patch, included or expected in
subsequent patches:
In subsequent patches in the present series: math-finite.h and related
changes; SNANF128.
Expected in a subsequent patch series as part of completing enabling
_Float128 support: a definition of __MATH_TG for the
__HAVE_DISTINCT_FLOAT128 case; tgmath.h support.
Not needed in the initial support: anything to do with FP_FAST_FMAF128.
(GCC 7 does not have built-in fmaf128 etc. or define __FP_FAST_FMAF128
etc.; that's not part of the minimal set of built-in functions for all
floating-point types. FP_FAST_FMAF128 would presumably be true for POWER9
if you add a POWER9 version of fmaf128 that uses the xsmaddqp instruction,
which ought to be faster than the software version of fmaf128, but of
course that's not needed in the initial support.)
--
Joseph S. Myers
joseph@codesourcery.com