[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [patches] e500 prctl usage



On Fri, Aug 31, 2007 at 11:56:47AM -0400, Daniel Jacobowitz wrote:
> Hi Kumar,
> 
> I'm looking at another batch of e500 fenv-related failures, similar to
> my last patch.

I have no idea if the IEEE emulation kernel patch I'm looking at is
the most recent - so some of this may already be fixed.  I'm not sure
exactly where the kernel patch came from, either.  Problems I have
noticed with it so far:

- The initial state of PR_GET_FPEXC is a classic FP state, not an
e500 state.  I made glibc detect this and treat that as
PR_FP_EXC_SW_ENABLE with no exceptions enabled - ugly.  Shouldn't
the default if CONFIG_SPE be to use fpexc_mode for enabled
exceptions?

- When handling an exception, the sticky bit in SPEFSCR is only set
if the exception is enabled via prctl.  If the exception happened
then the sticky bit should be set even if the exception is disabled,
so that fetestexcept will see it.

- When enabling SPE for the first time in a task, the kernel sets
SPEFSCR to 0x3c instead of 0x7c.  Invalid operation is still disabled.

- SIGFPE never gets raised even when it should.  spedata_handler
returns -EINVAL, -ENOSYS, or -EFAULT if an error occurs, but
always zero if it could successfully decode the instruction.
SPEFloatingPointException checks for -EFAULT and -EINVAL
(ignored -ENOSYS), 0 (immediate return), and only raises SIGFPE
on some other unspecified return value which will never happen.

- Also, I am not sure this is a kernel bug.  But observe:

0x10015bd0      183     {
3: /x fetestexcept (-1) = 0x3e303c
1: x/i $pc  0x10015bd0 <_IO_vfprintf_internal+20>:      evstdd r15,0(r11)
(gdb) stepi
0x10015bd4      183     {
3: /x fetestexcept (-1) = 0x3c
1: x/i $pc  0x10015bd4 <_IO_vfprintf_internal+24>:      mr      r15,r5

That call to fetestexcept in my tree is basically just mfspefscr.
Somehow stepping over a store has cleared all the sticky bits.  It's a
store to the stack even so there shouldn't be a page fault handler
involved.  The ISA documentation says that the hardware will never
clear sticky bits, so my guess is that the kernel has done so;
possibly a problem with context switching SPEFSCR in my kernel?

I have attached my work-in-progress patch.  This fixes all test-fenv
failures except for those associated with the above bugs (it includes
workarounds for the first two).  I don't want to commit it until
I can test it a little better, though.

-- 
Daniel Jacobowitz
CodeSourcery

Index: sysdeps/powerpc/powerpc32/e500/spe-raise.c
===================================================================
--- sysdeps/powerpc/powerpc32/e500/spe-raise.c	(revision 180364)
+++ sysdeps/powerpc/powerpc32/e500/spe-raise.c	(working copy)
@@ -25,6 +25,10 @@ __FERAISEEXCEPT_INTERNAL (int excepts)
 {
   unsigned long f;
 
+  /* NOTE: This really shouldn't be necessary!  The below bits should
+     take care of it.  But in the latest kernel I have with e500 fixups,
+     SPEFSCR sticky bits are only set if the exception is enabled.  They
+     should be set regardless.  */
   f = fegetenv_register ();
   f |= (excepts & FE_ALL_EXCEPT);
   fesetenv_register (f);
Index: sysdeps/powerpc/powerpc32/e500/fpu/fegetenv.c
===================================================================
--- sysdeps/powerpc/powerpc32/e500/fpu/fegetenv.c	(revision 180364)
+++ sysdeps/powerpc/powerpc32/e500/fpu/fegetenv.c	(working copy)
@@ -31,6 +31,12 @@ __fegetenv (fenv_t *envp)
 
   INTERNAL_SYSCALL (prctl, err, 2, PR_GET_FPEXC, &u.l[0]);
   u.l[1] = fegetenv_register ();
+
+  /* Make sure the mandatory prctl flag is set.  Otherwise this may
+     be stray classic PowerPC FP state.  */
+  if ((u.l[0] & PR_FP_EXC_SW_ENABLE) == 0)
+    u.l[0] = PR_FP_EXC_SW_ENABLE;
+
   *envp = u.fenv;
 
   /* Success.  */
Index: sysdeps/powerpc/powerpc32/e500/fpu/fesetenv.c
===================================================================
--- sysdeps/powerpc/powerpc32/e500/fpu/fesetenv.c	(revision 180364)
+++ sysdeps/powerpc/powerpc32/e500/fpu/fesetenv.c	(working copy)
@@ -29,7 +29,7 @@ __fesetenv (const fenv_t *envp)
   INTERNAL_SYSCALL_DECL (err);
 
   u.fenv = *envp;
-  INTERNAL_SYSCALL (prctl, err, 2, PR_SET_FPEXC, &u.l[0]);
+  INTERNAL_SYSCALL (prctl, err, 2, PR_SET_FPEXC, u.l[0]);
   fesetenv_register (u.l[1]);
 
   /* Success.  */
Index: sysdeps/powerpc/powerpc32/e500/fpu/fenv_const.c
===================================================================
--- sysdeps/powerpc/powerpc32/e500/fpu/fenv_const.c	(revision 180364)
+++ sysdeps/powerpc/powerpc32/e500/fpu/fenv_const.c	(working copy)
@@ -18,10 +18,14 @@
    Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
    02111-1307 USA.  */
 
-/* If the default argument is used we use this value.  */
+/* If the default argument is used we use this value.  Round to nearest
+   and the mandatory PR_FP_EXC_SW_ENABLE flag.  All exceptions are
+   enabled in the SPEFSCR; the kernel will fix up some cases and signal
+   (or not signal) others.  */
 const unsigned long long __fe_dfl_env __attribute__ ((aligned (8))) =
-0x0ULL;
+0x800000007cULL;
 
-/* Floating-point environment where none of the exceptions are masked.  */
+/* Floating-point environment where none of the exceptions are masked.  Same
+   as above, plus PR_FP_EXC_DIV et cetera.  */
 const unsigned long long __fe_enabled_env __attribute__ ((aligned (8))) =
-0xF00000000ULL;
+0x1f00800000007cULL;
Index: sysdeps/powerpc/powerpc32/e500/fpu/fedisblxcpt.c
===================================================================
--- sysdeps/powerpc/powerpc32/e500/fpu/fedisblxcpt.c	(revision 180364)
+++ sysdeps/powerpc/powerpc32/e500/fpu/fedisblxcpt.c	(working copy)
@@ -30,6 +30,11 @@ fedisableexcept (int excepts)
 
   INTERNAL_SYSCALL (prctl, err, 2, PR_GET_FPEXC, &pflags);
 
+  /* Make sure the mandatory prctl flag is set.  Otherwise this may
+     be stray classic PowerPC FP state.  */
+  if ((pflags & PR_FP_EXC_SW_ENABLE) == 0)
+     pflags = PR_FP_EXC_SW_ENABLE;
+
   /* Save old enable bits.  */
   if (pflags & PR_FP_EXC_OVF) 
     result |= FE_OVERFLOW;
Index: sysdeps/powerpc/powerpc32/e500/fpu/fegetexcept.c
===================================================================
--- sysdeps/powerpc/powerpc32/e500/fpu/fegetexcept.c	(revision 180364)
+++ sysdeps/powerpc/powerpc32/e500/fpu/fegetexcept.c	(working copy)
@@ -19,13 +19,35 @@
    02111-1307 USA.  */
 
 #include <fenv_libc.h>
+#include <sysdep.h>
+#include <sys/prctl.h>
 
 int
 fegetexcept (void)
 {
-  unsigned long fpescr;
+  unsigned int result = 0, pflags, r;
+  INTERNAL_SYSCALL_DECL (err);
 
-  fpescr = fegetenv_register ();
+  INTERNAL_SYSCALL (prctl, err, 2, PR_GET_FPEXC, &pflags);
+  if (INTERNAL_SYSCALL_ERROR_P (r, err))
+    return -1;
 
-  return fpescr & FE_ALL_EXCEPT;
+  /* Make sure the mandatory prctl flag is set.  Otherwise this may
+     be stray classic PowerPC FP state.  */
+  if ((pflags & PR_FP_EXC_SW_ENABLE) == 0)
+     pflags = PR_FP_EXC_SW_ENABLE;
+
+  /* Check the enable bits.  */
+  if (pflags & PR_FP_EXC_OVF) 
+    result |= FE_OVERFLOW;
+  if (pflags & PR_FP_EXC_UND) 
+    result |= FE_UNDERFLOW;
+  if (pflags & PR_FP_EXC_INV) 
+    result |= FE_INVALID;
+  if (pflags & PR_FP_EXC_DIV) 
+    result |= FE_DIVBYZERO;
+  if (pflags & PR_FP_EXC_RES) 
+    result |= FE_INEXACT;
+
+  return result;
 }
Index: sysdeps/powerpc/powerpc32/e500/fpu/feholdexcpt.c
===================================================================
--- sysdeps/powerpc/powerpc32/e500/fpu/feholdexcpt.c	(revision 180364)
+++ sysdeps/powerpc/powerpc32/e500/fpu/feholdexcpt.c	(working copy)
@@ -28,16 +28,22 @@ feholdexcept (fenv_t *envp)
   fenv_union_t u;
   INTERNAL_SYSCALL_DECL (err);
 
-
   /* Get the current state.  */
   INTERNAL_SYSCALL (prctl, err, 2, PR_GET_FPEXC, &u.l[0]);
   u.l[1] = fegetenv_register ();
+
+  /* Make sure the mandatory prctl flag is set.  Otherwise this may
+     be stray classic PowerPC FP state.  */
+  if ((u.l[0] & PR_FP_EXC_SW_ENABLE) == 0)
+    u.l[0] = PR_FP_EXC_SW_ENABLE;
+
   *envp = u.fenv;
 
-  /* Clear everything except for the rounding mode.  */
-  u.l[1] &= 3;
+  /* Clear any set exceptions.  */
+  u.l[1] &= ~FE_ALL_EXCEPT;
 
   /* Put the new state in effect.  */
+  INTERNAL_SYSCALL (prctl, err, 2, PR_SET_FPEXC, PR_FP_EXC_SW_ENABLE);
   fesetenv_register (u.l[1]);
 
   return 0;
Index: sysdeps/powerpc/powerpc32/e500/fpu/feenablxcpt.c
===================================================================
--- sysdeps/powerpc/powerpc32/e500/fpu/feenablxcpt.c	(revision 180364)
+++ sysdeps/powerpc/powerpc32/e500/fpu/feenablxcpt.c	(working copy)
@@ -30,6 +30,11 @@ feenableexcept (int excepts)
 
   INTERNAL_SYSCALL (prctl, err, 2, PR_GET_FPEXC, &pflags);
 
+  /* Make sure the mandatory prctl flag is set.  Otherwise this may
+     be stray classic PowerPC FP state.  */
+  if ((pflags & PR_FP_EXC_SW_ENABLE) == 0)
+     pflags = PR_FP_EXC_SW_ENABLE;
+
   /* Save old enable bits.  */
   if (pflags & PR_FP_EXC_OVF) 
     result |= FE_OVERFLOW;