Bypass math wrappers with -fno-math-errno

Message ID HE1PR0801MB205886FEFE1BC4014C192EDF83560@HE1PR0801MB2058.eurprd08.prod.outlook.com
State New, archived
Headers

Commit Message

Wilco Dijkstra Nov. 8, 2017, 2:50 p.m. UTC
  Several math functions have wrappers which handle errno.  They are
bypassed if __FINITE_MATH_ONLY__ is set.  Given the wrappers impose
a significant overhead, bypassing them is worthwhile (and it is best
to remove them completely in the future).

Since the only purpose of the wrappers is to set errno, we can bypass
them if __NO_MATH_ERRNO__ is set as well.  There are a few target specific
functions which define finite variants of math functions and those do not
set exceptions.  In those cases it would be incorrect to bypass the wrapper
unless __FINITE_MATH_ONLY__ is set.  To support this add a define that
allows targets to disable this.  It's currently set for x86 and x86_64 
due to defining several x87 math functions which only support finite math.

OK for commit?

ChangeLog:
2017-11-08  Wilco Dijkstra  <wdijkstr@arm.com>

	* math/math.h (math_errhandling): Bypass errno wrappers if
	__NO_MATH_ERRNO__ is defined, unless HAS_FINITE_MATH_FUNCTIONS is defined.
	* sysdeps/x86/fpu/bits/mathinline.h (HAS_FINITE_MATH_FUNCTIONS): Define.
--
  

Comments

Joseph Myers Nov. 8, 2017, 4:55 p.m. UTC | #1
On Wed, 8 Nov 2017, Wilco Dijkstra wrote:

> Several math functions have wrappers which handle errno.  They are
> bypassed if __FINITE_MATH_ONLY__ is set.  Given the wrappers impose
> a significant overhead, bypassing them is worthwhile (and it is best
> to remove them completely in the future).
> 
> Since the only purpose of the wrappers is to set errno, we can bypass
> them if __NO_MATH_ERRNO__ is set as well.  There are a few target specific
> functions which define finite variants of math functions and those do not
> set exceptions.  In those cases it would be incorrect to bypass the wrapper
> unless __FINITE_MATH_ONLY__ is set.  To support this add a define that
> allows targets to disable this.  It's currently set for x86 and x86_64 
> due to defining several x87 math functions which only support finite math.

I think this sort of ABI change needs a much more detailed analysis and 
proposal seeking consensus before any patch is proposed based on the 
results of such discussion.

That is, in exactly what circumstances are _finite functions different 
beyond just not using wrappers?  And, in what circumstances would an 
implementation that genuinely relies on -ffinite-math-only be more 
efficient than one that just avoids wrappers (and in such circumstances, 
should be add such an implementation or consider it out of scope)?  
Because this is an ABI change that can only be made, but not reversed: if 
we make such a change and then wish to add a genuinely -ffinite-math-only 
function version in future, it needs to use a different name such as 
_really_finite so that it isn't used by any binaries that were built with 
2.27 with -fno-math-errno and so rely on _finite meaning only 
no-math-errno.

HAS_FINITE_MATH_FUNCTIONS is in the user namespace and so inappropriate 
for installed headers.  bits/mathinline.h is included only for 
__USE_EXTERN_INLINES and so cannot be used for defining macros tested in 
code that is not conditional on __USE_EXTERN_INLINES.
  
Wilco Dijkstra Nov. 20, 2017, 3:32 p.m. UTC | #2
Joseph Myers wrote:
> On Wed, 8 Nov 2017, Wilco Dijkstra wrote:
>
> > Several math functions have wrappers which handle errno.  They are
> > bypassed if __FINITE_MATH_ONLY__ is set.  Given the wrappers impose
> > a significant overhead, bypassing them is worthwhile (and it is best
> > to remove them completely in the future).
> > 
> > Since the only purpose of the wrappers is to set errno, we can bypass
> > them if __NO_MATH_ERRNO__ is set as well.  There are a few target specific
> > functions which define finite variants of math functions and those do not
> > set exceptions.  In those cases it would be incorrect to bypass the wrapper
> > unless __FINITE_MATH_ONLY__ is set.  To support this add a define that
> > allows targets to disable this.  It's currently set for x86 and x86_64 
> > due to defining several x87 math functions which only support finite math.
> 
> I think this sort of ABI change needs a much more detailed analysis and 
> proposal seeking consensus before any patch is proposed based on the 
> results of such discussion.

Sure - I just wanted to propose something concrete. The alternative would be
to remove the wrappers like we've done for a few functions so far, but that is
a fair amount of work (although I guess we could remove the finite-math.h
header once the commonly used functions had their wrappers merged).

> That is, in exactly what circumstances are _finite functions different 
> beyond just not using wrappers?  And, in what circumstances would an 
> implementation that genuinely relies on -ffinite-math-only be more 
> efficient than one that just avoids wrappers (and in such circumstances, 
> should be add such an implementation or consider it out of scope)?

My feeling is that bypassing/removing the wrappers gives the most benefit.
You always need to check some special cases, and checks for infinity/NaN 
can often be merged with those. Adding fast math variants with a larger ULP
error would definitely be more useful than finite math variants.

> Because this is an ABI change that can only be made, but not reversed: if 
> we make such a change and then wish to add a genuinely -ffinite-math-only 
> function version in future, it needs to use a different name such as 
> _really_finite so that it isn't used by any binaries that were built with 
> 2.27 with -fno-math-errno and so rely on _finite meaning only 
> no-math-errno.

Yes that is a good point.

Wilco
  

Patch

diff --git a/math/math.h b/math/math.h
index d46c26533eff01dfbcee8f59d022febdbdb8d769..26a985728ec86fa4302455a98bf3326afc901b37 100644
--- a/math/math.h
+++ b/math/math.h
@@ -709,8 +709,9 @@  iszero (__T __val)
 #endif
 
 /* Define special entry points to use when the compiler got told to
-   only expect finite results.  */
-#if defined __FINITE_MATH_ONLY__ && __FINITE_MATH_ONLY__ > 0
+   only expect finite results or errno is not used.  */
+#if (defined __FINITE_MATH_ONLY__ && __FINITE_MATH_ONLY__ > 0) \
+    || (defined __NO_MATH_ERRNO__ && !defined HAS_FINITE_MATH_FUNCTIONS)
 
 /* Include bits/math-finite.h for double.  */
 # define _Mdouble_ double
diff --git a/sysdeps/x86/fpu/bits/mathinline.h b/sysdeps/x86/fpu/bits/mathinline.h
index cf29751a2781d11cd835b2e8213443b52d84f10f..973a3603240a37459baf49f341c5c294377c3bf7 100644
--- a/sysdeps/x86/fpu/bits/mathinline.h
+++ b/sysdeps/x86/fpu/bits/mathinline.h
@@ -20,6 +20,10 @@ 
 # error "Never use <bits/mathinline.h> directly; include <math.h> instead."
 #endif
 
+/* Some math functions have finite variants which do not support exceptions,
+   so we may use the finite variant only with __FINITE_MATH_ONLY__.  */
+#define HAS_FINITE_MATH_FUNCTIONS 1
+
 #ifndef __extern_always_inline
 # define __MATH_INLINE __inline
 #else