bug-gnulib
[Top][All Lists]
Advanced

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

signed-nan: Don't assume that '-' works as expected on NaN values


From: Bruno Haible
Subject: signed-nan: Don't assume that '-' works as expected on NaN values
Date: Fri, 19 Apr 2024 14:05:40 +0200

On an OpenBSD 7.4/mips64 machine, I'm seeing these test failures:

FAIL: test-signbit
FAIL: test-totalorder
FAIL: test-totalorderf

More details about the 'test-signbit' failure: Each of these assertions fails.

  ASSERT (signbit (negative_NaNf ()));
  ASSERT (signbit (negative_SNaNf ()));
  ASSERT (signbit (negative_NaNd ()));
  ASSERT (signbit (negative_SNaNd ()));

Debugging the negative_NaNf function, I see that where I had written

  - nan

the compiler (clang 13) generates a 'neg.s' instruction. And likewise in the
negative_NaNd function, a 'neg.d' instruction. In the MIPS instruction set [1],
these instructions may be a no-op on NaNs, depending on FPU control flags.

The patch below fixes the failures, by introducing a specific function, rather
than unary minus, for negating the sign of NaNs.

[1] "MIPSĀ® Architecture for Progreammers, Volume II-A: The MIPSĀ® Instruction 
Set Manual",
    page 307 (317)


2024-04-19  Bruno Haible  <bruno@clisp.org>

        signed-nan: Don't assume that '-' works as expected on NaN values.
        * lib/signed-nan.h (minus_NaNf): New function.
        (positive_NaNf, negative_NaNf): Use it.
        (minus_NaNd): New function.
        (positive_NaNd, negative_NaNd): Use it.
        (minus_NaNl): New function.
        (positive_NaNl, negative_NaNl): Use it.
        * tests/test-totalorder.c (TOTALORDER_MINUS): New macro.
        * tests/test-totalorderf.c (TOTALORDER_MINUS): New macro.
        * tests/test-totalorderl.c (TOTALORDER_MINUS): New macro.
        * tests/test-totalorder.h (negative_NaN_with_payload): Use it.
        * tests/test-totalordermag.c (TOTALORDER_MINUS): New macro.
        * tests/test-totalordermagf.c (TOTALORDER_MINUS): New macro.
        * tests/test-totalordermagl.c (TOTALORDER_MINUS): New macro.
        * tests/test-totalordermag.h (negative_NaN_with_payload): Use it.

diff --git a/lib/signed-nan.h b/lib/signed-nan.h
index 14d2c525f5..c2c2def719 100644
--- a/lib/signed-nan.h
+++ b/lib/signed-nan.h
@@ -26,6 +26,23 @@ extern "C" {
 #endif
 
 
+/* Returns - x, implemented by inverting the sign bit,
+   so that it works also on 'float' NaN values.  */
+_GL_UNUSED static float
+minus_NaNf (float x)
+{
+#if defined __mips__
+  /* The mips instruction neg.s may have no effect on NaNs.
+     Therefore, invert the sign bit using integer operations.  */
+  union { unsigned int i; float value; } u;
+  u.value = x;
+  u.i ^= 1U << 31;
+  return u.value;
+#else
+  return - x;
+#endif
+}
+
 /* Returns a quiet 'float' NaN with sign bit == 0.  */
 _GL_UNUSED static float
 positive_NaNf ()
@@ -33,7 +50,7 @@ positive_NaNf ()
   /* 'volatile' works around a GCC bug:
      <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111655>  */
   float volatile nan = NaNf ();
-  return (signbit (nan) ? - nan : nan);
+  return (signbit (nan) ? minus_NaNf (nan) : nan);
 }
 
 /* Returns a quiet 'float' NaN with sign bit == 1.  */
@@ -43,10 +60,27 @@ negative_NaNf ()
   /* 'volatile' works around a GCC bug:
      <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111655>  */
   float volatile nan = NaNf ();
-  return (signbit (nan) ? nan : - nan);
+  return (signbit (nan) ? nan : minus_NaNf (nan));
 }
 
 
+/* Returns - x, implemented by inverting the sign bit,
+   so that it works also on 'double' NaN values.  */
+_GL_UNUSED static double
+minus_NaNd (double x)
+{
+#if defined __mips__
+  /* The mips instruction neg.d may have no effect on NaNs.
+     Therefore, invert the sign bit using integer operations.  */
+  union { unsigned long long i; double value; } u;
+  u.value = x;
+  u.i ^= 1ULL << 63;
+  return u.value;
+#else
+  return - x;
+#endif
+}
+
 /* Returns a quiet 'double' NaN with sign bit == 0.  */
 _GL_UNUSED static double
 positive_NaNd ()
@@ -54,7 +88,7 @@ positive_NaNd ()
   /* 'volatile' works around a GCC bug:
      <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111655>  */
   double volatile nan = NaNd ();
-  return (signbit (nan) ? - nan : nan);
+  return (signbit (nan) ? minus_NaNd (nan) : nan);
 }
 
 /* Returns a quiet 'double' NaN with sign bit == 1.  */
@@ -64,10 +98,18 @@ negative_NaNd ()
   /* 'volatile' works around a GCC bug:
      <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111655>  */
   double volatile nan = NaNd ();
-  return (signbit (nan) ? nan : - nan);
+  return (signbit (nan) ? nan : minus_NaNd (nan));
 }
 
 
+/* Returns - x, implemented by inverting the sign bit,
+   so that it works also on 'long double' NaN values.  */
+_GL_UNUSED static long double
+minus_NaNl (long double x)
+{
+  return - x;
+}
+
 /* Returns a quiet 'long double' NaN with sign bit == 0.  */
 _GL_UNUSED static long double
 positive_NaNl ()
@@ -75,7 +117,7 @@ positive_NaNl ()
   /* 'volatile' works around a GCC bug:
      <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111655>  */
   long double volatile nan = NaNl ();
-  return (signbit (nan) ? - nan : nan);
+  return (signbit (nan) ? minus_NaNl (nan) : nan);
 }
 
 /* Returns a quiet 'long double' NaN with sign bit == 1.  */
@@ -85,7 +127,7 @@ negative_NaNl ()
   /* 'volatile' works around a GCC bug:
      <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111655>  */
   long double volatile nan = NaNl ();
-  return (signbit (nan) ? nan : - nan);
+  return (signbit (nan) ? nan : minus_NaNl (nan));
 }
 
 
diff --git a/tests/test-totalorder.c b/tests/test-totalorder.c
index 412c3d301e..2112b49af9 100644
--- a/tests/test-totalorder.c
+++ b/tests/test-totalorder.c
@@ -26,6 +26,7 @@ SIGNATURE_CHECK (totalorder, int, (const double *, const 
double *));
 #define TOTALORDER_TYPE memory_double
 #define TOTALORDER_INF Infinityd
 #define TOTALORDER_MINUS_ZERO minus_zerod
+#define TOTALORDER_MINUS minus_NaNd
 #define TOTALORDER_SETPAYLOAD setpayload
 #define TOTALORDER_HAVE_SNAN HAVE_SNAND
 #define TOTALORDER_POSITIVE_SNAN memory_positive_SNaNd
diff --git a/tests/test-totalorder.h b/tests/test-totalorder.h
index b02cec310b..e675bd334f 100644
--- a/tests/test-totalorder.h
+++ b/tests/test-totalorder.h
@@ -36,7 +36,7 @@ negative_NaN_with_payload (int payload)
 {
   TOTALORDER_TYPE x;
   ASSERT (TOTALORDER_SETPAYLOAD (&x.value, payload) == 0);
-  x.value = - x.value;
+  x.value = TOTALORDER_MINUS (x.value);
   return x;
 }
 
diff --git a/tests/test-totalorderf.c b/tests/test-totalorderf.c
index a3dd23a296..a7223feaf6 100644
--- a/tests/test-totalorderf.c
+++ b/tests/test-totalorderf.c
@@ -26,6 +26,7 @@ SIGNATURE_CHECK (totalorderf, int, (const float *, const 
float *));
 #define TOTALORDER_TYPE memory_float
 #define TOTALORDER_INF Infinityf
 #define TOTALORDER_MINUS_ZERO minus_zerof
+#define TOTALORDER_MINUS minus_NaNf
 #define TOTALORDER_SETPAYLOAD setpayloadf
 #define TOTALORDER_HAVE_SNAN HAVE_SNANF
 #define TOTALORDER_POSITIVE_SNAN memory_positive_SNaNf
diff --git a/tests/test-totalorderl.c b/tests/test-totalorderl.c
index f3db8194f7..79bb7d9ab8 100644
--- a/tests/test-totalorderl.c
+++ b/tests/test-totalorderl.c
@@ -26,6 +26,7 @@ SIGNATURE_CHECK (totalorderl, int, (const long double *, 
const long double *));
 #define TOTALORDER_TYPE memory_long_double
 #define TOTALORDER_INF Infinityl
 #define TOTALORDER_MINUS_ZERO minus_zerol
+#define TOTALORDER_MINUS minus_NaNl
 #define TOTALORDER_SETPAYLOAD setpayloadl
 #define TOTALORDER_HAVE_SNAN HAVE_SNANL
 #define TOTALORDER_POSITIVE_SNAN memory_positive_SNaNl
diff --git a/tests/test-totalordermag.c b/tests/test-totalordermag.c
index 5b2c4c70d9..7ac2498a50 100644
--- a/tests/test-totalordermag.c
+++ b/tests/test-totalordermag.c
@@ -26,6 +26,7 @@ SIGNATURE_CHECK (totalordermag, int, (const double *, const 
double *));
 #define TOTALORDER_TYPE memory_double
 #define TOTALORDER_INF Infinityd
 #define TOTALORDER_MINUS_ZERO minus_zerod
+#define TOTALORDER_MINUS minus_NaNd
 #define TOTALORDER_SETPAYLOAD setpayload
 #define TOTALORDER_HAVE_SNAN HAVE_SNAND
 #define TOTALORDER_POSITIVE_SNAN memory_positive_SNaNd
diff --git a/tests/test-totalordermag.h b/tests/test-totalordermag.h
index 4d4746fae1..ec86d55c27 100644
--- a/tests/test-totalordermag.h
+++ b/tests/test-totalordermag.h
@@ -36,7 +36,7 @@ negative_NaN_with_payload (int payload)
 {
   TOTALORDER_TYPE x;
   ASSERT (TOTALORDER_SETPAYLOAD (&x.value, payload) == 0);
-  x.value = - x.value;
+  x.value = TOTALORDER_MINUS (x.value);
   return x;
 }
 
diff --git a/tests/test-totalordermagf.c b/tests/test-totalordermagf.c
index b6ada9d59f..01d1ab0b12 100644
--- a/tests/test-totalordermagf.c
+++ b/tests/test-totalordermagf.c
@@ -26,6 +26,7 @@ SIGNATURE_CHECK (totalordermagf, int, (const float *, const 
float *));
 #define TOTALORDER_TYPE memory_float
 #define TOTALORDER_INF Infinityf
 #define TOTALORDER_MINUS_ZERO minus_zerof
+#define TOTALORDER_MINUS minus_NaNf
 #define TOTALORDER_SETPAYLOAD setpayloadf
 #define TOTALORDER_HAVE_SNAN HAVE_SNANF
 #define TOTALORDER_POSITIVE_SNAN memory_positive_SNaNf
diff --git a/tests/test-totalordermagl.c b/tests/test-totalordermagl.c
index 1d33404405..34e8a102b5 100644
--- a/tests/test-totalordermagl.c
+++ b/tests/test-totalordermagl.c
@@ -26,6 +26,7 @@ SIGNATURE_CHECK (totalordermagl, int, (const long double *, 
const long double *)
 #define TOTALORDER_TYPE memory_long_double
 #define TOTALORDER_INF Infinityl
 #define TOTALORDER_MINUS_ZERO minus_zerol
+#define TOTALORDER_MINUS minus_NaNl
 #define TOTALORDER_SETPAYLOAD setpayloadl
 #define TOTALORDER_HAVE_SNAN HAVE_SNANL
 #define TOTALORDER_POSITIVE_SNAN memory_positive_SNaNl






reply via email to

[Prev in Thread] Current Thread [Next in Thread]