Fix regression in succf()/predf() (#140)

And add more thorough test. Signed-off-by: Cary Phillips <[email protected]>
AcademySoftwareFoundation · May 7, 2021 · 6ddc829 · 6ddc829
1 parent 1c8a010
commit 6ddc829
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 23 deletions.
diff --git a/src/Imath/ImathFun.cpp b/src/Imath/ImathFun.cpp
@@ -27,7 +27,7 @@ succf (float f) noexcept
 
         u.i = 0x00000001;
     }
-    else if (u.i > 0)
+    else if (u.f > 0)
     {
         // Positive float, normalized or denormalized.
         // Incrementing the largest positive float
@@ -65,7 +65,7 @@ predf (float f) noexcept
 
         u.i = 0x80000001;
     }
-    else if (u.i > 0)
+    else if (u.f > 0)
     {
         // Positive float, normalized or denormalized.
 

diff --git a/src/ImathTest/testFun.cpp b/src/ImathTest/testFun.cpp
@@ -8,6 +8,10 @@
 #endif
 
 #include "ImathFun.h"
+#if __cplusplus >= 202002L
+#    include <bit>
+#endif
+#include <iostream>
 #include <assert.h>
 #include <iostream>
 #include <stdio.h>
@@ -18,16 +22,32 @@ using namespace std;
 #if ULONG_MAX == 18446744073709551615LU
 typedef long unsigned int Int64;
 #else
-typedef long long unsigned int Int64;
+    typedef long long unsigned int Int64;
+#endif
+
+#if __cplusplus < 202002L
+    template <typename To, typename From>
+    static inline To
+    bit_cast (From from)
+    {
+        static_assert (sizeof (From) == sizeof (To), "Type sizes do not match");
+        union
+        {
+            From f;
+            To   t;
+        } u;
+        u.f = from;
+        return u.t;
+    }
 #endif
 
 void
-testf (float f)
+testf (float f, bool changeExpected = true)
 {
     printf ("\n");
 
-    float sf  = IMATH_INTERNAL_NAMESPACE::succf (f);
-    float pf  = IMATH_INTERNAL_NAMESPACE::predf (f);
+    float sf = IMATH_INTERNAL_NAMESPACE::succf (f);
+    float pf = IMATH_INTERNAL_NAMESPACE::predf (f);
     float spf = IMATH_INTERNAL_NAMESPACE::succf (IMATH_INTERNAL_NAMESPACE::predf (f));
     float psf = IMATH_INTERNAL_NAMESPACE::predf (IMATH_INTERNAL_NAMESPACE::succf (f));
 
@@ -36,15 +56,29 @@ testf (float f)
     printf ("pf %.9g\n", pf);
     printf ("spf %.9g\n", spf);
     printf ("psf %.9g\n", psf);
+
+    fflush (stdout);
+
+    if (changeExpected)
+    {
+        assert (pf < f);
+        assert (f < sf);
+    }
+    else
+    {
+        // No bit change expected if input was inf or NaN
+        assert (bit_cast<unsigned> (pf) == bit_cast<unsigned> (f));
+        assert (bit_cast<unsigned> (sf) == bit_cast<unsigned> (f));
+    }
 }
 
 void
-testd (double d)
+testd (double d, bool changeExpected = true)
 {
     printf ("\n");
 
-    double sd  = IMATH_INTERNAL_NAMESPACE::succd (d);
-    double pd  = IMATH_INTERNAL_NAMESPACE::predd (d);
+    double sd = IMATH_INTERNAL_NAMESPACE::succd (d);
+    double pd = IMATH_INTERNAL_NAMESPACE::predd (d);
     double spd = IMATH_INTERNAL_NAMESPACE::succd (IMATH_INTERNAL_NAMESPACE::predd (d));
     double psd = IMATH_INTERNAL_NAMESPACE::predd (IMATH_INTERNAL_NAMESPACE::succd (d));
 
@@ -53,6 +87,20 @@ testd (double d)
     printf ("pd %.18lg\n", pd);
     printf ("spd %.18lg\n", spd);
     printf ("psd %.18lg\n", psd);
+
+    fflush (stdout);
+
+    if (changeExpected)
+    {
+        assert (pd < d);
+        assert (d < sd);
+    }
+    else
+    {
+        // No bit change expected if input was inf or NaN
+        assert (bit_cast<Int64> (pd) == bit_cast<Int64> (d));
+        assert (bit_cast<Int64> (sd) == bit_cast<Int64> (d));
+    }
 }
 
 void
@@ -196,15 +244,13 @@ testFun()
     testf (7);
     testf (0.7);
 
-    union
-    {
-        float f;
-        int i;
-    } u;
+    union {float f; int i;} u;
     u.i = 0x7f800000; //  inf
-    testf (u.f);
+    testf (u.f, false);
+    u.i = 0xff800000; // -inf
+    testf (u.f, false);
     u.i = 0x7f800001; //  nan
-    testf (u.f);
+    testf (u.f, false);
     u.i = 0x7f7fffff; //  FLT_MAX
     testf (u.f);
     u.i = 0xff7fffff; // -FLT_MAX
@@ -218,15 +264,13 @@ testFun()
     testd (7);
     testd (0.7);
 
-    union
-    {
-        double d;
-        Int64 i;
-    } v;
+    union {double d; Int64 i;} v;
     v.i = 0x7ff0000000000000ULL; //  inf
-    testd (v.d);
+    testd (v.d, false);
+    v.i = 0xfff0000000000000ULL; // -inf
+    testd (v.d, false);
     v.i = 0x7ff0000000000001ULL; //  NAN
-    testd (v.d);
+    testd (v.d, false);
     v.i = 0x7fefffffffffffffULL; //  FLT_MAX
     testd (v.d);
     v.i = 0xffefffffffffffffULL; // -FLT_MAX