Patchwork [v3,powerpc] No need to enter "Ignore Exceptions Mode"

login
register
mail settings
Submitter Paul Clarke
Date Sept. 27, 2019, 8:47 p.m.
Message ID <1569617237-7092-1-git-send-email-pc@us.ibm.com>
Download mbox | patch
Permalink /patch/34695/
State New
Headers show

Comments

Paul Clarke - Sept. 27, 2019, 8:47 p.m.
From: "Paul A. Clarke" <pc@us.ibm.com>

Since at least POWER8, there is no performance advantage to entering
"Ignore Exceptions Mode", and doing so conditionally requires the
conditional logic as well as a system call.  Make it a no-op for
uses within glibc.

2019-09-27  Paul A. Clarke  <pc@us.ibm.com>

	* sysdeps/powerpc/fpu/fenv_private.h:
	(__ENTER_NON_STOP): New.
	(__EXIT_NON_STOP): New.
	(libc_feholdexcept_setround_ppc): Use __ENTER_NON_STOP.
	(__libc_femergeenv_ppc): Use __ENTER_NON_STOP and __EXIT_NON_STOP.
	(libc_feholdsetround_noex_ppc_ctx): Use __EXIT_NON_STOP.
---
v2: This was a new patch (#2) in the series "Various FPSCR-related changes".
v3: I was a bit uncomfortable with the approach used in v1/v2, which had
  side effects, although these were expected to be innocuous.
  I reimplemented this patch to be implemented only for uses that are
  internal to glibc.  These are all cases where the mode is to be changed
  only temporarily.  So, the state will be consistent after the code
  block is complete -- no unexpected side effects.

 sysdeps/powerpc/fpu/fenv_private.h | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)
Paul E Murphy - Sept. 30, 2019, 2:32 p.m.
On 9/27/19 3:47 PM, Paul A. Clarke wrote:
> From: "Paul A. Clarke" <pc@us.ibm.com>
> 
> Since at least POWER8, there is no performance advantage to entering
> "Ignore Exceptions Mode", and doing so conditionally requires the
> conditional logic as well as a system call.  Make it a no-op for
> uses within glibc.
> 
> 2019-09-27  Paul A. Clarke  <pc@us.ibm.com>
> 
> 	* sysdeps/powerpc/fpu/fenv_private.h:
> 	(__ENTER_NON_STOP): New.
> 	(__EXIT_NON_STOP): New.
> 	(libc_feholdexcept_setround_ppc): Use __ENTER_NON_STOP.
> 	(__libc_femergeenv_ppc): Use __ENTER_NON_STOP and __EXIT_NON_STOP.
> 	(libc_feholdsetround_noex_ppc_ctx): Use __EXIT_NON_STOP.
> ---
> v2: This was a new patch (#2) in the series "Various FPSCR-related changes".
> v3: I was a bit uncomfortable with the approach used in v1/v2, which had
>    side effects, although these were expected to be innocuous.
>    I reimplemented this patch to be implemented only for uses that are
>    internal to glibc.  These are all cases where the mode is to be changed
>    only temporarily.  So, the state will be consistent after the code
>    block is complete -- no unexpected side effects.
> 
>   sysdeps/powerpc/fpu/fenv_private.h | 17 +++++++++++++----
>   1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/sysdeps/powerpc/fpu/fenv_private.h b/sysdeps/powerpc/fpu/fenv_private.h
> index 5eedc3b..d186575 100644
> --- a/sysdeps/powerpc/fpu/fenv_private.h
> +++ b/sysdeps/powerpc/fpu/fenv_private.h
> @@ -23,6 +23,15 @@
>   #include <fenv_libc.h>
>   #include <fpu_control.h>
>   
> +#ifdef _ARCH_PWR8
> +/* There is no performance advantage to non-stop mode.  */
> +#define __ENTER_NON_STOP(old, new) do {} while (old * new * 0 != 0)
> +#define __EXIT_NON_STOP(old, new) do {} while (old * new * 0 != 0)
> +#else
> +#define __ENTER_NON_STOP __TEST_AND_ENTER_NON_STOP
> +#define __EXIT_NON_STOP __TEST_AND_EXIT_NON_STOP
The naming of the new macros seem misleading to me. Maybe
__TEST_AND_*_NON_STOP_P7?

> +#endif
> +
>   static __always_inline void
>   libc_feholdexcept_setround_ppc (fenv_t *envp, int r)
>   {
> @@ -30,7 +39,7 @@ libc_feholdexcept_setround_ppc (fenv_t *envp, int r)
>   
>     old.fenv = *envp = fegetenv_register ();
>   
> -  __TEST_AND_ENTER_NON_STOP (old.l, 0ULL);
> +  __ENTER_NON_STOP (old.l, 0ULL);
>   
>     /* Clear everything and set the rounding mode.  */
>     new.l = r;
> @@ -49,8 +58,8 @@ __libc_femergeenv_ppc (const fenv_t *envp, unsigned long long old_mask,
>     /* Merge bits while masking unwanted bits from new and old env.  */
>     new.l = (old.l & old_mask) | (new.l & new_mask);
>   
> -  __TEST_AND_EXIT_NON_STOP (old.l, new.l);
> -  __TEST_AND_ENTER_NON_STOP (old.l, new.l);
> +  __EXIT_NON_STOP (old.l, new.l);
> +  __ENTER_NON_STOP (old.l, new.l);
>   
>     /* If requesting to keep status, replace control, and merge exceptions,
>        and exceptions haven't changed, we can just set new control instead
> @@ -141,7 +150,7 @@ libc_feholdsetround_noex_ppc_ctx (struct rm_ctx *ctx, int r)
>     ctx->env = old.fenv;
>     if (__glibc_unlikely (new.l != old.l))
>       {
> -      __TEST_AND_ENTER_NON_STOP (old.l, 0ULL);
> +      __ENTER_NON_STOP (old.l, 0ULL);
>         fesetenv_control (new.fenv);
>         ctx->updated_status = true;
>       }
>

OK.

Patch

diff --git a/sysdeps/powerpc/fpu/fenv_private.h b/sysdeps/powerpc/fpu/fenv_private.h
index 5eedc3b..d186575 100644
--- a/sysdeps/powerpc/fpu/fenv_private.h
+++ b/sysdeps/powerpc/fpu/fenv_private.h
@@ -23,6 +23,15 @@ 
 #include <fenv_libc.h>
 #include <fpu_control.h>
 
+#ifdef _ARCH_PWR8
+/* There is no performance advantage to non-stop mode.  */
+#define __ENTER_NON_STOP(old, new) do {} while (old * new * 0 != 0)
+#define __EXIT_NON_STOP(old, new) do {} while (old * new * 0 != 0)
+#else
+#define __ENTER_NON_STOP __TEST_AND_ENTER_NON_STOP
+#define __EXIT_NON_STOP __TEST_AND_EXIT_NON_STOP
+#endif
+
 static __always_inline void
 libc_feholdexcept_setround_ppc (fenv_t *envp, int r)
 {
@@ -30,7 +39,7 @@  libc_feholdexcept_setround_ppc (fenv_t *envp, int r)
 
   old.fenv = *envp = fegetenv_register ();
 
-  __TEST_AND_ENTER_NON_STOP (old.l, 0ULL);
+  __ENTER_NON_STOP (old.l, 0ULL);
 
   /* Clear everything and set the rounding mode.  */
   new.l = r;
@@ -49,8 +58,8 @@  __libc_femergeenv_ppc (const fenv_t *envp, unsigned long long old_mask,
   /* Merge bits while masking unwanted bits from new and old env.  */
   new.l = (old.l & old_mask) | (new.l & new_mask);
 
-  __TEST_AND_EXIT_NON_STOP (old.l, new.l);
-  __TEST_AND_ENTER_NON_STOP (old.l, new.l);
+  __EXIT_NON_STOP (old.l, new.l);
+  __ENTER_NON_STOP (old.l, new.l);
 
   /* If requesting to keep status, replace control, and merge exceptions,
      and exceptions haven't changed, we can just set new control instead
@@ -141,7 +150,7 @@  libc_feholdsetround_noex_ppc_ctx (struct rm_ctx *ctx, int r)
   ctx->env = old.fenv;
   if (__glibc_unlikely (new.l != old.l))
     {
-      __TEST_AND_ENTER_NON_STOP (old.l, 0ULL);
+      __ENTER_NON_STOP (old.l, 0ULL);
       fesetenv_control (new.fenv);
       ctx->updated_status = true;
     }