fenv.h fixes for softfloat

[David Schultz]

The patch below should make the fenv.h functions work on ARM chips
without an FPU.  (Hardware FPU support is missing anyway, and
what's in fenv.h right now is probably wrong.)

It would be great if someone could test this, because I don't have
the hardware.  The regression tests in tools/regression/lib/msun,
particularly test-fenv.c, should provide some idea of what's
working and what isn't.

Index: msun/arm/fenv.h
===================================================================
--- msun/arm/fenv.h (revision 229262)
+++ msun/arm/fenv.h (working copy)
@@ -64,13 +64,25 @@
 #define _FPUSW_SHIFT 16
 #define _ENABLE_MASK (FE_ALL_EXCEPT << _FPUSW_SHIFT)
 
-#ifdef ARM_HARD_FLOAT
+#ifndef ARM_HARD_FLOAT
+/*
+ * The following macros map between the softfloat emulator's flags and
+ * the hardware's FPSR.  The hardware this file was written for doesn't
+ * have rounding control bits, so we stick those in the system ID byte.
+ */
+#define __set_env(env, flags, mask, rnd) env = ((flags) \
+ | (mask)<<_FPUSW_SHIFT \
+ | (rnd) << 24)
+#define __env_flags(env) ((env) & FE_ALL_EXCEPT)
+#define __env_mask(env) (((env) >> _FPUSW_SHIFT) \
+ & FE_ALL_EXCEPT)
+#define __env_round(env) (((env) >> 24) & _ROUND_MASK)
+#include <fenv-softfloat.h>
+
+#else /* ARM_HARD_FLOAT */
+
 #define __rfs(__fpsr) __asm __volatile("rfs %0" : "=r" (*(__fpsr)))
 #define __wfs(__fpsr) __asm __volatile("wfs %0" : : "r" (__fpsr))
-#else
-#define __rfs(__fpsr)
-#define __wfs(__fpsr)
-#endif
 
 __fenv_static inline int
 feclearexcept(int __excepts)
@@ -218,6 +230,8 @@
 
 #endif /* __BSD_VISIBLE */
 
+#endif /* ARM_HARD_FLOAT */
+
 __END_DECLS
 
 #endif /* !_FENV_H_ */
Index: msun/arm/Makefile.inc
===================================================================
--- msun/arm/Makefile.inc (revision 229262)
+++ msun/arm/Makefile.inc (working copy)
@@ -1,4 +1,5 @@
 # $FreeBSD$
 
+INCS += fenv-softfloat.h
 LDBL_PREC = 53
 SYM_MAPS += ${.CURDIR}/arm/Symbol.map
Index: msun/src/fenv-softfloat.h
===================================================================
--- msun/src/fenv-softfloat.h (revision 0)
+++ msun/src/fenv-softfloat.h (revision 0)
@@ -0,0 +1,182 @@
+/*-
+ * Copyright (c) 2004-2011 David Schultz <[hidden email]>
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ *
+ * $FreeBSD$
+ */
+
+#ifndef _FENV_H_
+#error "This file is meant to be included only by <fenv.h>."
+#endif
+
+/*
+ * This file implements the functionality of <fenv.h> on platforms that
+ * lack an FPU and use softfloat in libc for floating point.  To use it,
+ * you must write an <fenv.h> that provides the following:
+ *
+ *   - a typedef for fenv_t, which may be an integer or struct type
+ *   - a typedef for fexcept_t (XXX This file assumes fexcept_t is a
+ *     simple integer type containing the exception mask.)
+ *   - definitions of FE_* constants for the five exceptions and four
+ *     rounding modes in IEEE 754, as described in fenv(3)
+ *   - a definition, and the corresponding external symbol, for FE_DFL_ENV
+ *   - a macro __set_env(env, flags, mask, rnd), which sets the given fenv_t
+ *     from the exception flags, mask, and rounding mode
+ *   - macros __env_flags(env), __env_mask(env), and __env_round(env), which
+ *     extract fields from an fenv_t
+ *   - a definition of __fenv_static
+ *
+ * If the architecture supports an optional FPU, it's recommended that you
+ * define fenv_t and fexcept_t to match the hardware ABI.  Otherwise, it
+ * doesn't matter how you define them.
+ */
+
+extern int __softfloat_float_exception_flags;
+extern int __softfloat_float_exception_mask;
+extern int __softfloat_float_rounding_mode;
+void __softfloat_float_raise(int);
+
+__fenv_static inline int
+feclearexcept(int __excepts)
+{
+
+ __softfloat_float_exception_flags &= ~__excepts;
+ return (0);
+}
+
+__fenv_static inline int
+fegetexceptflag(fexcept_t *__flagp, int __excepts)
+{
+
+ *__flagp = __softfloat_float_exception_flags & __excepts;
+ return (0);
+}
+
+__fenv_static inline int
+fesetexceptflag(const fexcept_t *__flagp, int __excepts)
+{
+
+ __softfloat_float_exception_flags |= *__flagp & __excepts;
+ return (0);
+}
+
+__fenv_static inline int
+feraiseexcept(int __excepts)
+{
+
+ __softfloat_float_raise(__excepts);
+ return (0);
+}
+
+__fenv_static inline int
+fetestexcept(int __excepts)
+{
+
+ return (__softfloat_float_exception_flags & __excepts);
+}
+
+__fenv_static inline int
+fegetround(void)
+{
+
+ return (__softfloat_float_rounding_mode);
+}
+
+__fenv_static inline int
+fesetround(int __round)
+{
+
+ __softfloat_float_rounding_mode = __round;
+ return (0);
+}
+
+__fenv_static inline int
+fegetenv(fenv_t *__envp)
+{
+
+ __set_env(*__envp, __softfloat_float_exception_flags,
+    __softfloat_float_exception_mask, __softfloat_float_rounding_mode);
+ return (0);
+}
+
+__fenv_static inline int
+feholdexcept(fenv_t *__envp)
+{
+ fenv_t __env;
+
+ fegetenv(__envp);
+ __softfloat_float_exception_mask = 0;
+ return (0);
+}
+
+__fenv_static inline int
+fesetenv(const fenv_t *__envp)
+{
+
+ __softfloat_float_exception_flags = __env_flags(*__envp);
+ __softfloat_float_exception_mask = __env_mask(*__envp);
+ __softfloat_float_rounding_mode = __env_round(*__envp);
+ return (0);
+}
+
+__fenv_static inline int
+feupdateenv(const fenv_t *__envp)
+{
+ int __oflags = __softfloat_float_exception_flags;
+
+ fesetenv(__envp);
+ feraiseexcept(__oflags);
+ return (0);
+}
+
+#if __BSD_VISIBLE
+
+/* We currently provide no external definitions of the functions below. */
+
+static inline int
+feenableexcept(int __mask)
+{
+ int __omask = __softfloat_float_exception_mask;
+
+ __softfloat_float_exception_mask |= __mask;
+ return (__omask);
+}
+
+static inline int
+fedisableexcept(int __mask)
+{
+ int __omask = __softfloat_float_exception_mask;
+
+ __softfloat_float_exception_mask &= ~__mask;
+ return (__omask);
+}
+
+static inline int
+fegetexcept(void)
+{
+
+ return (__softfloat_float_exception_mask);
+}
+
+#endif /* __BSD_VISIBLE */

Property changes on: msun/src/fenv-softfloat.h
___________________________________________________________________
Added: svn:mime-type
   + text/plain
Added: svn:keywords
   + FreeBSD=%H
Added: svn:eol-style
   + native

Index: msun/Makefile
===================================================================
--- msun/Makefile (revision 229262)
+++ msun/Makefile (working copy)
@@ -125,7 +125,7 @@
 
 SRCS= ${COMMON_SRCS} ${ARCH_SRCS}
 
-INCS= fenv.h math.h
+INCS+= fenv.h math.h
 
 MAN= acos.3 acosh.3 asin.3 asinh.3 atan.3 atan2.3 atanh.3 \
  ceil.3 ccos.3 ccosh.3 cexp.3 \
Index: libc/arm/softfloat/softfloat.h
===================================================================
--- libc/arm/softfloat/softfloat.h (revision 229262)
+++ libc/arm/softfloat/softfloat.h (working copy)
@@ -45,7 +45,7 @@
 /* #define FLOATX80 */
 /* #define FLOAT128 */
 
-#include <machine/ieeefp.h>
+#include <fenv.h>
 
 /*
 -------------------------------------------------------------------------------
@@ -84,12 +84,12 @@
 Software IEC/IEEE floating-point rounding mode.
 -------------------------------------------------------------------------------
 */
-extern fp_rnd_t float_rounding_mode;
+extern int float_rounding_mode;
 enum {
-    float_round_nearest_even = FP_RN,
-    float_round_to_zero      = FP_RZ,
-    float_round_down         = FP_RM,
-    float_round_up           = FP_RP
+    float_round_nearest_even = FE_TONEAREST,
+    float_round_to_zero      = FE_TOWARDZERO,
+    float_round_down         = FE_DOWNWARD,
+    float_round_up           = FE_UPWARD
 };
 
 /*
@@ -97,14 +97,14 @@
 Software IEC/IEEE floating-point exception flags.
 -------------------------------------------------------------------------------
 */
-extern fp_except float_exception_flags;
-extern fp_except float_exception_mask;
+extern int float_exception_flags;
+extern int float_exception_mask;
 enum {
-    float_flag_inexact   = FP_X_IMP,
-    float_flag_underflow = FP_X_UFL,
-    float_flag_overflow  = FP_X_OFL,
-    float_flag_divbyzero = FP_X_DZ,
-    float_flag_invalid   = FP_X_INV
+    float_flag_inexact   = FE_INEXACT,
+    float_flag_underflow = FE_UNDERFLOW,
+    float_flag_overflow  = FE_OVERFLOW,
+    float_flag_divbyzero = FE_DIVBYZERO,
+    float_flag_invalid   = FE_INVALID
 };
 
 /*
@@ -113,7 +113,7 @@
 exception flags.
 -------------------------------------------------------------------------------
 */
-void float_raise( fp_except );
+void float_raise( int );
 
 /*
 -------------------------------------------------------------------------------
Index: libc/softfloat/softfloat-specialize
===================================================================
--- libc/softfloat/softfloat-specialize (revision 229262)
+++ libc/softfloat/softfloat-specialize (working copy)
@@ -58,8 +58,8 @@
 should be simply `float_exception_flags |= flags;'.
 -------------------------------------------------------------------------------
 */
-fp_except float_exception_mask = 0;
-void float_raise( fp_except flags )
+int float_exception_mask = 0;
+void float_raise( int flags )
 {
 
     float_exception_flags |= flags;
Index: libc/softfloat/bits64/softfloat.c
===================================================================
--- libc/softfloat/bits64/softfloat.c (revision 229262)
+++ libc/softfloat/bits64/softfloat.c (working copy)
@@ -71,8 +71,8 @@
 and exception flags.
 -------------------------------------------------------------------------------
 */
-fp_rnd_t float_rounding_mode = float_round_nearest_even;
-fp_except float_exception_flags = 0;
+int float_rounding_mode = float_round_nearest_even;
+int float_exception_flags = 0;
 #ifdef FLOATX80
 int8 floatx80_rounding_precision = 80;
 #endif
Index: libc/softfloat/Symbol.map
===================================================================
--- libc/softfloat/Symbol.map (revision 229262)
+++ libc/softfloat/Symbol.map (working copy)
@@ -18,16 +18,10 @@
 };
 
 FBSDprivate_1.0 {
- _softfloat_float_exception_flags;
- _softfloat_float_exception_mask;
- _softfloat_float_rounding_mode;
- _softfloat_float_raise;
- _softfloat_float32_eq;
- _softfloat_float32_le;
- _softfloat_float32_lt;
- _softfloat_float64_eq;
- _softfloat_float64_le;
- _softfloat_float64_lt;
+ __softfloat_float_exception_flags;
+ __softfloat_float_exception_mask;
+ __softfloat_float_rounding_mode;
+ __softfloat_float_raise;
  __eqdf2;
  __eqsf2;
  __gedf2;
Index: libc/softfloat/softfloat-for-gcc.h
===================================================================
--- libc/softfloat/softfloat-for-gcc.h (revision 229262)
+++ libc/softfloat/softfloat-for-gcc.h (working copy)
@@ -5,17 +5,17 @@
  * Move private identifiers with external linkage into implementation
  * namespace.  -- Klaus Klein <[hidden email]>, May 5, 1999
  */
-#define float_exception_flags _softfloat_float_exception_flags
-#define float_exception_mask _softfloat_float_exception_mask
-#define float_rounding_mode _softfloat_float_rounding_mode
-#define float_raise _softfloat_float_raise
+#define float_exception_flags __softfloat_float_exception_flags
+#define float_exception_mask __softfloat_float_exception_mask
+#define float_rounding_mode __softfloat_float_rounding_mode
+#define float_raise __softfloat_float_raise
 /* The following batch are called by GCC through wrappers */
-#define float32_eq _softfloat_float32_eq
-#define float32_le _softfloat_float32_le
-#define float32_lt _softfloat_float32_lt
-#define float64_eq _softfloat_float64_eq
-#define float64_le _softfloat_float64_le
-#define float64_lt _softfloat_float64_lt
+#define float32_eq __softfloat_float32_eq
+#define float32_le __softfloat_float32_le
+#define float32_lt __softfloat_float32_lt
+#define float64_eq __softfloat_float64_eq
+#define float64_le __softfloat_float64_le
+#define float64_lt __softfloat_float64_lt
 
 /*
  * Macros to define functions with the GCC expected names
Index: libc/softfloat/bits32/softfloat.c
===================================================================
--- libc/softfloat/bits32/softfloat.c (revision 229262)
+++ libc/softfloat/bits32/softfloat.c (working copy)
@@ -77,8 +77,8 @@
 Floating-point rounding mode and exception flags.
 -------------------------------------------------------------------------------
 */
-fp_rnd_t float_rounding_mode = float_round_nearest_even;
-fp_except float_exception_flags = 0;
+int float_rounding_mode = float_round_nearest_even;
+int float_exception_flags = 0;
 
 /*
 -------------------------------------------------------------------------------
Index: libc/mips/softfloat/softfloat.h
===================================================================
--- libc/mips/softfloat/softfloat.h (revision 229262)
+++ libc/mips/softfloat/softfloat.h (working copy)
@@ -45,7 +45,7 @@
 /* #define FLOATX80 */
 /* #define FLOAT128 */
 
-#include <machine/ieeefp.h>
+#include <fenv.h>
 
 /*
 -------------------------------------------------------------------------------
@@ -84,12 +84,12 @@
 Software IEC/IEEE floating-point rounding mode.
 -------------------------------------------------------------------------------
 */
-extern fp_rnd_t float_rounding_mode;
+extern int float_rounding_mode;
 enum {
-    float_round_nearest_even = FP_RN,
-    float_round_to_zero      = FP_RZ,
-    float_round_down         = FP_RM,
-    float_round_up           = FP_RP
+    float_round_nearest_even = FE_TONEAREST,
+    float_round_to_zero      = FE_TOWARDZERO,
+    float_round_down         = FE_DOWNWARD,
+    float_round_up           = FE_UPWARD
 };
 
 /*
@@ -97,14 +97,14 @@
 Software IEC/IEEE floating-point exception flags.
 -------------------------------------------------------------------------------
 */
-extern fp_except float_exception_flags;
-extern fp_except float_exception_mask;
+extern int float_exception_flags;
+extern int float_exception_mask;
 enum {
-    float_flag_inexact   = FP_X_IMP,
-    float_flag_underflow = FP_X_UFL,
-    float_flag_overflow  = FP_X_OFL,
-    float_flag_divbyzero = FP_X_DZ,
-    float_flag_invalid   = FP_X_INV
+    float_flag_inexact   = FE_INEXACT,
+    float_flag_underflow = FE_UNDERFLOW,
+    float_flag_overflow  = FE_OVERFLOW,
+    float_flag_divbyzero = FE_DIVBYZERO,
+    float_flag_invalid   = FE_INVALID
 };
 
 /*
@@ -113,7 +113,7 @@
 exception flags.
 -------------------------------------------------------------------------------
 */
-void float_raise( fp_except );
+void float_raise( int );
 
 /*
 -------------------------------------------------------------------------------
_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arm
To unsubscribe, send any mail to "[hidden email]"

[Ian Lepore]

On Sun, 2012-01-08 at 13:36 -0500, David Schultz wrote:
> The patch below should make the fenv.h functions work on ARM chips
> without an FPU.  (Hardware FPU support is missing anyway, and
> what's in fenv.h right now is probably wrong.)
>
> It would be great if someone could test this, because I don't have
> the hardware.  The regression tests in tools/regression/lib/msun,
> particularly test-fenv.c, should provide some idea of what's
> working and what isn't.

Right now the most recent FreeBSD version I can conveniently build and
test with on ARM is 8.2.  Your patches applied cleanly to that code.

The compiler whined about missing definition of __fenv_static, so I
added "#define __fenv_static static" in arm/fenv.h, that allowed the
library compile to complete.

My hardware is an Atmel at91rm9200 (180mhz armv4, no fpu).

I'm using this compiler:
  # gcc -v
  Using built-in specs.
  Target: arm-undermydesk-freebsd
  Configured with: FreeBSD/arm system compiler
  Thread model: posix
  gcc version 4.2.2 20070831 prerelease [FreeBSD]

Building the test with this command in tools/regression/lib/msun:
  cc -pipe -O0 test-fenv.c -o test-fenv -static -lm

Which results in this file:
  # file test-fenv                                                                                
  test-fenv: ELF 32-bit LSB executable, ARM, version 1 (FreeBSD), statically linked, for FreeBSD 8.2 (802508), not stripped

Here's a run with the stock/unpatched 8.2 math lib:
  tflex# /tmp/test-fenv
  1..8
  ok 1 - fenv
  Assertion failed: (fetestexcept(std_except_sets[i]) == 0), function test_fetestclearexcept, file test-fenv.c, line 150.
  Abort (core dumped)

And here with your patches applied:
  tflex# /tmp/test-fenv
  1..8
  ok 1 - fenv
  ok 2 - fenv
  Assertion failed: (fetestexcept(ALL_STD_EXCEPT) == (ALL_STD_EXCEPT ^ excepts)), function test_fegsetexceptflag, file test-fenv.c, line 193.
  Abort (core dumped)

That looks like progress, if not complete success. :)

My first attempts at this yesterday yielded confusing results. I had
copied test-fenv.c into our product-build environment for convenience,
and I forgot that environment puts -DNDEBUG on the command line by
default.  Man, does that ever lead to confusing output... lots of tests
that didn't work appeared to, then test 6 appeared to fail in a
completely confusing way (well, confusing until I realized all those
asserts in the source expanded to nothing).  Given how heavily the test
suite code relies on assert() expanding to actual code, it might not be
a bad idea to put something in each test-whatever.c along the lines of

  #ifdef NDEBUG
  #error This code will not work with NDEBUG defined
  #endif

Now that I've gotten past the workflow-glitches it'll be easy for me to
re-test a new patch or generate more info for you.

-- Ian


_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arm
To unsubscribe, send any mail to "[hidden email]"

[David Schultz]

Thanks for the detailed report!  Some replies below...

On Mon, Jan 09, 2012, Ian Lepore wrote:

> On Sun, 2012-01-08 at 13:36 -0500, David Schultz wrote:
> > The patch below should make the fenv.h functions work on ARM chips
> > without an FPU.  (Hardware FPU support is missing anyway, and
> > what's in fenv.h right now is probably wrong.)
> >
> > It would be great if someone could test this, because I don't have
> > the hardware.  The regression tests in tools/regression/lib/msun,
> > particularly test-fenv.c, should provide some idea of what's
> > working and what isn't.
>
> Right now the most recent FreeBSD version I can conveniently build and
> test with on ARM is 8.2.  Your patches applied cleanly to that code.
>
> The compiler whined about missing definition of __fenv_static, so I
> added "#define __fenv_static static" in arm/fenv.h, that allowed the
> library compile to complete.

That's fine.  The #define you were missing is in HEAD but not 8.2.

> Here's a run with the stock/unpatched 8.2 math lib:
>   tflex# /tmp/test-fenv
>   1..8
>   ok 1 - fenv
>   Assertion failed: (fetestexcept(std_except_sets[i]) == 0), function test_fetestclearexcept, file test-fenv.c, line 150.
>   Abort (core dumped)
>
> And here with your patches applied:
>   tflex# /tmp/test-fenv
>   1..8
>   ok 1 - fenv
>   ok 2 - fenv
>   Assertion failed: (fetestexcept(ALL_STD_EXCEPT) == (ALL_STD_EXCEPT ^ excepts)), function test_fegsetexceptflag, file test-fenv.c, line 193.
>   Abort (core dumped)
>
> That looks like progress, if not complete success. :)
>
> My first attempts at this yesterday yielded confusing results. I had
> copied test-fenv.c into our product-build environment for convenience,
> and I forgot that environment puts -DNDEBUG on the command line by
> default.  Man, does that ever lead to confusing output... lots of tests
> that didn't work appeared to, then test 6 appeared to fail in a
> completely confusing way (well, confusing until I realized all those
> asserts in the source expanded to nothing).  Given how heavily the test
> suite code relies on assert() expanding to actual code, it might not be
> a bad idea to put something in each test-whatever.c along the lines of
>
>   #ifdef NDEBUG
>   #error This code will not work with NDEBUG defined
>   #endif

Small progress!  My apologies for the hastily written regression
test.  Its main purpose in life is to blow up when something
breaks, so it uses assert() and doesn't include the extra
mechanics to report exactly what went wrong.

Debugging it without hardware could be tedious, and I don't
actually have the time to dedicate to ARM development.  But it's
encouraging that all the symbols resolve and nothing bad seems to
happen.  Perhaps I can commit the softfloat bindings and one of
the ARM developers can track down the (hopefully simple) bugs in
softfloat or the bindings that prevent fenv.h from working.  It's
bound to be better than the no-ops we have now, in any case.
_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arm
To unsubscribe, send any mail to "[hidden email]"

[Olivier Houchard-3]

Hi David,

On Wed, Jan 11, 2012 at 12:26:34AM -0500, David Schultz wrote:
> Debugging it without hardware could be tedious, and I don't
> actually have the time to dedicate to ARM development.  But it's
> encouraging that all the symbols resolve and nothing bad seems to
> happen.  Perhaps I can commit the softfloat bindings and one of
> the ARM developers can track down the (hopefully simple) bugs in
> softfloat or the bindings that prevent fenv.h from working.  It's
> bound to be better than the no-ops we have now, in any case.

I'm fine with this, if it doesn't make things worse I'd rather not see this
code lost.

Regards,

Olivier
_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arm
To unsubscribe, send any mail to "[hidden email]"

[Ian Lepore]

On Wed, 2012-01-11 at 11:18 +0100, Olivier Houchard wrote:

> Hi David,
>
> On Wed, Jan 11, 2012 at 12:26:34AM -0500, David Schultz wrote:
> > Debugging it without hardware could be tedious, and I don't
> > actually have the time to dedicate to ARM development.  But it's
> > encouraging that all the symbols resolve and nothing bad seems to
> > happen.  Perhaps I can commit the softfloat bindings and one of
> > the ARM developers can track down the (hopefully simple) bugs in
> > softfloat or the bindings that prevent fenv.h from working.  It's
> > bound to be better than the no-ops we have now, in any case.
>
> I'm fine with this, if it doesn't make things worse I'd rather not see this
> code lost.
>
> Regards,
>
> Olivier

I can probably find time to work with it a bit more, I just wanted to
quickly post the initial results, especially in case the feedback was
along the lines of "thanks, but testing in an 8.2 environment won't
work."  We rely pretty heavily on floating point math on ARM at work, so
I may even be able to dedicate some non-hobbyist cycles to it (it occurs
to me that running the whole suite of regression tests now that we've
upgraded from 6.2->8.2 would probably be wise).

-- Ian


_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arm
To unsubscribe, send any mail to "[hidden email]"

[David Schultz]

On Wed, Jan 11, 2012, Ian Lepore wrote:
> I can probably find time to work with it a bit more, I just wanted to
> quickly post the initial results, especially in case the feedback was
> along the lines of "thanks, but testing in an 8.2 environment won't
> work."  We rely pretty heavily on floating point math on ARM at work, so
> I may even be able to dedicate some non-hobbyist cycles to it (it occurs
> to me that running the whole suite of regression tests now that we've
> upgraded from 6.2->8.2 would probably be wise).

That would be great.  A few of the libm regression tests and libm
functions depend on fenv.h working, by the way.  But most of the
important stuff should work.
_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arm
To unsubscribe, send any mail to "[hidden email]"

[Ian Lepore]

On Wed, 2012-01-11 at 12:55 -0500, David Schultz wrote:

> On Wed, Jan 11, 2012, Ian Lepore wrote:
> > I can probably find time to work with it a bit more, I just wanted to
> > quickly post the initial results, especially in case the feedback was
> > along the lines of "thanks, but testing in an 8.2 environment won't
> > work."  We rely pretty heavily on floating point math on ARM at work, so
> > I may even be able to dedicate some non-hobbyist cycles to it (it occurs
> > to me that running the whole suite of regression tests now that we've
> > upgraded from 6.2->8.2 would probably be wise).
>
> That would be great.  A few of the libm regression tests and libm
> functions depend on fenv.h working, by the way.  But most of the
> important stuff should work.

Now we have real progress.  Your code just needed a couple little
tweaks, and we needed an arm implementation of __flt_rounds() for
FLT_ROUNDS to work right.  This gets most of the regression suite
working (I'll post a log of results separately).

Here are the patches (apply these on top of your original patches, but
from /usr/src since it has to reach into sys/)...


diff -r a617cd67f1b1 -r 27a0ad9ff826 lib/libc/arm/gen/Makefile.inc
--- lib/libc/arm/gen/Makefile.inc Fri Jan 13 19:21:01 2012 -0700
+++ lib/libc/arm/gen/Makefile.inc Fri Jan 13 19:28:16 2012 -0700
@@ -3,4 +3,4 @@
 
 SRCS+= _ctx_start.S _setjmp.S _set_tp.c alloca.S fabs.c \
  infinity.c ldexp.c makecontext.c modf.c \
- setjmp.S signalcontext.c sigsetjmp.S divsi3.S
+ setjmp.S signalcontext.c sigsetjmp.S divsi3.S flt_rounds.c
diff -r a617cd67f1b1 -r 27a0ad9ff826 lib/libc/arm/gen/flt_rounds.c
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ lib/libc/arm/gen/flt_rounds.c Fri Jan 13 19:28:16 2012 -0700
@@ -0,0 +1,69 @@
+/*-
+ * Copyright (c) 2011 David Schultz <[hidden email]>
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ *
+ * $FreeBSD:  $
+ */
+
+#include <sys/cdefs.h>
+__FBSDID("$FreeBSD:  $");
+
+#include <machine/float.h>
+#include <fenv.h>
+
+#ifndef ARM_HARD_FLOAT
+
+int
+__flt_rounds(void)
+{
+
+ /*
+ * Translate our rounding modes to the unnamed
+ * manifest constants required by C99 et. al.
+ */
+ switch (__softfloat_float_rounding_mode) {
+ case  FE_TOWARDZERO: return (0);
+ case  FE_TONEAREST:  return (1);
+ case  FE_UPWARD:     return (2);
+ case  FE_DOWNWARD:   return (3);
+ }
+ return (-1);
+}
+
+#else /* ARM_HARD_FLOAT */
+
+int
+__flt_rounds(void)
+{
+
+ /*
+ * Apparently, the rounding mode is specified as part of the
+ * instruction format on ARM, so the dynamic rounding mode is
+ * indeterminate.  Some FPUs may differ.
+ */
+ return (-1);
+}
+
+#endif /* ARM_HARD_FLOAT */
+
diff -r a617cd67f1b1 -r 27a0ad9ff826 lib/msun/src/fenv-softfloat.h
--- lib/msun/src/fenv-softfloat.h Fri Jan 13 19:21:01 2012 -0700
+++ lib/msun/src/fenv-softfloat.h Fri Jan 13 19:28:16 2012 -0700
@@ -77,6 +77,7 @@ __fenv_static inline int
 fesetexceptflag(const fexcept_t *__flagp, int __excepts)
 {
 
+ __softfloat_float_exception_flags &= ~__excepts;
  __softfloat_float_exception_flags |= *__flagp & __excepts;
  return (0);
 }
@@ -126,6 +127,7 @@ feholdexcept(fenv_t *__envp)
  fenv_t __env;
 
  fegetenv(__envp);
+ __softfloat_float_exception_flags = 0;
  __softfloat_float_exception_mask = 0;
  return (0);
 }
diff -r a617cd67f1b1 -r 27a0ad9ff826 sys/arm/include/float.h
--- sys/arm/include/float.h Fri Jan 13 19:21:01 2012 -0700
+++ sys/arm/include/float.h Fri Jan 13 19:28:16 2012 -0700
@@ -44,7 +44,7 @@ extern int __flt_rounds(void);
 __END_DECLS
 
 #define FLT_RADIX 2 /* b */
-#define FLT_ROUNDS -1
+#define FLT_ROUNDS __flt_rounds()
 #define FLT_EVAL_METHOD (-1) /* XXX */
 #define DECIMAL_DIG 17 /* max precision in decimal digits */
 



_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arm
To unsubscribe, send any mail to "[hidden email]"

[Ian Lepore]

On Wed, 2012-01-11 at 12:55 -0500, David Schultz wrote:

> On Wed, Jan 11, 2012, Ian Lepore wrote:
> > I can probably find time to work with it a bit more, I just wanted to
> > quickly post the initial results, especially in case the feedback was
> > along the lines of "thanks, but testing in an 8.2 environment won't
> > work."  We rely pretty heavily on floating point math on ARM at work, so
> > I may even be able to dedicate some non-hobbyist cycles to it (it occurs
> > to me that running the whole suite of regression tests now that we've
> > upgraded from 6.2->8.2 would probably be wise).
>
> That would be great.  A few of the libm regression tests and libm
> functions depend on fenv.h working, by the way.  But most of the
> important stuff should work.

These are the results of running all the regression tests (except those
requiring complex number support) after applying the minor tweaks to
David's original patches.  The build and run conditions are the same as
I mentioned earlier in the thread.  

I looked into why test-lrint failed, and got as far as determing that
the rint() routine is raising FE_INVALID when it executes either of the
two "w = TWO52[sx]" expressions in the routine.  The path from there
leads deep into places my innumeracy won't let me go.  

The other errors also look to be more related to numerical details of
the implementation than to the changes in the environment support, and
I'm really really not a math guy, so this seems like a good time to lob
it back to someone who actually understands the code.


---------- test-conj ----------
1..42
ok 1 # conjf(0.0 + 0.0I)
ok 2 # conj(0.0 + 0.0I)
ok 3 # conjl(0.0 + 0.0I)
ok 4 # conjf(0.0 + 1.0I)
ok 5 # conj(0.0 + 1.0I)
ok 6 # conjl(0.0 + 1.0I)
ok 7 # conjf(1.0 + 0.0I)
ok 8 # conj(1.0 + 0.0I)
ok 9 # conjl(1.0 + 0.0I)
ok 10 # conjf(-1.0 + 0.0I)
ok 11 # conj(-1.0 + 0.0I)
ok 12 # conjl(-1.0 + 0.0I)
ok 13 # conjf(1.0 + -0.0I)
ok 14 # conj(1.0 + -0.0I)
ok 15 # conjl(1.0 + -0.0I)
ok 16 # conjf(0.0 + -1.0I)
ok 17 # conj(0.0 + -1.0I)
ok 18 # conjl(0.0 + -1.0I)
ok 19 # conjf(2.0 + 4.0I)
ok 20 # conj(2.0 + 4.0I)
ok 21 # conjl(2.0 + 4.0I)
ok 22 # conjf(0.0 + infI)
ok 23 # conj(0.0 + infI)
ok 24 # conjl(0.0 + infI)
ok 25 # conjf(0.0 + -infI)
ok 26 # conj(0.0 + -infI)
ok 27 # conjl(0.0 + -infI)
ok 28 # conjf(inf + 0.0I)
ok 29 # conj(inf + 0.0I)
ok 30 # conjl(inf + 0.0I)
ok 31 # conjf(nan + 1.0I)
ok 32 # conj(nan + 1.0I)
ok 33 # conjl(nan + 1.0I)
ok 34 # conjf(1.0 + nanI)
ok 35 # conj(1.0 + nanI)
ok 36 # conjl(1.0 + nanI)
ok 37 # conjf(nan + nanI)
ok 38 # conj(nan + nanI)
ok 39 # conjl(nan + nanI)
ok 40 # conjf(-inf + infI)
ok 41 # conj(-inf + infI)
ok 42 # conjl(-inf + infI)
---------- test-csqrt ----------
1..15
ok 1 - csqrt
ok 2 - csqrt
ok 3 - csqrt
ok 4 - csqrt
ok 5 - csqrt
ok 6 - csqrt
ok 7 - csqrt
ok 8 - csqrt
ok 9 - csqrt
ok 10 - csqrt
ok 11 - csqrt
ok 12 - csqrt
ok 13 - csqrt
ok 14 - csqrt
ok 15 - csqrt
---------- test-exponential ----------
1..3
ok 1 - exponential
ok 2 - exponential
ok 3 - exponential
---------- test-fenv ----------
1..8
ok 1 - fenv
ok 2 - fenv
ok 3 - fenv
ok 4 - fenv
ok 5 - fenv
ok 6 - fenv
ok 7 - fenv
ok 8 - fenv
---------- test-fma ----------
1..19
ok 1 - fma zeroes
ok 2 - fma zeroes
ok 3 - fma zeroes
ok 4 - fma zeroes
ok 5 - fma infinities
ok 6 - fma infinities
ok 7 - fma infinities
ok 8 - fma infinities
ok 9 - fma NaNs
ok 10 - fma small z
ok 11 - fma small z
ok 12 - fma small z
ok 13 - fma small z
ok 14 - fma big z
ok 15 - fma big z
ok 16 - fma big z
ok 17 - fma big z
ok 18 - fma accuracy
Assertion failed: (fpequal((fmaf)((0x1.800002p+0), (0x1.800002p+0),
(-0x1.000002p-46)), (0x1.200002p+1))), function test_double_rounding,
file test-fma.c, line 456.
Abort trap
---------- test-fmaxmin ----------
1..12
ok 1 - big = 0, small = 0
ok 2 - big = 10, small = 9.999996185302734375
ok 3 - big = 10.000003814697265625, small = 10
ok 4 - big = -1, small = -1
ok 5 - big = -1, small = -0
fmaxl(0, nan) raised 0x1
fmaxl(0, nan) = nan, expected 0
fmaxl(nan, 0) raised 0x1
fminl(0, nan) raised 0x1
fminl(0, nan) = nan, expected 0
fminl(nan, 0) raised 0x1
FAILURE in rounding mode 0
not ok 6 - big = 0, small = nan
fmaxl(inf, nan) raised 0x1
fmaxl(inf, nan) = nan, expected inf
fmaxl(nan, inf) raised 0x1
fminl(inf, nan) raised 0x1
fminl(inf, nan) = nan, expected inf
fminl(nan, inf) raised 0x1
FAILURE in rounding mode 0
not ok 7 - big = inf, small = nan
ok 8 - big = inf, small = 0
ok 9 - big = -1, small = -inf
ok 10 - big = 1, small = -inf
fmaxl(nan, nan) raised 0x1
fmaxl(nan, nan) raised 0x1
fminl(nan, nan) raised 0x1
fminl(nan, nan) raised 0x1
FAILURE in rounding mode 0
not ok 11 - big = nan, small = nan
ok 12 - big = 0, small = -0
---------- test-ilogb ----------
1..3
ok 1 - ilogb
ok 2 - ilogbf
Assertion failed: (i == e), function main, file test-ilogb.c, line 80.
Abort trap
---------- test-invtrig ----------
1..7
ok 1 - special
ok 2 - atan2 special
ok 3 - accuracy
ok 4 - atan2 p2x
ok 5 - tiny inputs
ok 6 - atan huge inputs
ok 7 - inverse
---------- test-logarithm ----------
1..3
ok 1 - logarithm
ok 2 - logarithm
ok 3 - logarithm
---------- test-lrint ----------
1..1
Assertion failed: (fetestexcept(FE_ALL_EXCEPT) == (0)), function
run_tests, file test-lrint.c, line 81.
Abort trap
---------- test-lround ----------
1..1
Assertion failed: (fetestexcept(FE_ALL_EXCEPT) == (0)), function main,
file test-lround.c, line 74.
Abort trap
---------- test-nan ----------
1..1
ok 1 - nan
---------- test-nearbyint ----------
1..12
ok 1 # nearbyint(+-0)
ok 2 # modf(+-0)
ok 3 # nearbyint(+-0.5)
ok 4 # modf(+-0.5)
ok 5 # nearbyint(+-3.14159)
ok 6 # modf(+-3.14159)
ok 7 # nearbyint(+-65536.5)
ok 8 # modf(+-65536.5)
ok 9 # nearbyint(+-inf)
Assertion failed: (fpequal(out, modf(in, &ipart))), function test_modf,
file test-nearbyint.c, line 147.
Abort trap
---------- test-next ----------
1..5
ok 1 - next
ok 2 - next
ok 3 - next
ok 4 - next
216: idd(nexttoward(INFINITY, DBL_MAX * 2.0L)) returned inf, expecting
0x1.fffffffffffffp+1023
Abort trap
---------- test-rem ----------
1..2
ok 1 - rem
ok 2 - rem
---------- test-trig ----------
1..3
ok 1 - trig
ok 2 - trig
ok 3 - trig


-- Ian


_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arm
To unsubscribe, send any mail to "[hidden email]"

[David Schultz]

On Fri, Jan 13, 2012, Ian Lepore wrote:
> These are the results of running all the regression tests (except those
> requiring complex number support) after applying the minor tweaks to
> David's original patches.  The build and run conditions are the same as
> I mentioned earlier in the thread.  

Thanks!  These are very helpful.  Some of the failures are due to
bugs that were fixed since 8.2-RELEASE.  (Indeed, the regression
tests were added because of the bugs.)  Other failures are more
mysterious.

> I looked into why test-lrint failed, and got as far as determing that
> the rint() routine is raising FE_INVALID when it executes either of the
> two "w = TWO52[sx]" expressions in the routine.  The path from there
> leads deep into places my innumeracy won't let me go.  
>
> The other errors also look to be more related to numerical details of
> the implementation than to the changes in the environment support, and
> I'm really really not a math guy, so this seems like a good time to lob
> it back to someone who actually understands the code.

That's strange, because the test on line 81 uses 1.0 as the input.
For integral inputs, rint() shouldn't even be reaching those lines.
Are you sure it wasn't rintl() that failed?

> ---------- test-fma ----------
> 1..19
> ok 1 - fma zeroes
> ok 2 - fma zeroes
> ok 3 - fma zeroes
> ok 4 - fma zeroes
> ok 5 - fma infinities
> ok 6 - fma infinities
> ok 7 - fma infinities
> ok 8 - fma infinities
> ok 9 - fma NaNs
> ok 10 - fma small z
> ok 11 - fma small z
> ok 12 - fma small z
> ok 13 - fma small z
> ok 14 - fma big z
> ok 15 - fma big z
> ok 16 - fma big z
> ok 17 - fma big z
> ok 18 - fma accuracy
> Assertion failed: (fpequal((fmaf)((0x1.800002p+0), (0x1.800002p+0),
> (-0x1.000002p-46)), (0x1.200002p+1))), function test_double_rounding,
> file test-fma.c, line 456.
> Abort trap

No need to worry about this one.  It's fixed in r226371 and MFC'd
to 8-STABLE in r229840.

> ---------- test-fmaxmin ----------
> 1..12
> ok 1 - big = 0, small = 0
> ok 2 - big = 10, small = 9.999996185302734375
> ok 3 - big = 10.000003814697265625, small = 10
> ok 4 - big = -1, small = -1
> ok 5 - big = -1, small = -0
> fmaxl(0, nan) raised 0x1
> fmaxl(0, nan) = nan, expected 0
> fmaxl(nan, 0) raised 0x1
> fminl(0, nan) raised 0x1
> fminl(0, nan) = nan, expected 0
> fminl(nan, 0) raised 0x1
> FAILURE in rounding mode 0
> not ok 6 - big = 0, small = nan
> fmaxl(inf, nan) raised 0x1
> fmaxl(inf, nan) = nan, expected inf
> fmaxl(nan, inf) raised 0x1
> fminl(inf, nan) raised 0x1
> fminl(inf, nan) = nan, expected inf
> fminl(nan, inf) raised 0x1
> FAILURE in rounding mode 0
> not ok 7 - big = inf, small = nan
> ok 8 - big = inf, small = 0
> ok 9 - big = -1, small = -inf
> ok 10 - big = 1, small = -inf
> fmaxl(nan, nan) raised 0x1
> fmaxl(nan, nan) raised 0x1
> fminl(nan, nan) raised 0x1
> fminl(nan, nan) raised 0x1
> FAILURE in rounding mode 0
> not ok 11 - big = nan, small = nan
> ok 12 - big = 0, small = -0

The interesting thing about these failures is that the reported
inputs to the tests are wrong.  I suggest running the tests in
tools/regression/lib/libc/stdio, particularly test-printfloat.

> ---------- test-ilogb ----------
> 1..3
> ok 1 - ilogb
> ok 2 - ilogbf
> Assertion failed: (i == e), function main, file test-ilogb.c, line 80.
> Abort trap

Likely related to the previous errors.  I'm wondering if the
definitions in libc/arm/_fpmath.h get the endianness right
for the ARM chip you're using.

> ---------- test-lrint ----------
> 1..1
> Assertion failed: (fetestexcept(FE_ALL_EXCEPT) == (0)), function
> run_tests, file test-lrint.c, line 81.
> Abort trap
> ---------- test-lround ----------
> 1..1
> Assertion failed: (fetestexcept(FE_ALL_EXCEPT) == (0)), function main,
> file test-lround.c, line 74.
> Abort trap

As mentioned above, it's hard to say what's causing these failures
if fenv and softfloat work.

> ---------- test-nearbyint ----------
> 1..12
> ok 1 # nearbyint(+-0)
> ok 2 # modf(+-0)
> ok 3 # nearbyint(+-0.5)
> ok 4 # modf(+-0.5)
> ok 5 # nearbyint(+-3.14159)
> ok 6 # modf(+-3.14159)
> ok 7 # nearbyint(+-65536.5)
> ok 8 # modf(+-65536.5)
> ok 9 # nearbyint(+-inf)
> Assertion failed: (fpequal(out, modf(in, &ipart))), function test_modf,
> file test-nearbyint.c, line 147.
> Abort trap

Minor bug, fixed in r226606.

> ---------- test-next ----------
> 1..5
> ok 1 - next
> ok 2 - next
> ok 3 - next
> ok 4 - next
> 216: idd(nexttoward(INFINITY, DBL_MAX * 2.0L)) returned inf, expecting
> 0x1.fffffffffffffp+1023
> Abort trap

This was a bug in the test, which didn't know how to cope with
architectures where long double doesn't have more range than
double.  The test is fixed in 230102.
_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arm
To unsubscribe, send any mail to "[hidden email]"

[Ian Lepore]

On Sat, 2012-01-14 at 03:12 -0500, David Schultz wrote:

> On Fri, Jan 13, 2012, Ian Lepore wrote:
> > These are the results of running all the regression tests (except those
> > requiring complex number support) after applying the minor tweaks to
> > David's original patches.  The build and run conditions are the same as
> > I mentioned earlier in the thread.  
>
> Thanks!  These are very helpful.  Some of the failures are due to
> bugs that were fixed since 8.2-RELEASE.  (Indeed, the regression
> tests were added because of the bugs.)  Other failures are more
> mysterious.

Well then, they're all mysterious.  I wanted to avoid exactly this kind
of trouble of testing old code that had already been fixed after 8.2.  I
did a "start over" a couple days ago where I pulled a fresh copy of our
8.2 source (really 8-stable as of svn rev 223361, then with lots of our
own patches on top, but none in libc or libm), then I overlaid the
complete contents of lib/msun and tools/regression/lib/msun from
-current and got that building on 8.2 before applying your patches.  The
snapshot of -current I pulled the math code from was Jan 4th or 5th (via
cvs, I haven't got an svn working environment here yet).

My notes on backporting the -current code to 8.2 had this paragraph,
could this be part of the trouble...

        I also had to revert all the Symbol.map files that tried to mark
        some of the new functions as available only in FSBD_1.3 lib
        version, because the lib tool was unhappy with that.
       
I didn't think that would be significant in terms of getting this
testing done.

Also, I did not pull the complete libc from -current, could that be the
problem?  The testing was with a stock 8.2 libc with only your patches
from this list applied.

I just spot-checked the s_fma.c routine in my test chroot against the
svn log for r226371 and I've definitely got those changes, and I assume
that's true for the other things you mentioned as already-fixed.

When it comes to the test-lrint failure, I'm very sure it was running
rint(), because I was debugging by sticking printf()s in the routine in
s_rint.c (I don't have a working gdb for 8.2 arm, so printf debugging is
my only option, but really I tend to prefer it to gdb anyway).  It did
strike me as very odd that the test of rounding 1.0 was the one to fail
with FE_INVALID.

There definitely isn't anything drastically wrong with our 8.2 floating
point environment, even with the patched backported -current stuff,
because in addition to the test suite, I'm also running our regular app
on this platform linked against the same libraries.  It's steering an
atomic clock, keeping it within a few nanos of UTC(GPS), and I'm
measuring its performance against another unit here running older code
and they're performing comparably.  There could be little glitches in
non-critical things like printf, but nothing like wrong-endianess.

I'll see if I can figure out whether there's anything environmental
going on here.  I'm linking the tests with -static, my intent being that
I wanted to make sure I was running the code I think I am, not code from
some cached shared lib or something.  Static linking is a thing that
doesn't get tested much anymore, and does sometimes lead to glitches you
don't see with dynamic linking.  For example, I'm compiling the tests
with -O0 but I think the libs get compiled with -O1, and I've seen that
cause trouble with inline functions -- the linker will whine about
multiple definitions of an object having different sizes, although I
didn't see any such warnings in my test compiles.

-- Ian


_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arm
To unsubscribe, send any mail to "[hidden email]"

[David Schultz]

On Sat, Jan 14, 2012, Ian Lepore wrote:

> On Sat, 2012-01-14 at 03:12 -0500, David Schultz wrote:
> > On Fri, Jan 13, 2012, Ian Lepore wrote:
> > > These are the results of running all the regression tests (except those
> > > requiring complex number support) after applying the minor tweaks to
> > > David's original patches.  The build and run conditions are the same as
> > > I mentioned earlier in the thread.  
> >
> > Thanks!  These are very helpful.  Some of the failures are due to
> > bugs that were fixed since 8.2-RELEASE.  (Indeed, the regression
> > tests were added because of the bugs.)  Other failures are more
> > mysterious.
>
> Well then, they're all mysterious.  I wanted to avoid exactly this kind
> of trouble of testing old code that had already been fixed after 8.2.  I
> did a "start over" a couple days ago where I pulled a fresh copy of our
> 8.2 source (really 8-stable as of svn rev 223361, then with lots of our
> own patches on top, but none in libc or libm), then I overlaid the
> complete contents of lib/msun and tools/regression/lib/msun from
> -current and got that building on 8.2 before applying your patches.  The
> snapshot of -current I pulled the math code from was Jan 4th or 5th (via
> cvs, I haven't got an svn working environment here yet).
>
> My notes on backporting the -current code to 8.2 had this paragraph,
> could this be part of the trouble...
>
>         I also had to revert all the Symbol.map files that tried to mark
>         some of the new functions as available only in FSBD_1.3 lib
>         version, because the lib tool was unhappy with that.
>        
> I didn't think that would be significant in terms of getting this
> testing done.
>
> Also, I did not pull the complete libc from -current, could that be the
> problem?  The testing was with a stock 8.2 libc with only your patches
> from this list applied.
>
> I just spot-checked the s_fma.c routine in my test chroot against the
> svn log for r226371 and I've definitely got those changes, and I assume
> that's true for the other things you mentioned as already-fixed.

For historical reasons, modf() lives in libc, but otherwise the
libc sources shouldn't matter.  If you have the msun sources from
HEAD and you're still getting those errors, there may be something
wrong with fenv, softfloat, or printf.

It would be interesting to see the results of the test-printfloat
test in regression/lib/libc/stdio.

> When it comes to the test-lrint failure, I'm very sure it was running
> rint(), because I was debugging by sticking printf()s in the routine in
> s_rint.c (I don't have a working gdb for 8.2 arm, so printf debugging is
> my only option, but really I tend to prefer it to gdb anyway).  It did
> strike me as very odd that the test of rounding 1.0 was the one to fail
> with FE_INVALID.

This is a pretty basic failure, so if we can figure it out, it
might explain some of the other bugs.  A minimal test that
reproduces the problem would be a good start.  I still find it
hard to believe that 1.0 is the value being passed to rint().
Printing the values of i0 and i1 in hex inside rint() would show
you the bit pattern being passed in, and might explain why the
wrong branch is being taken.  The correct representation of 1.0
is 0x3ff0000000000000, with the all-zero word in i1.

> I'll see if I can figure out whether there's anything environmental
> going on here.  I'm linking the tests with -static, my intent being that
> I wanted to make sure I was running the code I think I am, not code from
> some cached shared lib or something.  Static linking is a thing that
> doesn't get tested much anymore, and does sometimes lead to glitches you
> don't see with dynamic linking.  For example, I'm compiling the tests
> with -O0 but I think the libs get compiled with -O1, and I've seen that
> cause trouble with inline functions -- the linker will whine about
> multiple definitions of an object having different sizes, although I
> didn't see any such warnings in my test compiles.

The tests are compiled with -O0 by default.  The -fno-builtin flag
(for both the tests and for libm itself) is also useful, since it
rules out bugs due to bogus compiler builtins.
_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arm
To unsubscribe, send any mail to "[hidden email]"

[Ian Lepore]

On Sat, 2012-01-14 at 13:29 -0500, David Schultz wrote:
> It would be interesting to see the results of the test-printfloat
> test in regression/lib/libc/stdio.

Well that narrowed it down quickly.  I commented out the abort() calls
so all the tests would run, and commented out the alternate locale
testing because we don't install locales on our boxes.  I'm also adding
-fno-builtin to the test compiles now.  

Everything relating to printing a long double fails.  I don't know yet
whether it's just printing, or whether all the failures at this point
are being caused by wrong or incomplete long double support in my
environment.


tflex# ./test-printfloat
1..11
66: printf("%13LE", 1.0L) ==> [ 0.000000E+00], expected [ 1.000000E+00]
66: wprintf("%13LE", 1.0L) ==> [ 0.000000E+00], expected [ 1.000000E+00]
67: printf("%13Lf", 1.0L) ==> [     0.000000], expected [     1.000000]
67: wprintf("%13Lf", 1.0L) ==> [     0.000000], expected [     1.000000]
68: printf("%13LG", 1.0L) ==> [            0], expected [            1]
68: wprintf("%13LG", 1.0L) ==> [            0], expected [            1]
75: printf("%Le", 1234567.8L) ==> [1.859918e+05], expected [1.234568e+06]
75: wprintf("%Le", 1234567.8L) ==> [1.859918e+05], expected [1.234568e+06]
76: printf("%Lf", 1234567.8L) ==> [185991.800000], expected [1234567.800000]
76: wprintf("%Lf", 1234567.8L) ==> [185991.800000], expected [1234567.800000]
77: printf("%LG", 1234567.8L) ==> [185992], expected [1.23457E+06]
77: wprintf("%LG", 1234567.8L) ==> [185992], expected [1.23457E+06]
ok 1 - printfloat
ok 2 - printfloat
ok 3 - printfloat
ok 4 - printfloat
xx 5 - no locale testing
ok 6 - printfloat
ok 7 - printfloat
ok 8 - printfloat
221: printf("%.3Lf", 4.4375L) ==> [0.437], expected [4.437]
221: wprintf("%.3Lf", 4.4375L) ==> [0.437], expected [4.437]
222: printf("%.3Lf", -4.4375L) ==> [-0.438], expected [-4.438]
222: wprintf("%.3Lf", -4.4375L) ==> [-0.438], expected [-4.438]
227: printf("%.3Lf", 4.4375L) ==> [0.438], expected [4.438]
227: wprintf("%.3Lf", 4.4375L) ==> [0.438], expected [4.438]
228: printf("%.3Lf", -4.4375L) ==> [-0.437], expected [-4.437]
228: wprintf("%.3Lf", -4.4375L) ==> [-0.437], expected [-4.437]
233: printf("%.3Lf", 4.4375L) ==> [0.437], expected [4.437]
233: wprintf("%.3Lf", 4.4375L) ==> [0.437], expected [4.437]
234: printf("%.3Lf", -4.4375L) ==> [-0.437], expected [-4.437]
234: wprintf("%.3Lf", -4.4375L) ==> [-0.437], expected [-4.437]
239: printf("%.3Lf", 4.4375L) ==> [0.438], expected [4.438]
239: wprintf("%.3Lf", 4.4375L) ==> [0.438], expected [4.438]
240: printf("%.3Lf", -4.4375L) ==> [-0.438], expected [-4.438]
240: wprintf("%.3Lf", -4.4375L) ==> [-0.438], expected [-4.438]
ok 9 - printfloat
273: printf("%La", 0x3.243f6a8885a31p0L) ==> [0x1.921fb54442d18p+1], expected [0xc.90fdaa22168cp-2]
273: wprintf("%La", 0x3.243f6a8885a31p0L) ==> [0x1.921fb54442d18p+1], expected [0xc.90fdaa22168cp-2]
274: printf("%La", 0x1p-1074L) ==> [0x1p-1074], expected [0x8p-1077]
274: wprintf("%La", 0x1p-1074L) ==> [0x1p-1074], expected [0x8p-1077]
275: printf("%La", 0x9.8765p-1024L) ==> [0x1.30ecap-1021], expected [0x9.8765p-1024]
275: wprintf("%La", 0x9.8765p-1024L) ==> [0x1.30ecap-1021], expected [0x9.8765p-1024]
ok 10 - printfloat
ok 11 - printfloat

- Ian


_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arm
To unsubscribe, send any mail to "[hidden email]"

[Ian Lepore]

On Sat, 2012-01-14 at 12:07 -0700, Ian Lepore wrote:

> On Sat, 2012-01-14 at 13:29 -0500, David Schultz wrote:
> > It would be interesting to see the results of the test-printfloat
> > test in regression/lib/libc/stdio.
>
> Well that narrowed it down quickly.  I commented out the abort() calls
> so all the tests would run, and commented out the alternate locale
> testing because we don't install locales on our boxes.  I'm also adding
> -fno-builtin to the test compiles now.  
>
> Everything relating to printing a long double fails.  I don't know yet
> whether it's just printing, or whether all the failures at this point
> are being caused by wrong or incomplete long double support in my
> environment.
>

It does appear to be just printf().  I can do math on long doubles and
pass them as args to other functions and so on and it all works fine and
everything has the expected values as long as I cast the value to double
and printf that way.

While writing the above, another thought popped into my head and I just
tested it: it's not the va_args mechanism corrupting the long doubles,
this code prints the expected values for the long doubles you pass in:

        void foo(int x, ...)
        {
        va_list args;
        long double ld;
       
        va_start(args, x);
        ld = va_arg(args, long double);
        va_end(args);
       
        printf("foo: %f\n", (double)ld);
        }

So something is wrong in the long double printf support in my libc,
although a cursory glance at the code shows that it has some support for
long doubles.  I haven't yet done any repo spelunking to see if there
may be fixes I don't have in the 8.2 lib.

-- Ian


_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arm
To unsubscribe, send any mail to "[hidden email]"

[David Schultz]

On Sat, Jan 14, 2012, Ian Lepore wrote:

> 66: printf("%13LE", 1.0L) ==> [ 0.000000E+00], expected [ 1.000000E+00]
> 66: wprintf("%13LE", 1.0L) ==> [ 0.000000E+00], expected [ 1.000000E+00]
> 67: printf("%13Lf", 1.0L) ==> [     0.000000], expected [     1.000000]
> 67: wprintf("%13Lf", 1.0L) ==> [     0.000000], expected [     1.000000]
> 68: printf("%13LG", 1.0L) ==> [            0], expected [            1]
> 68: wprintf("%13LG", 1.0L) ==> [            0], expected [            1]
> 75: printf("%Le", 1234567.8L) ==> [1.859918e+05], expected [1.234568e+06]
> 75: wprintf("%Le", 1234567.8L) ==> [1.859918e+05], expected [1.234568e+06]
> 76: printf("%Lf", 1234567.8L) ==> [185991.800000], expected [1234567.800000]
> 76: wprintf("%Lf", 1234567.8L) ==> [185991.800000], expected [1234567.800000]
> 77: printf("%LG", 1234567.8L) ==> [185992], expected [1.23457E+06]
> 77: wprintf("%LG", 1234567.8L) ==> [185992], expected [1.23457E+06]

[...]

> 221: printf("%.3Lf", 4.4375L) ==> [0.437], expected [4.437]
> 221: wprintf("%.3Lf", 4.4375L) ==> [0.437], expected [4.437]
> 222: printf("%.3Lf", -4.4375L) ==> [-0.438], expected [-4.438]
> 222: wprintf("%.3Lf", -4.4375L) ==> [-0.438], expected [-4.438]
> 227: printf("%.3Lf", 4.4375L) ==> [0.438], expected [4.438]
> 227: wprintf("%.3Lf", 4.4375L) ==> [0.438], expected [4.438]
> 228: printf("%.3Lf", -4.4375L) ==> [-0.437], expected [-4.437]
> 228: wprintf("%.3Lf", -4.4375L) ==> [-0.437], expected [-4.437]
> 233: printf("%.3Lf", 4.4375L) ==> [0.437], expected [4.437]
> 233: wprintf("%.3Lf", 4.4375L) ==> [0.437], expected [4.437]
> 234: printf("%.3Lf", -4.4375L) ==> [-0.437], expected [-4.437]
> 234: wprintf("%.3Lf", -4.4375L) ==> [-0.437], expected [-4.437]
> 239: printf("%.3Lf", 4.4375L) ==> [0.438], expected [4.438]
> 239: wprintf("%.3Lf", 4.4375L) ==> [0.438], expected [4.438]
> 240: printf("%.3Lf", -4.4375L) ==> [-0.438], expected [-4.438]
> 240: wprintf("%.3Lf", -4.4375L) ==> [-0.438], expected [-4.438]

These issues all indicate something wrong with the way gdtoa is
set up for your chip.  ARM is a bit strange because some versions
use a floating-point format where the high and low words are
reversed from the normal order (in addition to the usual
endianness issues).

Suitable definitions in libc/arm/arith.h (and possibly
libc/arm/_fpmath.h) will probably fix the problem.  Could you try
swapping the sense of the #if in arith.h, and also removing the
Sudden_Underflow #define?  If the definitions are wrong, it might
screw up doubles as well, but it provides some insight into the
problem regardles.  The other place to check is the definition of
_IEEE_WORD_ORDER in libc/arm/_fpmath.h.

> 273: printf("%La", 0x3.243f6a8885a31p0L) ==> [0x1.921fb54442d18p+1], expected [0xc.90fdaa22168cp-2]
> 273: wprintf("%La", 0x3.243f6a8885a31p0L) ==> [0x1.921fb54442d18p+1], expected [0xc.90fdaa22168cp-2]
> 274: printf("%La", 0x1p-1074L) ==> [0x1p-1074], expected [0x8p-1077]
> 274: wprintf("%La", 0x1p-1074L) ==> [0x1p-1074], expected [0x8p-1077]
> 275: printf("%La", 0x9.8765p-1024L) ==> [0x1.30ecap-1021], expected [0x9.8765p-1024]
> 275: wprintf("%La", 0x9.8765p-1024L) ==> [0x1.30ecap-1021], expected [0x9.8765p-1024]

These failures are bugs in the tests.  There are three equally
valid ways to print a number in the hex format, and I changed the
implementation to use a different one a few years ago.  It looks
like I neglected to update the tests that apply to ARM.  Fixed in
r230114.
_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arm
To unsubscribe, send any mail to "[hidden email]"

[Ian Lepore]

On Sat, 2012-01-14 at 16:10 -0500, David Schultz wrote:

> On Sat, Jan 14, 2012, Ian Lepore wrote:
> > 66: printf("%13LE", 1.0L) ==> [ 0.000000E+00], expected [ 1.000000E+00]
> > 66: wprintf("%13LE", 1.0L) ==> [ 0.000000E+00], expected [ 1.000000E+00]
> > 67: printf("%13Lf", 1.0L) ==> [     0.000000], expected [     1.000000]
> > 67: wprintf("%13Lf", 1.0L) ==> [     0.000000], expected [     1.000000]
> > 68: printf("%13LG", 1.0L) ==> [            0], expected [            1]
> > 68: wprintf("%13LG", 1.0L) ==> [            0], expected [            1]
> > 75: printf("%Le", 1234567.8L) ==> [1.859918e+05], expected [1.234568e+06]
> > 75: wprintf("%Le", 1234567.8L) ==> [1.859918e+05], expected [1.234568e+06]
> > 76: printf("%Lf", 1234567.8L) ==> [185991.800000], expected [1234567.800000]
> > 76: wprintf("%Lf", 1234567.8L) ==> [185991.800000], expected [1234567.800000]
> > 77: printf("%LG", 1234567.8L) ==> [185992], expected [1.23457E+06]
> > 77: wprintf("%LG", 1234567.8L) ==> [185992], expected [1.23457E+06]
> [...]
> > 221: printf("%.3Lf", 4.4375L) ==> [0.437], expected [4.437]
> > 221: wprintf("%.3Lf", 4.4375L) ==> [0.437], expected [4.437]
> > 222: printf("%.3Lf", -4.4375L) ==> [-0.438], expected [-4.438]
> > 222: wprintf("%.3Lf", -4.4375L) ==> [-0.438], expected [-4.438]
> > 227: printf("%.3Lf", 4.4375L) ==> [0.438], expected [4.438]
> > 227: wprintf("%.3Lf", 4.4375L) ==> [0.438], expected [4.438]
> > 228: printf("%.3Lf", -4.4375L) ==> [-0.437], expected [-4.437]
> > 228: wprintf("%.3Lf", -4.4375L) ==> [-0.437], expected [-4.437]
> > 233: printf("%.3Lf", 4.4375L) ==> [0.437], expected [4.437]
> > 233: wprintf("%.3Lf", 4.4375L) ==> [0.437], expected [4.437]
> > 234: printf("%.3Lf", -4.4375L) ==> [-0.437], expected [-4.437]
> > 234: wprintf("%.3Lf", -4.4375L) ==> [-0.437], expected [-4.437]
> > 239: printf("%.3Lf", 4.4375L) ==> [0.438], expected [4.438]
> > 239: wprintf("%.3Lf", 4.4375L) ==> [0.438], expected [4.438]
> > 240: printf("%.3Lf", -4.4375L) ==> [-0.438], expected [-4.438]
> > 240: wprintf("%.3Lf", -4.4375L) ==> [-0.438], expected [-4.438]
>
> These issues all indicate something wrong with the way gdtoa is
> set up for your chip.  ARM is a bit strange because some versions
> use a floating-point format where the high and low words are
> reversed from the normal order (in addition to the usual
> endianness issues).
>
> Suitable definitions in libc/arm/arith.h (and possibly
> libc/arm/_fpmath.h) will probably fix the problem.  Could you try
> swapping the sense of the #if in arith.h, and also removing the
> Sudden_Underflow #define?  If the definitions are wrong, it might
> screw up doubles as well, but it provides some insight into the
> problem regardles.  The other place to check is the definition of
> _IEEE_WORD_ORDER in libc/arm/_fpmath.h.
>
> > 273: printf("%La", 0x3.243f6a8885a31p0L) ==> [0x1.921fb54442d18p+1], expected [0xc.90fdaa22168cp-2]
> > 273: wprintf("%La", 0x3.243f6a8885a31p0L) ==> [0x1.921fb54442d18p+1], expected [0xc.90fdaa22168cp-2]
> > 274: printf("%La", 0x1p-1074L) ==> [0x1p-1074], expected [0x8p-1077]
> > 274: wprintf("%La", 0x1p-1074L) ==> [0x1p-1074], expected [0x8p-1077]
> > 275: printf("%La", 0x9.8765p-1024L) ==> [0x1.30ecap-1021], expected [0x9.8765p-1024]
> > 275: wprintf("%La", 0x9.8765p-1024L) ==> [0x1.30ecap-1021], expected [0x9.8765p-1024]
>
> These failures are bugs in the tests.  There are three equally
> valid ways to print a number in the hex format, and I changed the
> implementation to use a different one a few years ago.  It looks
> like I neglected to update the tests that apply to ARM.  Fixed in
> r230114.

Woohoo!  We have success on the long double printf trouble.  I found
that a patch for the problem had been posted to this list almost two
years ago, but apparently never got commited:

http://lists.freebsd.org/pipermail/freebsd-arm/2010-April/002334.html

After applying it and your changes from r230114 the printfloat test now
runs cleanly.  Now I'll go see what the full suite of lib/msun tests
does.

-- Ian


_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arm
To unsubscribe, send any mail to "[hidden email]"

[Ian Lepore]

Here are some intermediate results from the llrint() problem before I
call it a night...

The totally degenerate case to get FE_INVALID set is (long long)1.0;

That is, llrint() calls rint() then casts the result to long long.
rint() doesn't even break a sweat converting 1.0 to 1.0 and returns with
no exception flags set, then the cast from 1.0 to 1LL raises FE_INVALID.

With the original value 1.1 instead of 1.0, rint() itself raises
FE_INVALID, in the path that falls out the bottom.  I added printfs:

        printf("rint 7a: except %#x\n", fetestexcept(FE_ALL_EXCEPT));
        INSERT_WORDS(x,i0,i1);
        printf("rint 7b: except %#x\n", fetestexcept(FE_ALL_EXCEPT));
        STRICT_ASSIGN(double,w,TWO52[sx]+x);
        printf("rint 7c: except %#x\n", fetestexcept(FE_ALL_EXCEPT));
        result = w-TWO52[sx];
        printf("rint 8: except %#x i0=%#x i1=%#x x=%g w=%g result=%g\n", fetestexcept(FE_ALL_EXCEPT), i0, i1, x, w, result);
        return result;

And a run using rint(1.1) gave this output:

        rint 7a: except 0
        rint 7b: except 0
        rint 7c: except 0x10
        rint 8: except 0x10 i0=0x3ff20000 i1=0x9999999a x=1.125 w=4.5036e+15 result=1

I think this may be overall bad news.  It seems to imply that certain
normal operations on doubles can raise FE_INVALID as a side effect while
generating valid results, and because of those words in the spec about
how the rounding routines have to behave as if they're a single
operation, I think that means that each rounding routine has to check
for invalid inputs explicitly and raise FE_INVALID, and otherwise don't
let FE_INVALID from rounding or casting leak out to the caller.  Ick.

Or, maybe the fact that (long long)1.0 raises FE_INVALID is the real
problem, like it's not a normal or expected side effect.  Hmmm, for that
matter, what does the spec say about things like casting raising
exceptions?  I think I'll go fix dinner and leave you to ponder that. :)

-- Ian




_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arm
To unsubscribe, send any mail to "[hidden email]"

[David Schultz]

On Sat, Jan 14, 2012, Ian Lepore wrote:

> With the original value 1.1 instead of 1.0, rint() itself raises
> FE_INVALID, in the path that falls out the bottom.  I added printfs:
>
>         printf("rint 7a: except %#x\n", fetestexcept(FE_ALL_EXCEPT));
>         INSERT_WORDS(x,i0,i1);
>         printf("rint 7b: except %#x\n", fetestexcept(FE_ALL_EXCEPT));
>         STRICT_ASSIGN(double,w,TWO52[sx]+x);
>         printf("rint 7c: except %#x\n", fetestexcept(FE_ALL_EXCEPT));
>         result = w-TWO52[sx];
>         printf("rint 8: except %#x i0=%#x i1=%#x x=%g w=%g result=%g\n", fetestexcept(FE_ALL_EXCEPT), i0, i1, x, w, result);
>         return result;
>
> And a run using rint(1.1) gave this output:
>
>         rint 7a: except 0
>         rint 7b: except 0
>         rint 7c: except 0x10
>         rint 8: except 0x10 i0=0x3ff20000 i1=0x9999999a x=1.125 w=4.5036e+15 result=1

I believe 0x10 is FE_INEXACT on arm, not FE_INVALID.  FE_INEXACT
is correct here, because w+TWO52[sx] is inexact.  That still
doesn't explain the lrint() failures, but it makes them less
mysterious.  For instance, the implementation of the cast from
double to int could raise a bogus inexact exception when the
number is an integer to begin with.  In practice, I think the
emulated casts don't raise the proper exceptions even when they
should.  It might depend on whether it's linking against libgcc's
__fixdfsi() or libc's.

Nice catch with the _fpmath.h bug, by the way.  I'll commit that
fix as well.
_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arm
To unsubscribe, send any mail to "[hidden email]"

[Ian Lepore]

On Sun, 2012-01-15 at 21:26 -0500, David Schultz wrote:

> On Sat, Jan 14, 2012, Ian Lepore wrote:
> I believe 0x10 is FE_INEXACT on arm, not FE_INVALID.  FE_INEXACT
> is correct here, because w+TWO52[sx] is inexact.  That still
> doesn't explain the lrint() failures, but it makes them less
> mysterious.  For instance, the implementation of the cast from
> double to int could raise a bogus inexact exception when the
> number is an integer to begin with.  In practice, I think the
> emulated casts don't raise the proper exceptions even when they
> should.  It might depend on whether it's linking against libgcc's
> __fixdfsi() or libc's.

Doh!  I could've sworn at one point I was seeing FE_INVALID set after
the cast, but now the only thing I'm really sure of is that in jumping
from one theory and way of testing it to another I seem to have
thoroughly confused myself (and probably you too, sorry about that).

So, on theory that there's probably nothing wrong with rint() but maybe
something wrong with casting, I tried a test that does what the guts of
llrint() does, but breaking the casting into a separate step and looking
at the flags after each operation.  I also tried casting to (long)
versus (long long) and the results are different.  Here's the broken
down test code:

        fenv_t env;
        volatile long long llr;
        volatile double dr;
        volatile double _d = 1.0;
       
        feclearexcept(FE_ALL_EXCEPT);
        feholdexcept(&env);
        printf("except 1: %#x\n", fetestexcept(FE_ALL_EXCEPT));
       
        dr = rint(_d);
        printf("except 2: %#x\n", fetestexcept(FE_ALL_EXCEPT));
       
        if (fetestexcept(FE_INVALID))
            feclearexcept(FE_INEXACT);
        printf("except 3: %#x\n", fetestexcept(FE_ALL_EXCEPT));
       
        llr = (long)dr;
        printf("except 4: %#x\n", fetestexcept(FE_ALL_EXCEPT));
       
        llr = (long long)dr;
        printf("except 5: %#x\n", fetestexcept(FE_ALL_EXCEPT));
       
        feupdateenv(&env);
        printf("except 6: %#x _d=%f dr=%f llr=%lld\n", fetestexcept(FE_ALL_EXCEPT), _d, dr, llr);

And here's the output from running it:

        except 1: 0
        except 2: 0
        except 3: 0
        except 4: 0
        except 5: 0x10
        except 6: 0x10 _d=1.000000 dr=1.000000 llr=1

It's interesting that a cast to 32 bits is fine but a cast directly to
64 bits raises FE_INEXACT for the same value.

I did spot-check a few values other than 1.0, and they behaved the same
(IE, 1.0 is not special, it happens with anything.0), and of course with
non-integer starting values FE_INEXACT is set coming out of rint() as
you'd expect.

BTW, I never reposted the results of running all the tests after fixing
the long double printf glitch, but basically it looked like they were
identical to the prior results except that the values were printedly
correctly.  That is, it appears that everything that was failing still
failed, but I'm going to confirm that more rigorously soon (like making
sure I don't have any forgotten hacks/tweaks in any lib or test code).

-- Ian


_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arm
To unsubscribe, send any mail to "[hidden email]"

[David Schultz]

On Mon, Jan 16, 2012, Ian Lepore wrote:

> So, on theory that there's probably nothing wrong with rint() but maybe
> something wrong with casting, I tried a test that does what the guts of
> llrint() does, but breaking the casting into a separate step and looking
> at the flags after each operation.  I also tried casting to (long)
> versus (long long) and the results are different.  Here's the broken
> down test code:
>
>         fenv_t env;
>         volatile long long llr;
>         volatile double dr;
>         volatile double _d = 1.0;
>        
>         feclearexcept(FE_ALL_EXCEPT);
>         feholdexcept(&env);
>         printf("except 1: %#x\n", fetestexcept(FE_ALL_EXCEPT));
>        
>         dr = rint(_d);
>         printf("except 2: %#x\n", fetestexcept(FE_ALL_EXCEPT));
>        
>         if (fetestexcept(FE_INVALID))
>             feclearexcept(FE_INEXACT);
>         printf("except 3: %#x\n", fetestexcept(FE_ALL_EXCEPT));
>        
>         llr = (long)dr;
>         printf("except 4: %#x\n", fetestexcept(FE_ALL_EXCEPT));
>        
>         llr = (long long)dr;
>         printf("except 5: %#x\n", fetestexcept(FE_ALL_EXCEPT));
>        
>         feupdateenv(&env);
>         printf("except 6: %#x _d=%f dr=%f llr=%lld\n", fetestexcept(FE_ALL_EXCEPT), _d, dr, llr);
>
> And here's the output from running it:
>
>         except 1: 0
>         except 2: 0
>         except 3: 0
>         except 4: 0
>         except 5: 0x10
>         except 6: 0x10 _d=1.000000 dr=1.000000 llr=1
>
> It's interesting that a cast to 32 bits is fine but a cast directly to
> 64 bits raises FE_INEXACT for the same value.

The problem is that double->long conversions use the __fixdfsi
function in softfloat, but there's no equivalent double->longlong.
Instead, a semi-bogus __fixdfdi function is provided in libc/quad.
It'll take a little while to come up with a good fix.
_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arm
To unsubscribe, send any mail to "[hidden email]"

[Ian Lepore]

On Mon, 2012-01-16 at 14:51 -0500, David Schultz wrote:
> On Mon, Jan 16, 2012, Ian Lepore wrote:
> The problem is that double->long conversions use the __fixdfsi
> function in softfloat, but there's no equivalent double->longlong.
> Instead, a semi-bogus __fixdfdi function is provided in libc/quad.
> It'll take a little while to come up with a good fix.

Okay, so cast to 64 bits is a known problem.  To see if any other
problems are lurking in this area I redefined assert to be non-fatal in
test-lrint and ran it so we could see the results of the later tests.

It looks like there are two failures (at lines 104 and 108) that aren't
related (at least obviously/directly) to the llrint failures.  I'll
paste the whole log in case my sense of what's "obvious" is wrong...

        1..1
        nfassert: fetestexcept(FE_ALL_EXCEPT) == (0) file test-lrint.c line 96
            while testing llrint: x=1 result=1 excepts=0x10
       
        nfassert: fetestexcept(FE_ALL_EXCEPT) == (0) file test-lrint.c line 96
            while testing llrintf: x=1 result=1 excepts=0x10
       
        nfassert: fetestexcept(FE_ALL_EXCEPT) == (0) file test-lrint.c line 96
            while testing llrintl: x=1 result=1 excepts=0x10
       
        nfassert: fetestexcept(FE_ALL_EXCEPT) == (0) file test-lrint.c line 97
            while testing llrint: x=3.05418e+08 result=305418240 excepts=0x10
       
        nfassert: fetestexcept(FE_ALL_EXCEPT) == (0) file test-lrint.c line 97
            while testing llrintf: x=3.05418e+08 result=305418240 excepts=0x10
       
        nfassert: fetestexcept(FE_ALL_EXCEPT) == (0) file test-lrint.c line 97
            while testing llrintl: x=3.05418e+08 result=305418240 excepts=0x10
       
        nfassert: fetestexcept(FE_ALL_EXCEPT) == (0x0001) file test-lrint.c line 104
            while testing lrint: x=2.14749e+09 result=0 excepts=0x11
       
        nfassert: fetestexcept(FE_ALL_EXCEPT) == (0x0001) file test-lrint.c line 108
            while testing lrint: x=-2.14749e+09 result=0 excepts=0x11
       
        nfassert: (llrint)(_d) == (0) || fetestexcept(FE_INVALID) file test-lrint.c line 136
        nfassert: fetestexcept(FE_ALL_EXCEPT) == (0x0001) file test-lrint.c line 136
            while testing llrint: x=9.22337e+18 result=0 excepts=0
       
        nfassert: (llrintf)(_d) == (0) || fetestexcept(FE_INVALID) file test-lrint.c line 137
        nfassert: fetestexcept(FE_ALL_EXCEPT) == (0x0001) file test-lrint.c line 137
            while testing llrintf: x=9.22337e+18 result=0 excepts=0
       
        nfassert: fetestexcept(FE_ALL_EXCEPT) == (0) file test-lrint.c line 138
            while testing llrint: x=9.22337e+18 result=9223372036854774784 excepts=0x10
       
        nfassert: (llrint)(_d) == (0) || fetestexcept(FE_INVALID) file test-lrint.c line 140
        nfassert: fetestexcept(FE_ALL_EXCEPT) == (0x0001) file test-lrint.c line 140
            while testing llrint: x=-9.22337e+18 result=0 excepts=0x10
       
        nfassert: (llrintf)(_d) == (0) || fetestexcept(FE_INVALID) file test-lrint.c line 141
        nfassert: fetestexcept(FE_ALL_EXCEPT) == (0x0001) file test-lrint.c line 141
            while testing llrintf: x=-9.22337e+18 result=0 excepts=0
       
        ok 1 - lrint
       
       
Zooming in on the two lrint failures, it looks like the rint() routine
raises INEXACT and the cast to long raises INVALID.  I came up with that
by using instrumented code to print the state of the exception flags at
various points within s_rint.c and s_lrint.c

The s_lrint.c instrumentation was a problem, and I figured I'd better
detail it in case it indicates some wider subtle problem.  My goal was
to capture and display the flags after rint(), then clean them out so I
could separately display the flags raised by the cast, then blend the
flags back together so that the flags seen by the caller would be the
same as they would have been without my instrumentation changes:
       
        dtype
        fn(type x)
        {
            fenv_t env;
            volatile dtype d;
            volatile type rx;
       
            feholdexcept(&env);
            rx = roundit(x);
            printf("after rint %#x\n", fetestexcept(FE_ALL_EXCEPT));
            if (fetestexcept(FE_INVALID))
                feclearexcept(FE_INEXACT);
            feupdateenv(&env);
       
            feholdexcept(&env);
            printf("before cast %#x\n", fetestexcept(FE_ALL_EXCEPT));
            d = (dtype)rx;
            printf("after cast  %#x\n", fetestexcept(FE_ALL_EXCEPT));
            feupdateenv(&env);
            printf("after update %#x\n", fetestexcept(FE_ALL_EXCEPT));
            return (d);
        }

Here are the run results (testing just lines 104 and 108, by moving them
into main() making them lines 158 & 160 respectively):

        tflex# ./test-lrint
        1..1
        rint 0: except 0
        rint 1: except 0 i0=0x41dfffff i1=0xffe00000 sx=0 j0=30
        rint 6: except 0 i=0x3fffff j0=0x1e
        rint 8: except 0x10 i0=0x41dfffff i1=0xffe00000 x=2.14749e+09 w=4.50361e+15 result=2.14749e+09
        after rint   0x10
        before cast  0
        after cast   0x1
        after update 0x11
        nfassert: fetestexcept(FE_ALL_EXCEPT) == (0x0001) file test-lrint.c line 158
            while testing lrint: x=2.14749e+09 result=0 excepts=0x11
       
        rint 0: except 0
        rint 1: except 0 i0=0xc1e00000 i1=0x100000 sx=1 j0=31
        rint 6: except 0 i=0x1fffff j0=0x1f
        rint 8: except 0x10 i0=0xc1e00000 i1=0x100000 x=-2.14749e+09 w=-4.50361e+15 result=-2.14749e+09
        after rint   0x10
        before cast  0
        after cast   0x1
        after update 0x11
        nfassert: fetestexcept(FE_ALL_EXCEPT) == (0x0001) file test-lrint.c line 160
            while testing lrint: x=-2.14749e+09 result=0 excepts=0x11
       
Okay, so all of that is what I summarized above.  Here's the problem:

To get that output, I had to change __softfloat_float_exception_flags to
be declared as volatile in all 3 places it's mentioned (two extern decls
and the definition).  I tried just casting to volatile in
fetestexception() and that alone wasn't enough to printf everything
correctly.

Before changing that var to volatile, I got this output from the
instrumented code:

        tflex# ./test-lrint
        1..1
        rint 0: except 0
        rint 1: except 0 i0=0x41dfffff i1=0xffe00000 sx=0 j0=30
        rint 6: except 0 i=0x3fffff j0=0x1e
        rint 8: except 0x10 i0=0x41dfffff i1=0xffe00000 x=2.14748e+09 w=4.5036e+15 result=2.14748e+09
        after rint   0
        before cast  0
        after cast   0x1
        after update 0x11
        nfassert: fetestexcept(FE_ALL_EXCEPT) == (0x0001) file test-lrint.c line 158
            while testing lrint: x=2.14748e+09 result=0 excepts=0x11
       
You can see the insanity there that drove me to try using volatile:
rint() says it has raised INEXACT just before returning, but printing
the flags immediately upon return from rint() showed 0.  But, note that
the flag variable was correct because the final feupdateenv() raised it
again, so the second feholdenv() must have seen it set, right?  

But if you can't print the right values along the way, then the compiler
is somehow feeling free to re-order the code in such a way that the
printfs don't show the right values.  If it could do that in such a way
as to mess up the printfs, could it do so in a way that had more serious
side effects?

Notice also that I declared the intermidate vars 'd' and 'rx' as
volatile.  This was also necessary to prevent re-ordering the code in a
way that ruined more than just printfs.  It was deferring the cast until
the return statement, making all the code trying to print flags and
manipulate the fenv between the cast and the return moot.  Even though
the cast is accomplished with a function call, it's almost as if the
optimizer isn't treating it as a normal function call that can have the
side effect of modifying a global variable.

Note that this last one seems like a real problem, not just "my printfs
aren't right".  If I add feclearexcept(FE_INVALID) right before the last
feupdateenv(), then when the cast raises FE_INVALID it should get
cleared and not be seen by the caller of lrint().  But because the cast
gets deferred until the return, the caller does see FE_INVALID:

        tflex# ./test-lrint
        1..1
        rint 0: except 0
        rint 1: except 0 i0=0x41dfffff i1=0xffe00000 sx=0 j0=30
        rint 6: except 0 i=0x3fffff j0=0x1e
        rint 8: except 0x10 i0=0x41dfffff i1=0xffe00000 x=2.14748e+09 w=4.5036e+15 result=2.14748e+09
        after rint   0x10
        before cast  0
        after cast   0
        after update 0x10
        nfassert: fetestexcept(FE_ALL_EXCEPT) == (0x0001) file test-lrint.c line 158
            while testing lrint: x=2.14748e+09 result=0 excepts=0x11

Note that without 'd' being declared volatile the exception flags are
0x11 after the call even though the 0x01 flag was cleared before
returning from lrint() (the global flags var was volatile for this run;
from the printfs it looks like it was never set, but remember that's
because the optimizer relocated the cast to the point of the return).

This may all come down to a big steaming pile of compiler buggage that
you can't do anything about, but I figured I'd dump out everything I've
seen so that if you get future reports of odd behavior you're at least
forearmed with some knowledge of the ways wonky things can happen.

-- Ian



_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arm
To unsubscribe, send any mail to "[hidden email]"