Merge pull request #910 from cppalliance/improve_comp_bench

Improve benchmarks and minor fixes

Author: mborland

include/boost/decimal/detail/comparison.hpp

@@ -26,6 +26,11 @@
 namespace boost {
 namespace decimal {
 
+#ifdef _MSC_VER
+#pragma warning(push)
+#pragma warning(disable:4127)
+#endif
+
 template <BOOST_DECIMAL_DECIMAL_FLOATING_TYPE DecimalType>
 BOOST_DECIMAL_FORCE_INLINE constexpr auto equality_impl(DecimalType lhs, DecimalType rhs) noexcept -> bool
 {
@@ -75,75 +80,109 @@ BOOST_DECIMAL_FORCE_INLINE constexpr auto equality_impl(DecimalType lhs, Decimal
     }
 
     // Step 6: Normalize the significand and compare
-    // Instead of multiplying the larger number, divide the smaller one
-    if (delta_exp >= 0)
+    BOOST_DECIMAL_IF_CONSTEXPR (detail::decimal_val_v<DecimalType> >= 128)
     {
-        // Check if we can divide rhs_sig safely
-        if (delta_exp > 0 && rhs_sig % detail::pow10(static_cast<comp_type>(delta_exp)) != 0U)
+        // Instead of multiplying the larger number, divide the smaller one
+        //
+        // We try for multiplication even though it's a small range
+        // Since it's an order of magnitude faster
+        if (delta_exp <= 4 && delta_exp >= 4)
         {
-            return false;
+            if (delta_exp > 0)
+            {
+                lhs_sig *= detail::pow10(static_cast<comp_type>(delta_exp));
+            }
+            else if (delta_exp < 0)
+            {
+                rhs_sig *= detail::pow10(static_cast<comp_type>(-delta_exp));
+            }
         }
-        rhs_sig /= detail::pow10(static_cast<comp_type>(delta_exp));
+        else if (delta_exp > 0)
+        {
+            // Check if we can divide rhs_sig safely
+            // E.g. 9e0 != 90000000204928e-13 but if we just did division we would falsely get 9 ?= 9
+            if (rhs_sig % detail::pow10(static_cast<comp_type>(delta_exp)) != 0U)
+            {
+                return false;
+            }
+            rhs_sig /= detail::pow10(static_cast<comp_type>(delta_exp));
+        }
+        else if (delta_exp < 0)
+        {
+            // Check if we can divide lhs_sig safely
+            if (lhs_sig % detail::pow10(static_cast<comp_type>(-delta_exp)) != 0U)
+            {
+                return false;
+            }
+            lhs_sig /= detail::pow10(static_cast<comp_type>(-delta_exp));
+        }
+
+        return lhs_sig == rhs_sig;
     }
     else
     {
-        // Check if we can divide lhs_sig safely
-        if (lhs_sig % detail::pow10(static_cast<comp_type>(-delta_exp)) != 0U)
+        using promoted_sig_type = std::conditional_t<std::is_same<typename DecimalType::significand_type, std::uint32_t>::value, std::uint64_t, int128::uint128_t>;
+        promoted_sig_type promotes_lhs {lhs_sig};
+        promoted_sig_type promotes_rhs {rhs_sig};
+
+        if (delta_exp > 0)
         {
-            return false;
+            promotes_lhs *= detail::pow10(static_cast<promoted_sig_type>(delta_exp));
+        }
+        else if (delta_exp < 0)
+        {
+            promotes_rhs *= detail::pow10(static_cast<promoted_sig_type>(-delta_exp));
         }
-        lhs_sig /= detail::pow10(static_cast<comp_type>(-delta_exp));
-    }
 
-    return lhs_sig == rhs_sig;
+        return promotes_lhs == promotes_rhs;
+    }
 }
 
+#ifdef _MSC_VER
+#pragma warning(pop)
+#endif
+
 template <BOOST_DECIMAL_FAST_DECIMAL_FLOATING_TYPE DecimalType>
 BOOST_DECIMAL_FORCE_INLINE constexpr auto fast_equality_impl(const DecimalType& lhs, const DecimalType& rhs) noexcept -> bool
 {
-    if (lhs.significand_ == 0U && rhs.significand_ == 0U)
-    {
-        // -0 == +0
-        return true;
-    }
-
-    if (lhs.exponent_ != rhs.exponent_)
-    {
-        return false;
-    }
-    if (lhs.significand_ != rhs.significand_)
+    #ifndef BOOST_DECIMAL_FAST_MATH
+    if (isnan(lhs) || isnan(rhs))
     {
         return false;
     }
+    #endif
 
-    #ifndef BOOST_DECIMAL_FAST_MATH
-    if (isnan(lhs))
+    const auto lhs_sig {lhs.significand_};
+    const auto rhs_sig {rhs.significand_};
+
+    if (lhs_sig == 0U && rhs_sig == 0U)
     {
-        return false;
+        // -0 == +0
+        return true;
     }
-    #endif
 
-    return lhs.sign_ == rhs.sign_;
+    return  lhs_sig == rhs_sig &&
+            lhs.exponent_ == rhs.exponent_ &&
+            lhs.sign_ == rhs.sign_;
 }
 
 template <BOOST_DECIMAL_FAST_DECIMAL_FLOATING_TYPE DecimalType>
 BOOST_DECIMAL_FORCE_INLINE constexpr auto fast_inequality_impl(const DecimalType& lhs, const DecimalType& rhs) noexcept -> bool
 {
-    return
-            #ifndef BOOST_DECIMAL_FAST_MATH
-            isnan(lhs) || isnan(rhs) ||
-            #endif
-            (lhs.sign_ != rhs.sign_) ||
-            (lhs.exponent_ != rhs.exponent_) ||
-            (lhs.significand_ != rhs.significand_);
+    return !fast_equality_impl(lhs, rhs);
 }
 
+#ifdef _MSC_VER
+#pragma warning(push)
+#pragma warning(disable:4127) // BOOST_DECIMAL_IF_CONSTEXPR prior to C++17 is constant conditional expression
+#endif
+
 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<detail::is_ieee_type_v<DecimalType>, bool>
 {
-    using comp_type = std::conditional_t<(std::numeric_limits<T1>::digits10 > std::numeric_limits<T2>::digits10), T1, T2>;
+    using comp_type = detail::make_unsigned_t<std::conditional_t<(std::numeric_limits<T1>::digits10 > std::numeric_limits<T2>::digits10), T1, T2>>;
 
     BOOST_DECIMAL_ASSERT(lhs_sig >= 0U);
     BOOST_DECIMAL_ASSERT(rhs_sig >= 0U);
@@ -154,14 +193,11 @@ constexpr auto equal_parts_impl(T1 lhs_sig, U1 lhs_exp, bool lhs_sign,
         return false;
     }
 
-    auto new_lhs_sig {static_cast<comp_type>(lhs_sig)};
-    auto new_rhs_sig {static_cast<comp_type>(rhs_sig)};
-
     const auto delta_exp {lhs_exp - rhs_exp};
 
     // Check the value of delta exp to avoid to large a value for pow10
     // Also if only one of the significands is 0 then we know the values have to be mismatched
-    if (new_lhs_sig == static_cast<comp_type>(0) && new_rhs_sig == static_cast<comp_type>(0))
+    if (lhs_sig == 0U && rhs_sig == 0U)
     {
         return true;
     }
@@ -170,38 +206,73 @@ constexpr auto equal_parts_impl(T1 lhs_sig, U1 lhs_exp, bool lhs_sign,
         return false;
     }
 
+    auto new_lhs_sig {static_cast<comp_type>(lhs_sig)};
+    auto new_rhs_sig {static_cast<comp_type>(rhs_sig)};
+
     // Step 5: Normalize the significand and compare
     // Instead of multiplying the larger number, divide the smaller one
-    if (delta_exp >= 0)
+    BOOST_DECIMAL_IF_CONSTEXPR (detail::decimal_val_v<DecimalType> >= 128)
     {
-        // Check if we can divide rhs_sig safely
-        if (delta_exp > 0 && new_rhs_sig % detail::pow10(static_cast<comp_type>(delta_exp)) != 0U)
+        // Instead of multiplying the larger number, divide the smaller one
+        //
+        // We try for multiplication even though it's a small range
+        // Since it's an order of magnitude faster
+        if (delta_exp <= 4 && delta_exp >= 4)
         {
-            return false;
+            if (delta_exp > 0)
+            {
+                new_lhs_sig *= detail::pow10(static_cast<comp_type>(delta_exp));
+            }
+            else if (delta_exp < 0)
+            {
+                new_rhs_sig *= detail::pow10(static_cast<comp_type>(-delta_exp));
+            }
+        }
+        else if (delta_exp > 0)
+        {
+            // Check if we can divide rhs_sig safely
+            // E.g. 9e0 != 90000000204928e-13 but if we just did division we would falsely get 9 ?= 9
+            if (new_rhs_sig % detail::pow10(static_cast<comp_type>(delta_exp)) != 0U)
+            {
+                return false;
+            }
+            new_rhs_sig /= detail::pow10(static_cast<comp_type>(delta_exp));
         }
-        new_rhs_sig /= detail::pow10(static_cast<comp_type>(delta_exp));
+        else if (delta_exp < 0)
+        {
+            // Check if we can divide lhs_sig safely
+            if (new_lhs_sig % detail::pow10(static_cast<comp_type>(-delta_exp)) != 0U)
+            {
+                return false;
+            }
+            new_lhs_sig /= detail::pow10(static_cast<comp_type>(-delta_exp));
+        }
+
+        return new_lhs_sig == new_rhs_sig;
     }
     else
     {
-        // Check if we can divide lhs_sig safely
-        if (new_lhs_sig % detail::pow10(static_cast<comp_type>(-delta_exp)) != 0U)
+        using promoted_sig_type = std::conditional_t<std::is_same<comp_type, std::uint32_t>::value, std::uint64_t, int128::uint128_t>;
+        promoted_sig_type promotes_lhs {new_lhs_sig};
+        promoted_sig_type promotes_rhs {new_rhs_sig};
+
+        if (delta_exp > 0)
         {
-            return false;
+            promotes_lhs *= detail::pow10(static_cast<promoted_sig_type>(delta_exp));
+        }
+        else if (delta_exp < 0)
+        {
+            promotes_rhs *= detail::pow10(static_cast<promoted_sig_type>(-delta_exp));
         }
-        new_lhs_sig /= detail::pow10(static_cast<comp_type>(-delta_exp));
-    }
-
-    #ifdef BOOST_DECIMAL_DEBUG_EQUAL
-    std::cerr << "Normalized Values"
-              << "\nlhs_sig: " << new_lhs_sig
-              << "\nlhs_exp: " << lhs_exp
-              << "\nrhs_sig: " << new_rhs_sig
-              << "\nrhs_exp: " << rhs_exp << std::endl;
-    #endif
 
-    return new_lhs_sig == new_rhs_sig;
+        return promotes_lhs == promotes_rhs;
+    }
 }
 
+#ifdef _MSC_VER
+#pragma warning(pop)
+#endif
+
 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,

include/boost/decimal/detail/mul_impl.hpp

@@ -193,38 +193,22 @@ template <typename ReturnType, BOOST_DECIMAL_INTEGRAL T1, BOOST_DECIMAL_INTEGRAL
 constexpr auto d128_mul_impl(T1 lhs_sig, U1 lhs_exp, bool lhs_sign,
                              T2 rhs_sig, U2 rhs_exp, bool rhs_sign) noexcept -> ReturnType
 {
-    bool sign {lhs_sign != rhs_sign};
+    const bool sign {lhs_sign != rhs_sign};
 
-    const auto lhs_dig {detail::num_digits(lhs_sig)};
-    const auto rhs_dig {detail::num_digits(rhs_sig)};
+    auto res_sig {detail::umul256(lhs_sig, rhs_sig)};
+    auto res_exp {lhs_exp + rhs_exp};
 
-    // TODO(mborland): Benchmark this since counting digits is a horribly slow operation
-    // If we can avoid it don't do 256 bit multiplication because it is slow
-    if (lhs_dig + rhs_dig <= std::numeric_limits<boost::int128::uint128_t>::digits10)
+    const auto sig_dig {detail::num_digits(res_sig)};
+
+    if (sig_dig > std::numeric_limits<boost::int128::uint128_t>::digits10)
     {
-        auto res_sig {lhs_sig * rhs_sig};
-        auto res_exp {lhs_exp + rhs_exp};
-        return {res_sig, res_exp, sign};
+        const auto digit_delta {sig_dig - std::numeric_limits<boost::int128::uint128_t>::digits10};
+        res_sig /= pow10(int128::uint128_t(digit_delta));
+        res_exp += digit_delta;
     }
-    else
-    {
-        // Once we have the normalized significands and exponents all we have to do is
-        // multiply the significands and add the exponents
-        auto res_sig {detail::umul256(lhs_sig, rhs_sig)};
-        auto res_exp {lhs_exp + rhs_exp};
-
-        const auto sig_dig {detail::num_digits(res_sig)};
 
-        if (sig_dig > std::numeric_limits<boost::int128::uint128_t>::digits10)
-        {
-            const auto digit_delta {sig_dig - std::numeric_limits<boost::int128::uint128_t>::digits10};
-            res_sig /= pow10(int128::uint128_t(digit_delta));
-            res_exp += digit_delta;
-        }
-
-        BOOST_DECIMAL_ASSERT((res_sig[3] | res_sig[2]) == 0U);
-        return {int128::uint128_t{res_sig[1], res_sig[0]}, res_exp, sign};
-    }
+    BOOST_DECIMAL_ASSERT((res_sig[3] | res_sig[2]) == 0U);
+    return {int128::uint128_t{res_sig[1], res_sig[0]}, res_exp, sign};
 }
 
 template <typename ReturnType, typename T>

test/CMakeLists.txt

@@ -10,10 +10,10 @@ if(HAVE_BOOST_TEST)
     find_package(fmt)
     if(fmt_FOUND)
         message(STATUS "Boost.Decimal: Test with fmtlib")
-        boost_test_jamfile(FILE Jamfile LINK_LIBRARIES Boost::decimal Boost::core Boost::math Boost::multiprecision Boost::charconv fmt::fmt COMPILE_DEFINITIONS BOOST_DECIMAL_TEST_FMT)
+        boost_test_jamfile(FILE Jamfile LINK_LIBRARIES Boost::decimal Boost::core Boost::math Boost::multiprecision Boost::charconv Boost::random fmt::fmt COMPILE_DEFINITIONS BOOST_DECIMAL_TEST_FMT)
     else()
         message(STATUS "Boost.Decimal: Test without fmtlib")
-        boost_test_jamfile(FILE Jamfile LINK_LIBRARIES Boost::decimal Boost::core Boost::math Boost::multiprecision Boost::charconv)
+        boost_test_jamfile(FILE Jamfile LINK_LIBRARIES Boost::decimal Boost::core Boost::math Boost::multiprecision Boost::charconv Boost::random)
     endif()
 
 endif()