Merge pull request #705 from cppalliance/comp_64

Better 64 bit comparisons
cppalliance · Jul 11, 2024 · 80872c3 · 80872c3
2 parents c115908 + 0d04df2
commit 80872c3
Show file tree

Hide file tree

Showing 12 changed files with 119 additions and 60 deletions.
diff --git a/fuzzing/old_crashes/fuzz_snprintf/crash-b064ecb574301d4db076a40a163c97e7b3493730 b/fuzzing/old_crashes/fuzz_snprintf/crash-b064ecb574301d4db076a40a163c97e7b3493730
diff --git a/fuzzing/old_crashes/fuzz_snprintf/crash-cabfe2d6422408ec1acebc5b61bb755b140c2d7a b/fuzzing/old_crashes/fuzz_snprintf/crash-cabfe2d6422408ec1acebc5b61bb755b140c2d7a
diff --git a/fuzzing/old_crashes/fuzz_snprintf/crash-e89fafed26debffb24aaa20cfcc7a08ad61ac5ff b/fuzzing/old_crashes/fuzz_snprintf/crash-e89fafed26debffb24aaa20cfcc7a08ad61ac5ff
@@ -0,0 +1 @@
+e1000a00000000000000000000p06
diff --git a/fuzzing/old_crashes/fuzz_snprintf/crash-fb2d3435b218b7000ccf2e76b3d2e820800f51aa b/fuzzing/old_crashes/fuzz_snprintf/crash-fb2d3435b218b7000ccf2e76b3d2e820800f51aa
@@ -0,0 +1 @@
+50000100000000000000078591790822
diff --git a/include/boost/decimal/charconv.hpp b/include/boost/decimal/charconv.hpp
@@ -615,7 +615,7 @@ BOOST_DECIMAL_CONSTEXPR auto to_chars_hex_impl(char* first, char* last, const Ta
 
     int exp {};
     Unsigned_Integer significand = frexp10(value, &exp);
-
+    BOOST_DECIMAL_ASSERT(significand != 0);
     // Strip zeros of the significand since frexp10 normalizes it
     while (significand % 10U == 0)
     {

diff --git a/include/boost/decimal/decimal32.hpp b/include/boost/decimal/decimal32.hpp
@@ -1056,10 +1056,12 @@ constexpr auto decimal32::operator-=(Integer rhs) noexcept
 
 constexpr auto operator==(decimal32 lhs, decimal32 rhs) noexcept -> bool
 {
+    #ifndef BOOST_DECIMAL_FAST_MATH
     if (isnan(lhs) || isnan(rhs))
     {
         return false;
     }
+    #endif
 
     return equal_parts_impl(lhs.full_significand(), lhs.biased_exponent(), lhs.isneg(),
                             rhs.full_significand(), rhs.biased_exponent(), rhs.isneg());

diff --git a/include/boost/decimal/decimal32_fast.hpp b/include/boost/decimal/decimal32_fast.hpp
@@ -519,22 +519,17 @@ constexpr auto operator<(decimal32_fast lhs, decimal32_fast rhs) noexcept -> boo
             return signbit(rhs);
         }
     }
-    #else
-    if (!lhs.isneg() && rhs.isneg())
-    {
-        return false;
-    }
-    else if (lhs.isneg() && !rhs.isneg())
-    {
-        return true;
-    }
     #endif
 
     if (lhs.significand_ == 0 || rhs.significand_ == 0)
     {
         if (lhs.significand_ == 0 && rhs.significand_ == 0)
         {
+            #ifndef BOOST_DECIMAL_FAST_MATH
+            return lhs.sign_ && !rhs.sign_;
+            #else
             return false;
+            #endif
         }
         return lhs.significand_ == 0 ? !rhs.sign_ : lhs.sign_;
     }

diff --git a/include/boost/decimal/decimal64.hpp b/include/boost/decimal/decimal64.hpp
@@ -1670,27 +1670,21 @@ constexpr auto operator!=(Integer lhs, decimal64 rhs) noexcept
 constexpr auto operator<(decimal64 lhs, decimal64 rhs) noexcept -> bool
 {
     #ifndef BOOST_DECIMAL_FAST_MATH
-    if (isnan(lhs) || isnan(rhs) ||
-        (!lhs.isneg() && rhs.isneg()))
-    {
-        return false;
-    }
-    else if (lhs.isneg() && !rhs.isneg())
-    {
-        return true;
-    }
-    else if (isfinite(lhs) && isinf(rhs))
-    {
-        return !rhs.isneg();
-    }
-    #else
-    if (!lhs.isneg() && rhs.isneg())
-    {
-        return false;
-    }
-    else if (lhs.isneg() && !rhs.isneg())
+    if (not_finite(lhs) || not_finite(rhs))
     {
-        return true;
+        if (isnan(lhs) || isnan(rhs) ||
+            (!lhs.isneg() && rhs.isneg()))
+        {
+            return false;
+        }
+        else if (lhs.isneg() && !rhs.isneg())
+        {
+            return true;
+        }
+        else if (isfinite(lhs) && isinf(rhs))
+        {
+            return !rhs.isneg();
+        }
     }
     #endif
 

diff --git a/include/boost/decimal/decimal64_fast.hpp b/include/boost/decimal/decimal64_fast.hpp
@@ -538,37 +538,45 @@ constexpr auto operator!=(Integer lhs, decimal64_fast rhs) noexcept
 constexpr auto operator<(decimal64_fast lhs, decimal64_fast rhs) noexcept -> bool
 {
     #ifndef BOOST_DECIMAL_FAST_MATH
-    if (isnan(lhs) || isnan(rhs))
+    if (not_finite(lhs) || not_finite(rhs))
     {
-        return false;
+        if (isnan(lhs) || isnan(rhs) ||
+            (!lhs.isneg() && rhs.isneg()))
+        {
+            return false;
+        }
+        else if (lhs.isneg() && !rhs.isneg())
+        {
+            return true;
+        }
+        else if (isfinite(lhs) && isinf(rhs))
+        {
+            return !signbit(rhs);
+        }
+        else if (isinf(lhs) && isfinite(rhs))
+        {
+            return signbit(rhs);
+        }
     }
     #endif
 
-    if (!lhs.isneg() && rhs.isneg())
-    {
-        return false;
-    }
-
-    if (lhs.isneg() && !rhs.isneg())
-    {
-        return true;
-    }
-
-    #ifndef BOOST_DECIMAL_FAST_MATH
-    if (isfinite(lhs) && isinf(rhs))
-    {
-        return !signbit(rhs);
-    }
-
-    if (isinf(lhs) && isfinite(rhs))
+    // Needed to correctly compare signed and unsigned zeros
+    if (lhs.significand_ == 0 || rhs.significand_ == 0)
     {
-        return signbit(rhs);
+        if (lhs.significand_ == 0 && rhs.significand_ == 0)
+        {
+            #ifndef BOOST_DECIMAL_FAST_MATH
+            return lhs.sign_ && !rhs.sign_;
+            #else
+            return false;
+            #endif
+        }
+        return lhs.significand_ == 0 ? !rhs.sign_ : lhs.sign_;
     }
-    #endif
 
-    if (lhs.significand_ == 0 || rhs.significand_ == 0)
+    if (lhs.sign_ != rhs.sign_)
     {
-        return lhs.significand_ == 0 ? !rhs.sign_ : lhs.sign_;
+        return lhs.sign_;
     }
 
     if (lhs.exponent_ != rhs.exponent_)

diff --git a/include/boost/decimal/detail/comparison.hpp b/include/boost/decimal/detail/comparison.hpp
@@ -27,9 +27,9 @@ namespace decimal {
 template <BOOST_DECIMAL_DECIMAL_FLOATING_TYPE DecimalType = decimal32, BOOST_DECIMAL_INTEGRAL T1,
           BOOST_DECIMAL_INTEGRAL U1, BOOST_DECIMAL_INTEGRAL T2, BOOST_DECIMAL_INTEGRAL U2>
 constexpr auto equal_parts_impl(T1 lhs_sig, U1 lhs_exp, bool lhs_sign,
-                                T2 rhs_sig, U2 rhs_exp, bool rhs_sign) noexcept -> std::enable_if_t<std::is_same<DecimalType, decimal32>::value, bool>
+                                T2 rhs_sig, U2 rhs_exp, bool rhs_sign) noexcept -> std::enable_if_t<std::is_same<DecimalType, decimal32>::value || std::is_same<DecimalType, decimal64>::value, bool>
 {
-    using comp_type = std::uint_fast64_t;
+    using comp_type = std::conditional_t<std::is_same<DecimalType, decimal32>::value, std::uint_fast32_t, std::uint_fast64_t>;
 
     BOOST_DECIMAL_ASSERT(lhs_sig >= 0);
     BOOST_DECIMAL_ASSERT(rhs_sig >= 0);
@@ -46,7 +46,9 @@ constexpr auto equal_parts_impl(T1 lhs_sig, U1 lhs_exp, bool lhs_sign,
     const auto delta_exp {lhs_exp - rhs_exp};
 
     // Check the value of delta exp to avoid to large a value for pow10
-    if (delta_exp > detail::precision_v<DecimalType> || delta_exp < -detail::precision_v<DecimalType>)
+    // Also if only one of the significands is 0 then we know the values have to be mismatched
+    if (delta_exp > detail::precision_v<DecimalType> || delta_exp < -detail::precision_v<DecimalType> ||
+        ((new_lhs_sig == static_cast<comp_type>(0)) ^ (new_rhs_sig == static_cast<comp_type>(0))))
     {
         return false;
     }
@@ -74,7 +76,7 @@ constexpr auto equal_parts_impl(T1 lhs_sig, U1 lhs_exp, bool lhs_sign,
 template <BOOST_DECIMAL_DECIMAL_FLOATING_TYPE DecimalType = decimal32, BOOST_DECIMAL_INTEGRAL T1,
           BOOST_DECIMAL_INTEGRAL U1, BOOST_DECIMAL_INTEGRAL T2, BOOST_DECIMAL_INTEGRAL U2>
 constexpr auto equal_parts_impl(T1 lhs_sig, U1 lhs_exp, bool lhs_sign,
-                                T2 rhs_sig, U2 rhs_exp, bool rhs_sign) noexcept -> std::enable_if_t<!std::is_same<DecimalType, decimal32>::value, bool>
+                                T2 rhs_sig, U2 rhs_exp, bool rhs_sign) noexcept -> std::enable_if_t<!(std::is_same<DecimalType, decimal32>::value || std::is_same<DecimalType, decimal64>::value), bool>
 {
     using comp_type = std::conditional_t<(std::numeric_limits<T1>::digits10 > std::numeric_limits<T2>::digits10), T1, T2>;
 
@@ -106,10 +108,12 @@ constexpr auto mixed_equality_impl(Decimal lhs, Integer rhs) noexcept
 {
     using exp_type = typename Decimal::biased_exponent_type;
 
+    #ifndef BOOST_DECIMAL_FAST_MATH
     if (isnan(lhs) || isinf(lhs))
     {
         return false;
     }
+    #endif
 
     bool rhs_isneg {false};
     BOOST_DECIMAL_IF_CONSTEXPR (detail::is_signed_v<Integer>)
@@ -134,10 +138,12 @@ constexpr auto mixed_decimal_equality_impl(Decimal1 lhs, Decimal2 rhs) noexcept
     static_assert(!std::is_same<Decimal1, Decimal2>::value, "Equality of same type exists in simpler form");
     using Bigger_Decimal_Type = std::conditional_t<(sizeof(lhs) > sizeof(rhs)), Decimal1, Decimal2>;
 
+    #ifndef BOOST_DECIMAL_FAST_MATH
     if (isnan(lhs) || isnan(rhs))
     {
         return false;
     }
+    #endif
 
     const auto new_lhs = to_decimal<Bigger_Decimal_Type>(lhs);
     const auto new_rhs = to_decimal<Bigger_Decimal_Type>(rhs);
@@ -164,7 +170,7 @@ constexpr auto operator!=(Decimal1 lhs, Decimal2 rhs) noexcept
 template <BOOST_DECIMAL_DECIMAL_FLOATING_TYPE DecimalType = decimal32, BOOST_DECIMAL_INTEGRAL T1,
         BOOST_DECIMAL_INTEGRAL U1, BOOST_DECIMAL_INTEGRAL T2, BOOST_DECIMAL_INTEGRAL U2>
 constexpr auto less_parts_impl(T1 lhs_sig, U1 lhs_exp, bool lhs_sign,
-                               T2 rhs_sig, U2 rhs_exp, bool rhs_sign) noexcept -> std::enable_if_t<std::is_same<DecimalType, decimal32>::value, bool>
+                               T2 rhs_sig, U2 rhs_exp, bool rhs_sign) noexcept -> std::enable_if_t<std::is_same<DecimalType, decimal32>::value || std::is_same<DecimalType, decimal64>::value, bool>
 {
     using comp_type = std::uint_fast64_t;
 
@@ -198,7 +204,7 @@ constexpr auto less_parts_impl(T1 lhs_sig, U1 lhs_exp, bool lhs_sign,
 template <BOOST_DECIMAL_DECIMAL_FLOATING_TYPE DecimalType = decimal32, BOOST_DECIMAL_INTEGRAL T1,
           BOOST_DECIMAL_INTEGRAL U1, BOOST_DECIMAL_INTEGRAL T2, BOOST_DECIMAL_INTEGRAL U2>
 constexpr auto less_parts_impl(T1 lhs_sig, U1 lhs_exp, bool lhs_sign,
-                               T2 rhs_sig, U2 rhs_exp, bool rhs_sign) noexcept -> std::enable_if_t<!std::is_same<DecimalType, decimal32>::value, bool>
+                               T2 rhs_sig, U2 rhs_exp, bool rhs_sign) noexcept -> std::enable_if_t<!(std::is_same<DecimalType, decimal32>::value || std::is_same<DecimalType, decimal64>::value), bool>
 {
     using comp_type = std::conditional_t<(std::numeric_limits<T1>::digits10 > std::numeric_limits<T2>::digits10), T1, T2>;
 
@@ -239,6 +245,7 @@ constexpr auto less_impl(Decimal lhs, Integer rhs) noexcept
 {
     using exp_type = typename Decimal::biased_exponent_type;
 
+    #ifndef BOOST_DECIMAL_FAST_MATH
     if (isnan(lhs))
     {
         return false;
@@ -247,6 +254,7 @@ constexpr auto less_impl(Decimal lhs, Integer rhs) noexcept
     {
         return lhs.isneg();
     }
+    #endif
 
     bool lhs_sign {lhs.isneg()};
     bool rhs_sign {false};
@@ -288,14 +296,21 @@ constexpr auto mixed_decimal_less_impl(Decimal1 lhs, Decimal2 rhs) noexcept
 {
     using Bigger_Decimal_Type = std::conditional_t<(sizeof(lhs) > sizeof(rhs)), Decimal1, Decimal2>;
 
-    if (isnan(lhs) || isnan(rhs) || (!lhs.isneg() && rhs.isneg()))
+
+    if (
+            #ifndef BOOST_DECIMAL_FAST_MATH
+            isnan(lhs) || isnan(rhs) ||
+            #endif
+            (!lhs.isneg() && rhs.isneg())
+        )
     {
         return false;
     }
     else if (lhs.isneg() && !rhs.isneg())
     {
         return true;
     }
+    #ifndef BOOST_DECIMAL_FAST_MATH
     else if (boost::decimal::isfinite(lhs) && isinf(rhs))
     {
         if (!signbit(rhs))
@@ -307,6 +322,7 @@ constexpr auto mixed_decimal_less_impl(Decimal1 lhs, Decimal2 rhs) noexcept
             return false;
         }
     }
+    #endif
 
     return less_parts_impl<Bigger_Decimal_Type>(lhs.full_significand(), lhs.biased_exponent(), lhs.isneg(),
                                                 rhs.full_significand(), rhs.biased_exponent(), rhs.isneg());
@@ -325,10 +341,12 @@ constexpr auto operator<=(Decimal1 lhs, Decimal2 rhs) noexcept
     -> std::enable_if_t<(detail::is_decimal_floating_point_v<Decimal1> &&
                          detail::is_decimal_floating_point_v<Decimal2>), bool>
 {
+    #ifndef BOOST_DECIMAL_FAST_MATH
     if (isnan(lhs) || isnan(rhs))
     {
         return false;
     }
+    #endif
 
     return !(rhs < lhs);
 }
@@ -346,10 +364,12 @@ constexpr auto operator>=(Decimal1 lhs, Decimal2 rhs) noexcept
     -> std::enable_if_t<(detail::is_decimal_floating_point_v<Decimal1> &&
                          detail::is_decimal_floating_point_v<Decimal2>), bool>
 {
+    #ifndef BOOST_DECIMAL_FAST_MATH
     if (isnan(lhs) || isnan(rhs))
     {
         return false;
     }
+    #endif
 
     return !(lhs < rhs);
 }

diff --git a/test/test_cmath.cpp b/test/test_cmath.cpp
@@ -1286,6 +1286,17 @@ void test_log10()
     }
 }
 
+template <typename T>
+void test_fpclassify()
+{
+    std::uniform_int_distribution<int> dist(1, 2);
+    BOOST_TEST_EQ(fpclassify(T(1) * dist(rng)), FP_NORMAL);
+    BOOST_TEST_EQ(fpclassify(std::numeric_limits<T>::infinity() * dist(rng)), FP_INFINITE);
+    BOOST_TEST_EQ(fpclassify(std::numeric_limits<T>::quiet_NaN() * dist(rng)), FP_NAN);
+    BOOST_TEST_EQ(fpclassify(T{0} * dist(rng)), FP_ZERO);
+    BOOST_TEST_EQ(fpclassify(-T{0} * dist(rng)), FP_ZERO);
+}
+
 int main()
 {
     test_fmax<decimal32>();
@@ -1529,5 +1540,13 @@ int main()
     test_log10<decimal128_fast>();
     #endif
 
+    test_fpclassify<decimal32>();
+    test_fpclassify<decimal64>();
+    test_fpclassify<decimal128>();
+
+    test_fpclassify<decimal32_fast>();
+    test_fpclassify<decimal64_fast>();
+    test_fpclassify<decimal128_fast>();
+
     return boost::report_errors();
 }
diff --git a/test/test_to_chars.cpp b/test/test_to_chars.cpp
@@ -42,6 +42,19 @@ void test_value(T val, const char* result)
     BOOST_TEST_CSTR_EQ(result, buffer);
 }
 
+template <typename T>
+void test_error_value(const char* input, chars_format format, int precision = -1)
+{
+    T val;
+    const auto r_from = from_chars(input, input + std::strlen(input), val, format);
+    BOOST_TEST(r_from);
+    std::cerr << "Val: " << val
+              << "\nIsneg: " << signbit(val) << std::endl;
+    char buffer[boost::decimal::limits<T>::max_chars] {};
+    const auto r_to = to_chars(buffer, buffer + sizeof(buffer), val, format, precision);
+    BOOST_TEST(r_to);
+}
+
 template <typename T>
 void test_non_finite_values()
 {
@@ -648,6 +661,12 @@ int main()
     // See: https://github.com/cppalliance/decimal/issues/478
     test_value(std::numeric_limits<decimal32>::epsilon(), "1e-07");
 
+    // Value found from fuzzing
+    for (int precision = -1; precision < 10; ++precision)
+    {
+        test_error_value<decimal64>("e1000a00000000000000000000p06", chars_format::hex, precision);
+    }
+
     return boost::report_errors();
 }