* [PATCH][PING] Inline C99 math functions
@ 2015-07-13 15:11 Wilco Dijkstra
2015-07-13 16:56 ` Carlos O'Donell
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Wilco Dijkstra @ 2015-07-13 15:11 UTC (permalink / raw)
To: 'GNU C Library'
[-- Attachment #1: Type: text/plain, Size: 5698 bytes --]
> 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?
---
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
--
1.9.1
[-- Attachment #2: bench-math-inlines.out --]
[-- Type: application/octet-stream, Size: 5964 bytes --]
"math-inlines": {
"__isnan_t": {
"inf/nan": {
"duration": 3.55523e+06,
"iterations": 500,
"mean": 7110
}
},
"__isnan_inl_t": {
"inf/nan": {
"duration": 1.98773e+06,
"iterations": 500,
"mean": 3975
}
},
"__isnan_builtin_t": {
"inf/nan": {
"duration": 1.45992e+06,
"iterations": 500,
"mean": 2919
}
},
"isnan_t": {
"inf/nan": {
"duration": 1.46146e+06,
"iterations": 500,
"mean": 2922
}
},
"__isinf_t": {
"inf/nan": {
"duration": 4.45733e+06,
"iterations": 500,
"mean": 8914
}
},
"__isinf_inl_t": {
"inf/nan": {
"duration": 1.62436e+06,
"iterations": 500,
"mean": 3248
}
},
"__isinf_ns_t": {
"inf/nan": {
"duration": 1.75246e+06,
"iterations": 500,
"mean": 3504
}
},
"__isinf_ns_builtin_t": {
"inf/nan": {
"duration": 1.59383e+06,
"iterations": 500,
"mean": 3187
}
},
"__isinf_builtin_t": {
"inf/nan": {
"duration": 1.57536e+06,
"iterations": 500,
"mean": 3150
}
},
"isinf_t": {
"inf/nan": {
"duration": 1.60067e+06,
"iterations": 500,
"mean": 3201
}
},
"__finite_t": {
"inf/nan": {
"duration": 3.5604e+06,
"iterations": 500,
"mean": 7120
}
},
"__finite_inl_t": {
"inf/nan": {
"duration": 1.98869e+06,
"iterations": 500,
"mean": 3977
}
},
"__isfinite_builtin_t": {
"inf/nan": {
"duration": 1.77556e+06,
"iterations": 500,
"mean": 3551
}
},
"isfinite_t": {
"inf/nan": {
"duration": 1.4908e+06,
"iterations": 500,
"mean": 2981
}
},
"__isnormal_inl_t": {
"inf/nan": {
"duration": 5.0671e+06,
"iterations": 500,
"mean": 10134
}
},
"__isnormal_inl2_t": {
"inf/nan": {
"duration": 5.55487e+06,
"iterations": 500,
"mean": 11109
}
},
"__isnormal_builtin_t": {
"inf/nan": {
"duration": 2.04409e+06,
"iterations": 500,
"mean": 4088
}
},
"isnormal_t": {
"inf/nan": {
"duration": 2.19744e+06,
"iterations": 500,
"mean": 4394
}
},
"__fpclassify_test1_t": {
"inf/nan": {
"duration": 1.59605e+06,
"iterations": 500,
"mean": 3192
}
},
"__fpclassify_test2_t": {
"inf/nan": {
"duration": 1.6258e+06,
"iterations": 500,
"mean": 3251
}
},
"__fpclassify_t": {
"inf/nan": {
"duration": 5.20152e+06,
"iterations": 500,
"mean": 10403
}
},
"fpclassify_t": {
"inf/nan": {
"duration": 1.49068e+06,
"iterations": 500,
"mean": 2981
}
},
"remainder_test1_t": {
"inf/nan": {
"duration": 9.76677e+07,
"iterations": 500,
"mean": 195335
}
},
"remainder_test2_t": {
"inf/nan": {
"duration": 9.59216e+07,
"iterations": 500,
"mean": 191843
}
},
"__isnan_t": {
"normal": {
"duration": 3.54827e+06,
"iterations": 500,
"mean": 7096
}
},
"__isnan_inl_t": {
"normal": {
"duration": 1.98789e+06,
"iterations": 500,
"mean": 3975
}
},
"__isnan_builtin_t": {
"normal": {
"duration": 1.46146e+06,
"iterations": 500,
"mean": 2922
}
},
"isnan_t": {
"normal": {
"duration": 1.46201e+06,
"iterations": 500,
"mean": 2924
}
},
"__isinf_t": {
"normal": {
"duration": 4.39731e+06,
"iterations": 500,
"mean": 8794
}
},
"__isinf_inl_t": {
"normal": {
"duration": 1.46842e+06,
"iterations": 500,
"mean": 2936
}
},
"__isinf_ns_t": {
"normal": {
"duration": 1.752e+06,
"iterations": 500,
"mean": 3504
}
},
"__isinf_ns_builtin_t": {
"normal": {
"duration": 1.59423e+06,
"iterations": 500,
"mean": 3188
}
},
"__isinf_builtin_t": {
"normal": {
"duration": 1.57521e+06,
"iterations": 500,
"mean": 3150
}
},
"isinf_t": {
"normal": {
"duration": 1.59967e+06,
"iterations": 500,
"mean": 3199
}
},
"__finite_t": {
"normal": {
"duration": 3.53613e+06,
"iterations": 500,
"mean": 7072
}
},
"__finite_inl_t": {
"normal": {
"duration": 1.99061e+06,
"iterations": 500,
"mean": 3981
}
},
"__isfinite_builtin_t": {
"normal": {
"duration": 1.49156e+06,
"iterations": 500,
"mean": 2983
}
},
"isfinite_t": {
"normal": {
"duration": 1.78218e+06,
"iterations": 500,
"mean": 3564
}
},
"__isnormal_inl_t": {
"normal": {
"duration": 6.2988e+06,
"iterations": 500,
"mean": 12597
}
},
"__isnormal_inl2_t": {
"normal": {
"duration": 2.21109e+06,
"iterations": 500,
"mean": 4422
}
},
"__isnormal_builtin_t": {
"normal": {
"duration": 2.38751e+06,
"iterations": 500,
"mean": 4775
}
},
"isnormal_t": {
"normal": {
"duration": 2.18877e+06,
"iterations": 500,
"mean": 4377
}
},
"__fpclassify_test1_t": {
"normal": {
"duration": 1.55142e+06,
"iterations": 500,
"mean": 3102
}
},
"__fpclassify_test2_t": {
"normal": {
"duration": 1.44541e+06,
"iterations": 500,
"mean": 2890
}
},
"__fpclassify_t": {
"normal": {
"duration": 6.23151e+06,
"iterations": 500,
"mean": 12463
}
},
"fpclassify_t": {
"normal": {
"duration": 1.49156e+06,
"iterations": 500,
"mean": 2983
}
},
"remainder_test1_t": {
"normal": {
"duration": 2.18576e+07,
"iterations": 500,
"mean": 43715
}
},
"remainder_test2_t": {
"normal": {
"duration": 2.05037e+07,
"iterations": 500,
"mean": 41007
}
}
}
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH][PING] Inline C99 math functions
2015-07-13 15:11 [PATCH][PING] Inline C99 math functions Wilco Dijkstra
@ 2015-07-13 16:56 ` Carlos O'Donell
2015-07-22 15:16 ` Joseph Myers
2015-11-04 15:04 ` Andreas Schwab
2 siblings, 0 replies; 8+ messages in thread
From: Carlos O'Donell @ 2015-07-13 16:56 UTC (permalink / raw)
To: Wilco Dijkstra, 'GNU C Library'
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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH][PING] Inline C99 math functions
2015-07-13 15:11 [PATCH][PING] Inline C99 math functions Wilco Dijkstra
2015-07-13 16:56 ` Carlos O'Donell
@ 2015-07-22 15:16 ` Joseph Myers
2015-07-22 15:58 ` Wilco Dijkstra
2015-11-04 15:04 ` Andreas Schwab
2 siblings, 1 reply; 8+ messages in thread
From: Joseph Myers @ 2015-07-22 15:16 UTC (permalink / raw)
To: Wilco Dijkstra; +Cc: 'GNU C Library'
On Mon, 13 Jul 2015, 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).
I don't see an updated ChangeLog entry (with the [BZ #N] notation I
requested). Please include the ChangeLog entry with each patch
submission.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH][PING] Inline C99 math functions
2015-07-22 15:16 ` Joseph Myers
@ 2015-07-22 15:58 ` Wilco Dijkstra
2015-07-22 19:47 ` Joseph Myers
0 siblings, 1 reply; 8+ messages in thread
From: Wilco Dijkstra @ 2015-07-22 15:58 UTC (permalink / raw)
To: 'Joseph Myers'; +Cc: 'GNU C Library'
> Joseph Myers wrote:
> On Mon, 13 Jul 2015, 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).
>
> I don't see an updated ChangeLog entry (with the [BZ #N] notation I
> requested). Please include the ChangeLog entry with each patch
> submission.
It was still in the quotes. Something like this?
2015-07-xx 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 - fixes [BZ #17441].
(isinf): Use __builtin_isinf_sign - fixes [BZ #15367].
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH][PING] Inline C99 math functions
2015-07-22 15:58 ` Wilco Dijkstra
@ 2015-07-22 19:47 ` Joseph Myers
0 siblings, 0 replies; 8+ messages in thread
From: Joseph Myers @ 2015-07-22 19:47 UTC (permalink / raw)
To: Wilco Dijkstra; +Cc: 'GNU C Library'
On Wed, 22 Jul 2015, Wilco Dijkstra wrote:
> > I don't see an updated ChangeLog entry (with the [BZ #N] notation I
> > requested). Please include the ChangeLog entry with each patch
> > submission.
>
> It was still in the quotes. Something like this?
>
> 2015-07-xx 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 - fixes [BZ #17441].
> (isinf): Use __builtin_isinf_sign - fixes [BZ #15367].
That's not the style used (and your line breaks are all wrong).
<TAB>[BZ #N]<LF> at the start of the ChangeLog entry, after the initial
author-date line and blank line and before the lines describing changes to
each file.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH][PING] Inline C99 math functions
2015-07-13 15:11 [PATCH][PING] Inline C99 math functions Wilco Dijkstra
2015-07-13 16:56 ` Carlos O'Donell
2015-07-22 15:16 ` Joseph Myers
@ 2015-11-04 15:04 ` Andreas Schwab
2015-11-06 14:29 ` Wilco Dijkstra
2 siblings, 1 reply; 8+ messages in thread
From: Andreas Schwab @ 2015-11-04 15:04 UTC (permalink / raw)
To: Wilco Dijkstra; +Cc: 'GNU C Library'
"Wilco Dijkstra" <wdijkstr@arm.com> writes:
>> 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?
FAIL: elf/check-localplt
$ cat elf/check-localplt.out
Missing required PLT reference: libm.so: __signbitl
Missing required PLT reference: libc.so: __signbitl
Missing required PLT reference: libm.so: __signbitf
Missing required PLT reference: libm.so: __signbit
Missing required PLT reference: libc.so: __signbit
Andreas.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH][PING] Inline C99 math functions
2015-11-04 15:04 ` Andreas Schwab
@ 2015-11-06 14:29 ` Wilco Dijkstra
2015-11-09 9:21 ` Andreas Schwab
0 siblings, 1 reply; 8+ messages in thread
From: Wilco Dijkstra @ 2015-11-06 14:29 UTC (permalink / raw)
To: 'Andreas Schwab'; +Cc: 'GNU C Library'
Andreas Schwab wrote:
> "Wilco Dijkstra" <wdijkstr@arm.com> writes:
>
> >> 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?
>
> FAIL: elf/check-localplt
> $ cat elf/check-localplt.out
> Missing required PLT reference: libm.so: __signbitl Missing required PLT
> reference: libc.so: __signbitl Missing required PLT reference: libm.so:
> __signbitf Missing required PLT reference: libm.so: __signbit Missing
> required PLT reference: libc.so: __signbit
I'm not exactly sure what this means - should we now remove the __signbit*
from
all the localplt.data files now that signbit is always inlined on all
targets?
Wilco
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH][PING] Inline C99 math functions
2015-11-06 14:29 ` Wilco Dijkstra
@ 2015-11-09 9:21 ` Andreas Schwab
0 siblings, 0 replies; 8+ messages in thread
From: Andreas Schwab @ 2015-11-09 9:21 UTC (permalink / raw)
To: Wilco Dijkstra; +Cc: 'GNU C Library'
"Wilco Dijkstra" <Wilco.Dijkstra@arm.com> writes:
> I'm not exactly sure what this means - should we now remove the __signbit*
> from
> all the localplt.data files now that signbit is always inlined on all
> targets?
Yes, that is correct. The references are not supposed to reappear.
Andreas.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-11-09 9:21 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-13 15:11 [PATCH][PING] Inline C99 math functions Wilco Dijkstra
2015-07-13 16:56 ` Carlos O'Donell
2015-07-22 15:16 ` Joseph Myers
2015-07-22 15:58 ` Wilco Dijkstra
2015-07-22 19:47 ` Joseph Myers
2015-11-04 15:04 ` Andreas Schwab
2015-11-06 14:29 ` Wilco Dijkstra
2015-11-09 9:21 ` Andreas Schwab
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).