diff mbox series

[RFA] : i386: Use generic division to generate INVALID and DIVZERO exceptions

Message ID CAFULd4adYq8rXfTTj5eZ1S17jGK7ruitf0B9mYK6nF--1s8eaw@mail.gmail.com
State New
Headers show
Series [RFA] : i386: Use generic division to generate INVALID and DIVZERO exceptions | expand

Commit Message

Uros Bizjak April 19, 2020, 6:45 p.m. UTC 
This patch implements the idea from glibc, where generic division
operations instead of assembly are used where appropriate.

--commit--
i386: Use generic division to generate INVALID and DIVZERO exceptions

Introduce math_force_eval to evaluate generic division to generate
INVALID and DIVZERO exceptions.

libgcc/ChangeLog:

2020-04-19  Uroš Bizjak  <ubizjak@gmail.com>

    * config/i386/sfp-exceptions.c (math_force_eval): New define.
    (__sfp_handle_exceptions): Use math_force_eval to evaluete
    generic division to generate INVALID and DIVZERO exceptions.

libatomic/ChangeLog:

2020-04-19  Uroš Bizjak  <ubizjak@gmail.com>

    * config/x86/fenv.c (math_force_eval): New define.
    (__atomic_feraiseexcept): Use math_force_eval to evaluete
    generic division to generate INVALID and DIVZERO exceptions.

libgfortran/ChangeLog:

2020-04-19  Uroš Bizjak  <ubizjak@gmail.com>

    * config/fpu-387.h (_math_force_eval): New define.
    (local_feraiseexcept): Use math_force_eval to evaluete
    generic division to generate INVALID and DIVZERO exceptions.
--/commit--

The patch was bootstrapped and regression tested on x86_64-linux-gnu
{,-m32}. Also, as part of glibc's patch evaluation, I have extensively
tested it to check that it really generates only correct exceptions
for both, x87 and SSE math.

Additionally, the patch improves code for DIVZERO exception, as we can
now use FDIVRP mnemonic. This mnemonic has its own share of confusion
(see e.g. SYSV386_COMPAT macro definition) and its use in asm should
be avoided.
--/commit message--

The patch looks safe to me, but I'd like to ask RMs for approval.

Uros.

Comments

Richard Biener April 20, 2020, 6:30 a.m. UTC | #1 
On Sun, 19 Apr 2020, Uros Bizjak wrote:

> This patch implements the idea from glibc, where generic division
> operations instead of assembly are used where appropriate.
> 
> --commit--
> i386: Use generic division to generate INVALID and DIVZERO exceptions
> 
> Introduce math_force_eval to evaluate generic division to generate
> INVALID and DIVZERO exceptions.
> 
> libgcc/ChangeLog:
> 
> 2020-04-19  Uroš Bizjak  <ubizjak@gmail.com>
> 
>     * config/i386/sfp-exceptions.c (math_force_eval): New define.
>     (__sfp_handle_exceptions): Use math_force_eval to evaluete
>     generic division to generate INVALID and DIVZERO exceptions.
> 
> libatomic/ChangeLog:
> 
> 2020-04-19  Uroš Bizjak  <ubizjak@gmail.com>
> 
>     * config/x86/fenv.c (math_force_eval): New define.
>     (__atomic_feraiseexcept): Use math_force_eval to evaluete
>     generic division to generate INVALID and DIVZERO exceptions.
> 
> libgfortran/ChangeLog:
> 
> 2020-04-19  Uroš Bizjak  <ubizjak@gmail.com>
> 
>     * config/fpu-387.h (_math_force_eval): New define.
>     (local_feraiseexcept): Use math_force_eval to evaluete
>     generic division to generate INVALID and DIVZERO exceptions.
> --/commit--
> 
> The patch was bootstrapped and regression tested on x86_64-linux-gnu
> {,-m32}. Also, as part of glibc's patch evaluation, I have extensively
> tested it to check that it really generates only correct exceptions
> for both, x87 and SSE math.
> 
> Additionally, the patch improves code for DIVZERO exception, as we can
> now use FDIVRP mnemonic. This mnemonic has its own share of confusion
> (see e.g. SYSV386_COMPAT macro definition) and its use in asm should
> be avoided.
> --/commit message--
> 
> The patch looks safe to me, but I'd like to ask RMs for approval.

If it doesn't fix a regression can it wait for stage1 please?

Thanks,
Richard.
diff mbox series

Patch

diff --git a/libatomic/config/x86/fenv.c b/libatomic/config/x86/fenv.c
index d7b1bbe5ea1..be1db42245c 100644
--- a/libatomic/config/x86/fenv.c
+++ b/libatomic/config/x86/fenv.c
@@ -47,6 +47,12 @@  struct fenv
   unsigned short int __unused5;
 };
 
+#ifdef __SSE_MATH__
+# define math_force_eval(x) asm volatile ("" : : "x" (x));
+#else
+# define math_force_eval(x) asm volatile ("" : : "f" (x));
+#endif
+
 /* Raise the supported floating-point exceptions from EXCEPTS.  Other
    bits in EXCEPTS are ignored.  */
 
@@ -56,12 +62,7 @@  __atomic_feraiseexcept (int excepts)
   if (excepts & FE_INVALID)
     {
       float f = 0.0f;
-#ifdef __SSE_MATH__
-      asm volatile ("%vdivss\t{%0, %d0|%d0, %0}" : "+x" (f));
-#else
-      asm volatile ("fdiv\t{%y0, %0|%0, %y0}" : "+t" (f));
-      /* No need for fwait, exception is triggered by emitted fstp.  */
-#endif
+      math_force_eval (f / f);
     }
   if (excepts & FE_DENORM)
     {
@@ -74,12 +75,7 @@  __atomic_feraiseexcept (int excepts)
   if (excepts & FE_DIVBYZERO)
     {
       float f = 1.0f, g = 0.0f;
-#ifdef __SSE_MATH__
-      asm volatile ("%vdivss\t{%1, %d0|%d0, %1}" : "+x" (f) : "xm" (g));
-#else
-      asm volatile ("fdivs\t%1" : "+t" (f) : "m" (g));
-      /* No need for fwait, exception is triggered by emitted fstp.  */
-#endif
+      math_force_eval (f / g);
     }
   if (excepts & FE_OVERFLOW)
     {
diff --git a/libgcc/config/i386/sfp-exceptions.c b/libgcc/config/i386/sfp-exceptions.c
index 31a24ced704..57047ab1d07 100644
--- a/libgcc/config/i386/sfp-exceptions.c
+++ b/libgcc/config/i386/sfp-exceptions.c
@@ -41,18 +41,19 @@  struct fenv
   unsigned short int __unused5;
 };
 
+#ifdef __SSE_MATH__
+# define math_force_eval(x) asm volatile ("" : : "x" (x));
+#else
+# define math_force_eval(x) asm volatile ("" : : "f" (x));
+#endif
+
 void
 __sfp_handle_exceptions (int _fex)
 {
   if (_fex & FP_EX_INVALID)
     {
       float f = 0.0f;
-#ifdef __SSE_MATH__
-      asm volatile ("%vdivss\t{%0, %d0|%d0, %0}" : "+x" (f));
-#else
-      asm volatile ("fdiv\t{%y0, %0|%0, %y0}" : "+t" (f));
-      /* No need for fwait, exception is triggered by emitted fstp.  */
-#endif
+      math_force_eval (f / f);
     }
   if (_fex & FP_EX_DENORM)
     {
@@ -65,12 +66,7 @@  __sfp_handle_exceptions (int _fex)
   if (_fex & FP_EX_DIVZERO)
     {
       float f = 1.0f, g = 0.0f;
-#ifdef __SSE_MATH__
-      asm volatile ("%vdivss\t{%1, %d0|%d0, %1}" : "+x" (f) : "xm" (g));
-#else
-      asm volatile ("fdivs\t%1" : "+t" (f) : "m" (g));
-      /* No need for fwait, exception is triggered by emitted fstp.  */
-#endif
+      math_force_eval (f / g);
     }
   if (_fex & FP_EX_OVERFLOW)
     {
diff --git a/libgfortran/config/fpu-387.h b/libgfortran/config/fpu-387.h
index 13be2045b72..08c994f69da 100644
--- a/libgfortran/config/fpu-387.h
+++ b/libgfortran/config/fpu-387.h
@@ -91,6 +91,11 @@  my_fenv_t;
 _Static_assert (sizeof(my_fenv_t) <= (size_t) GFC_FPE_STATE_BUFFER_SIZE,
 		"GFC_FPE_STATE_BUFFER_SIZE is too small");
 
+#ifdef __SSE_MATH__
+# define _math_force_eval(x) __asm__ __volatile__ ("" : : "x" (x));
+#else
+# define _math_force_eval(x) __asm__ __volatile__ ("" : : "f" (x));
+#endif
 
 /* Raise the supported floating-point exceptions from EXCEPTS.  Other
    bits in EXCEPTS are ignored.  Code originally borrowed from
@@ -102,12 +107,7 @@  local_feraiseexcept (int excepts)
   if (excepts & _FPU_MASK_IM)
     {
       float f = 0.0f;
-#ifdef __SSE_MATH__
-      __asm__ __volatile__ ("%vdivss\t{%0, %d0|%d0, %0}" : "+x" (f));
-#else
-      __asm__ __volatile__ ("fdiv\t{%y0, %0|%0, %y0}" : "+t" (f));
-      /* No need for fwait, exception is triggered by emitted fstp.  */
-#endif
+      _math_force_eval (f / f);
     }
   if (excepts & _FPU_MASK_DM)
     {
@@ -120,12 +120,7 @@  local_feraiseexcept (int excepts)
   if (excepts & _FPU_MASK_ZM)
     {
       float f = 1.0f, g = 0.0f;
-#ifdef __SSE_MATH__
-      __asm__ __volatile__ ("%vdivss\t{%1, %d0|%d0, %1}" : "+x" (f) : "xm" (g));
-#else
-      __asm__ __volatile__ ("fdivs\t%1" : "+t" (f) : "m" (g));
-      /* No need for fwait, exception is triggered by emitted fstp.  */
-#endif
+      _math_force_eval (f / g);
     }
   if (excepts & _FPU_MASK_OM)
     {