This is the mail archive of the mailing list for the glibc project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v2 1/3] Cleanup __ieee754_sqrt(f/l)

Joseph Myers wrote:

> I think the general design is right, but I also think this really needs a 
> test run (as do subsequent patches moving calls to 
> use sqrt/sqrtf/sqrtl directly) because of the high risk of 
> architecture-specific breakage.

I tried, there was a failure on 32-bit x86 due to including the w_sqrt_compat.c.
That's fixed now, the only failures I get are elf/check-execstack on a few targets. 

> (Direct calling of sqrtf128 with similar redirects to those for sqrt / 
> sqrtf / sqrtl is also appropriate: GCC 8 has it as a built-in function so 
> can inline calls with -fno-math-errno on powerpc64le for POWER9.  So the 
> patch should include a sqrtf128 redirect if __HAVE_DISTINCT_FLOAT128.)

I can't get this to work, some targets don't like __float128, others don't like _Float128
despite __HAVE_DISTINCT_FLOAT128 being defined. I don't see any calls to
sqrtf128, so I think it's best to leave this for now. 

There are many calls to simple functions like floor, round etc that aren't currently
inlined at all, so inlining those seems like a more useful thing to do right now.
I've renamed the define to NO_MATH_REDIRECT to make this simple.

Latest version:

This patch series cleans up the many uses of  __ieee754_sqrt(f/l) in GLIBC.
The goal is to enable GCC to do the inlining, and if this fails call the
__ieee754_sqrt function.  This is done by internally declaring sqrt with asm
redirects.  The compat symbols and sqrt wrappers need to disable the redirect.
The redirect is also disabled if there are already redirects defined when
using -ffinite-math-only.

All math functions (but not math tests) are built with -fno-math-errno which
means GCC will typically inline sqrt as a single instruction.

This means targets are no longer forced to add a special inline for sqrt.

Passes buildmanyglibcs.

2017-12-21  Wilco Dijkstra  <>

        * include/math.h (sqrt): Declare with asm redirect.
        (sqrtf): Likewise.
        (sqrtl): Likewise.
        * Makeconfig: Add -fno-math-errno for libc/libm, but build tests
        with -fmath-errno.
        * math/w_sqrt_compat.c: Define NO_MATH_REDIRECT.
        * math/w_sqrt_template.c: Likewise.
        * math/w_sqrtf_compat.c: Likewise.
        * math/w_sqrtl_compat.c: Likewise.
        * sysdeps/i386/fpu/w_sqrt.c: Likewise.
        * sysdeps/i386/fpu/w_sqrt_compat.c: Likewise.

diff --git a/Makeconfig b/Makeconfig
index 34bed9790fac1fd0be9e16b7fe2fa6f5c479b32b..b9da1c137bc495a2f63064765225dc8878cde306 100644
--- a/Makeconfig
+++ b/Makeconfig
@@ -831,6 +831,9 @@ endif
 # disable any optimization that assume default rounding mode.
 +math-flags = -frounding-math
+# Build libc/libm using -fno-math-errno, but run testsuite with -fmath-errno.
++extra-math-flags = $(if $(filter nonlib testsuite,$(in-module)),-fmath-errno,-fno-math-errno)
 # We might want to compile with some stack-protection flag.
 ifneq ($(stack-protector),)
@@ -966,6 +969,7 @@ endif
 override CFLAGS	= -std=gnu11 -fgnu89-inline $(config-extra-cflags) \
 		  $(filter-out %frame-pointer,$(+cflags)) $(+gccwarn-c) \
+		  $(+extra-math-flags) \
 		  $(sysdep-CFLAGS) $(CFLAGS-$(suffix $@)) $(CFLAGS-$(<F)) \
 		  $(CFLAGS-$(@F)) $(tls-model) \
 		  $(foreach lib,$(libof-$(basename $(@F))) \
diff --git a/include/math.h b/include/math.h
index 7ee291fc972088a1409949326f7024e8e3fe5415..adac78f497b0787757c20c4b9ba52fc9e43a7629 100644
--- a/include/math.h
+++ b/include/math.h
@@ -54,5 +54,17 @@ libm_hidden_proto (__expf128)
 libm_hidden_proto (__expm1f128)
 # endif
+# if !(defined __FINITE_MATH_ONLY__ && __FINITE_MATH_ONLY__ > 0)
+/* Declare sqrt for use within GLIBC.  Sqrt will typically inline into a
+   single instruction.  Use an asm to avoid use of PLTs if it doesn't.  */
+float (sqrtf) (float) asm ("__ieee754_sqrtf");
+double (sqrt) (double) asm ("__ieee754_sqrt");
+#   ifndef __NO_LONG_DOUBLE_MATH
+long double (sqrtl) (long double) asm ("__ieee754_sqrtl");
+#   endif
+#  endif
+# endif
diff --git a/math/w_sqrt_compat.c b/math/w_sqrt_compat.c
index 3280d2fbb86af2aeaf6e686fd38579b209c901aa..1a8db2fdef87e57df2f7dda117ef3ee48581ca1d 100644
--- a/math/w_sqrt_compat.c
+++ b/math/w_sqrt_compat.c
@@ -16,6 +16,7 @@
    License along with the GNU C Library; if not, see
    <>.  */
 #include <math.h>
 #include <math_private.h>
 #include <math-svid-compat.h>
diff --git a/math/w_sqrt_template.c b/math/w_sqrt_template.c
index 5fae302382d10e9b05df2665c9cb05126cd62f02..f03f04751a9334fc7cdb8c954fbbcd845ac8670f 100644
--- a/math/w_sqrt_template.c
+++ b/math/w_sqrt_template.c
@@ -21,6 +21,7 @@
    for each floating-point type.  */
 # include <errno.h>
 # include <fenv.h>
 # include <math.h>
diff --git a/math/w_sqrtf_compat.c b/math/w_sqrtf_compat.c
index 6c8c7e3857c7dacf52de6b575a2386095688b5dc..fe7418dbdcdbdb571cb1b9743cf9b17eaa5c8cb2 100644
--- a/math/w_sqrtf_compat.c
+++ b/math/w_sqrtf_compat.c
@@ -16,6 +16,7 @@
    License along with the GNU C Library; if not, see
    <>.  */
 #include <math.h>
 #include <math_private.h>
 #include <math-svid-compat.h>
diff --git a/math/w_sqrtl_compat.c b/math/w_sqrtl_compat.c
index 0590f6d155fee27d41bfd42c1dbdf19c381ba1c8..3fda9cf991e430f259b9d26f15219181b3e6dcbb 100644
--- a/math/w_sqrtl_compat.c
+++ b/math/w_sqrtl_compat.c
@@ -16,6 +16,7 @@
    License along with the GNU C Library; if not, see
    <>.  */
 #include <math.h>
 #include <math_private.h>
 #include <math-svid-compat.h>
diff --git a/sysdeps/i386/fpu/w_sqrt.c b/sysdeps/i386/fpu/w_sqrt.c
index d37a5d55bf439cebdff05f05dd66f910d6d48c18..8bef04e68a7be3d21d89e6eb41dcfa2561f3f6a5 100644
--- a/sysdeps/i386/fpu/w_sqrt.c
+++ b/sysdeps/i386/fpu/w_sqrt.c
@@ -1,5 +1,6 @@
 /* The inline __ieee754_sqrt is not correctly rounding; it's OK for
    most internal uses in glibc, but not for sqrt itself.  */
 #define __ieee754_sqrt __avoid_ieee754_sqrt
 #include <math.h>
 #include <math_private.h>
diff --git a/sysdeps/i386/fpu/w_sqrt_compat.c b/sysdeps/i386/fpu/w_sqrt_compat.c
index ddd36d0964c2945579bc59b02f127af88384acca..dd485f4b88c7f5348abaab81a1308aeadc9594ec 100644
--- a/sysdeps/i386/fpu/w_sqrt_compat.c
+++ b/sysdeps/i386/fpu/w_sqrt_compat.c
@@ -1,5 +1,6 @@
 /* The inline __ieee754_sqrt is not correctly rounding; it's OK for
    most internal uses in glibc, but not for sqrt itself.  */
 #define __ieee754_sqrt __avoid_ieee754_sqrt
 #include <math.h>
 #include <math_private.h>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]