This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
[PATCH] Only set rounding mode in SET_RESTORE_ROUND_53BIT
- From: Siddhesh Poyarekar <siddhesh at redhat dot com>
- To: libc-alpha at sourceware dot org
- Cc: Richard Henderson <rth at twiddle dot net>, carlos at redhat dot com
- Date: Thu, 30 May 2013 14:29:17 +0530
- Subject: [PATCH] Only set rounding mode in SET_RESTORE_ROUND_53BIT
- References: <20130529140319 dot GL2145 at spoyarek dot pnq dot redhat dot com>
On Wed, May 29, 2013 at 07:33:19PM +0530, Siddhesh Poyarekar wrote:
> I'm trying to understand the code that sets and restores the floating
> point state to try and eek out some performance from it. I haven't
> been able to figure out the need for SET_RESTORE_ROUND_53BIT vs
> SET_RESTORE_ROUND_53 for x86_64 - I can see that it's needed for x87:
>
> SET_RESTORE_ROUND (used in exp, log) only sets and restores the
> rounding mode and does not touch the exception masks. This was
> holdexcept_setround, i.e. the same implementation as
> SET_RESTORE_ROUND_53BIT before, but I assume the clearing of exception
> mask, etc. was removed because it could result in delay in raising
> floating point exceptions resulting from some operations within these
> functions if exceptions are masked.
>
> SET_RESTORE_ROUND_53BIT (used in sin/cos) sets the rounding mode,
> exception mask and clears all exception flags. Why can't we simply
> get away with setting and restoring just the rounding mode like in
> exp, log, etc? That opens up a good opportunity for optimization
> where I could avoid most of the set/restore code in the default case.
> sin and cos run almost twice as fast in some cases.
So to put all this in code, the below patch is what I intend to do.
Instead of modifying exception flags, SET_RESTORE_ROUND_53BIT only
sets rounding mode and floating point mode (in x87) and restores it on
exit from the lexical block. I tested x86_64 as well as i386 with
--disable-sse to verify that this does not cause any regressions in
the testsuite. Would this change be correct?
Siddhesh
* sysdeps/generic/math_private.h
(libc_feholdexcept_setround_53bit): Replace with
libc_feholdsetround_53bit.
(libc_feupdateenv_53bit): Replace with
libc_feresetround_53bit.
(SET_RESTORE_ROUND_53BIT): Adjust.
diff --git a/sysdeps/generic/math_private.h b/sysdeps/generic/math_private.h
index 9d6ecad..e98360d 100644
--- a/sysdeps/generic/math_private.h
+++ b/sysdeps/generic/math_private.h
@@ -446,8 +446,8 @@ default_libc_feholdexcept_setround (fenv_t *e, int r)
# define libc_feholdexcept_setroundl default_libc_feholdexcept_setround
#endif
-#ifndef libc_feholdexcept_setround_53bit
-# define libc_feholdexcept_setround_53bit libc_feholdexcept_setround
+#ifndef libc_feholdsetround_53bit
+# define libc_feholdsetround_53bit libc_feholdsetround
#endif
#ifndef libc_fetestexcept
@@ -492,8 +492,8 @@ default_libc_feupdateenv (fenv_t *e)
# define libc_feupdateenvl default_libc_feupdateenv
#endif
-#ifndef libc_feupdateenv_53bit
-# define libc_feupdateenv_53bit libc_feupdateenv
+#ifndef libc_feresetround_53bit
+# define libc_feresetround_53bit libc_feresetround
#endif
static __always_inline int
@@ -580,8 +580,8 @@ default_libc_feupdateenv_test (fenv_t *e, int ex)
/* Like SET_RESTORE_ROUND, but also set rounding precision to 53 bits. */
#define SET_RESTORE_ROUND_53BIT(RM) \
- fenv_t __libc_save_rm __attribute__((cleanup(libc_feupdateenv_53bit))); \
- libc_feholdexcept_setround_53bit (&__libc_save_rm, (RM))
+ fenv_t __libc_save_rm __attribute__((cleanup(libc_feresetround_53bit))); \
+ libc_feholdsetround_53bit (&__libc_save_rm, (RM))
#define __nan(str) \
(__builtin_constant_p (str) && str[0] == '\0' ? NAN : __nan (str))