This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH][PING] Inline C99 math functions
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Wilco Dijkstra <wdijkstr at arm dot com>, "'GNU C Library'" <libc-alpha at sourceware dot org>
- Date: Mon, 13 Jul 2015 12:56:07 -0400
- Subject: Re: [PATCH][PING] Inline C99 math functions
- Authentication-results: sourceware.org; auth=none
- References: <001c01d0bd7e$25a147c0$70e3d740$ at com>
On 07/13/2015 11:11 AM, Wilco Dijkstra wrote:
>> Wilco Dijkstra wrote:
>> Add inlining of the C99 math functions isinf/isnan/signbit/isfinite/isnormal/fpclassify using
>> GCC built-ins when available. Since going through the PLT is expensive for these small
>> functions, inlining results in major speedups (about 7x on Cortex-A57 for isinf). The GCC
>> built-ins are not correct if signalling NaN support is required, and thus are turned off in
>> that case (see GCC bug 66462). The test-snan.c tests sNaNs and so must be explicitly built
>> with -fsignaling-nans.
>>
>> As a result of this many target overrides and the various __isnan/__finite inlines in
>> math_private.h are no longer required. If agreed we could remove all this code and only keep
>> the generic definition of isinf/etc which will use the builtin.
>>
>> Tested on AArch64. OK for commit?
>>
>> ChangeLog:
>> 2015-06-15 Wilco Dijkstra <wdijkstr@arm.com>
>>
>> * math/Makefile: Build test-snan.c with -fsignaling-nans.
>> * math/math.h (fpclassify): Use __builtin_fpclassify when
>> available. (signbit): Use __builtin_signbit(f/l).
>> (isfinite): Use__builtin_isfinite. (isnormal): Use
>> __builtin_isnormal. (isnan): Use __builtin_isnan.
>> (isinf): Use __builtin_isinf_sign.
>
> As suggested __fpclassify is not inlined when optimizing for size, and a benchmark
> has been created (json output for x64 attached showing the large gains due to inlining).
>
> OK for commit?
This looks good to me for 2.23, it is not OK for 2.22.
Please wait until after 2.23 opens and until after you commit the benchmark.
Cheers,
Carlos.
> ---
> math/Makefile | 1 +
> math/math.h | 51 ++++++++++++++++++++++++++++++---------------------
> 2 files changed, 31 insertions(+), 21 deletions(-)
>
> diff --git a/math/Makefile b/math/Makefile
> index 9a3cf32..f78d75b 100644
> --- a/math/Makefile
> +++ b/math/Makefile
> @@ -155,6 +155,7 @@ CFLAGS-test-tgmath.c = -fno-builtin
> CFLAGS-test-tgmath2.c = -fno-builtin
> CFLAGS-test-tgmath-ret.c = -fno-builtin
> CFLAGS-test-powl.c = -fno-builtin
> +CFLAGS-test-snan.c = -fsignaling-nans
> CPPFLAGS-test-ifloat.c = -U__LIBC_INTERNAL_MATH_INLINES -D__FAST_MATH__ \
> -DTEST_FAST_MATH -fno-builtin
> CPPFLAGS-test-idouble.c = -U__LIBC_INTERNAL_MATH_INLINES -D__FAST_MATH__ \
> diff --git a/math/math.h b/math/math.h
> index 22f0989..1721118 100644
> --- a/math/math.h
> +++ b/math/math.h
> @@ -215,8 +215,15 @@ enum
> FP_NORMAL
> };
>
> +/* GCC bug 66462 means we cannot use the math builtins with -fsignaling-nan,
> + so disable builtins if this is enabled. When fixed in a newer GCC,
> + the __SUPPORT_SNAN__ check may be skipped for those versions. */
> +
> /* Return number of classification appropriate for X. */
> -# ifdef __NO_LONG_DOUBLE_MATH
> +# if __GNUC_PREREQ (4,4) && !defined __SUPPORT_SNAN__ && !defined __OPTIMIZE_SIZE__
> +# define fpclassify(x) __builtin_fpclassify (FP_NAN, FP_INFINITE, \
> + FP_NORMAL, FP_SUBNORMAL, FP_ZERO, x)
> +# elif defined __NO_LONG_DOUBLE_MATH
> # define fpclassify(x) \
> (sizeof (x) == sizeof (float) ? __fpclassifyf (x) : __fpclassify (x))
> # else
> @@ -229,32 +236,26 @@ enum
>
> /* Return nonzero value if sign of X is negative. */
> # if __GNUC_PREREQ (4,0)
> -# ifdef __NO_LONG_DOUBLE_MATH
> -# define signbit(x) \
> - (sizeof (x) == sizeof (float) \
> - ? __builtin_signbitf (x) : __builtin_signbit (x))
> -# else
> -# define signbit(x) \
> - (sizeof (x) == sizeof (float) \
> - ? __builtin_signbitf (x) \
> - : sizeof (x) == sizeof (double) \
> +# define signbit(x) \
> + (sizeof (x) == sizeof (float) \
> + ? __builtin_signbitf (x) \
> + : sizeof (x) == sizeof (double) \
> ? __builtin_signbit (x) : __builtin_signbitl (x))
> -# endif
> -# else
> -# ifdef __NO_LONG_DOUBLE_MATH
> -# define signbit(x) \
> +# elif defined __NO_LONG_DOUBLE_MATH
> +# define signbit(x) \
> (sizeof (x) == sizeof (float) ? __signbitf (x) : __signbit (x))
> -# else
> -# define signbit(x) \
> +# else
> +# define signbit(x) \
> (sizeof (x) == sizeof (float) \
> ? __signbitf (x) \
> : sizeof (x) == sizeof (double) \
> ? __signbit (x) : __signbitl (x))
> -# endif
> # endif
>
> /* Return nonzero value if X is not +-Inf or NaN. */
> -# ifdef __NO_LONG_DOUBLE_MATH
> +# if __GNUC_PREREQ (4,4) && !defined __SUPPORT_SNAN__
> +# define isfinite(x) __builtin_isfinite (x)
> +# elif defined __NO_LONG_DOUBLE_MATH
> # define isfinite(x) \
> (sizeof (x) == sizeof (float) ? __finitef (x) : __finite (x))
> # else
> @@ -266,11 +267,17 @@ enum
> # endif
>
> /* Return nonzero value if X is neither zero, subnormal, Inf, nor NaN. */
> -# define isnormal(x) (fpclassify (x) == FP_NORMAL)
> +# if __GNUC_PREREQ (4,4) && !defined __SUPPORT_SNAN__
> +# define isnormal(x) __builtin_isnormal (x)
> +# else
> +# define isnormal(x) (fpclassify (x) == FP_NORMAL)
> +# endif
>
> /* Return nonzero value if X is a NaN. We could use `fpclassify' but
> we already have this functions `__isnan' and it is faster. */
> -# ifdef __NO_LONG_DOUBLE_MATH
> +# if __GNUC_PREREQ (4,4) && !defined __SUPPORT_SNAN__
> +# define isnan(x) __builtin_isnan (x)
> +# elif defined __NO_LONG_DOUBLE_MATH
> # define isnan(x) \
> (sizeof (x) == sizeof (float) ? __isnanf (x) : __isnan (x))
> # else
> @@ -282,7 +289,9 @@ enum
> # endif
>
> /* Return nonzero value if X is positive or negative infinity. */
> -# ifdef __NO_LONG_DOUBLE_MATH
> +# if __GNUC_PREREQ (4,4) && !defined __SUPPORT_SNAN__
> +# define isinf(x) __builtin_isinf_sign (x)
> +# elif defined __NO_LONG_DOUBLE_MATH
> # define isinf(x) \
> (sizeof (x) == sizeof (float) ? __isinff (x) : __isinf (x))
> # else
>