From 9e8c3caef4fb5a73361d40778eac609ecb4e03b6 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Tue, 2 Jun 2026 15:35:42 -0400 Subject: [PATCH] Improve accuracy of Number::operator+= - Use more of the available range of the uint128 operands. - Also refactor Number::Guard::round() to return an enum. --- src/libxrpl/basics/Number.cpp | 38 ++++++++++------- src/test/basics/Number_test.cpp | 75 ++++++++++++++++++++------------- 2 files changed, 67 insertions(+), 46 deletions(-) diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 23be06475b..8e3887e475 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -206,10 +206,16 @@ public: void doDropDigit(T& mantissa, int& exponent) noexcept; + enum class Round { + Down = -1, + Even = 0, + Up = 1, + }; + // Indicate round direction: 1 is up, -1 is down, 0 is even // This enables the client to round towards nearest, and on // tie, round towards even. - [[nodiscard]] int + [[nodiscard]] Round round() const noexcept; // Modify the result to the correctly rounded value @@ -314,41 +320,41 @@ Number::Guard::doDropDigit(uint128_t& mantissa, int& exponent) noexce // -1 if Guard is less than half // 0 if Guard is exactly half // 1 if Guard is greater than half -int +Number::Guard::Round Number::Guard::round() const noexcept { auto mode = Number::getround(); if (mode == RoundingMode::TowardsZero) - return -1; + return Round::Down; if (mode == RoundingMode::Downward) { if (sbit_) { if (digits_ > 0 || xbit_) - return 1; + return Round::Up; } - return -1; + return Round::Down; } if (mode == RoundingMode::Upward) { if (sbit_) - return -1; + return Round::Down; if (digits_ > 0 || xbit_) - return 1; - return -1; + return Round::Up; + return Round::Down; } // assume round to nearest if mode is not one of the predefined values if (digits_ > 0x5000'0000'0000'0000) - return 1; + return Round::Up; if (digits_ < 0x5000'0000'0000'0000) - return -1; + return Round::Down; if (xbit_) - return 1; - return 0; + return Round::Up; + return Round::Even; } template @@ -388,7 +394,7 @@ Number::Guard::doRoundUp( std::string location) { auto r = round(); - if (r == 1 || (r == 0 && (mantissa & 1) == 1)) + if (r == Round::Up || (r == Round::Even && (mantissa & 1) == 1)) { auto const safeToIncrement = [&maxMantissa](auto const& mantissa) { return mantissa < maxMantissa && mantissa < kMaxRep; @@ -454,7 +460,7 @@ Number::Guard::doRoundDown( internalrep const& minMantissa) { auto r = round(); - if (r == 1 || (r == 0 && (mantissa & 1) == 1)) + if (r == Round::Up || (r == Round::Even && (mantissa & 1) == 1)) { --mantissa; if (mantissa < minMantissa) @@ -471,7 +477,7 @@ void Number::Guard::doRound(rep& drops, std::string location) const { auto r = round(); - if (r == 1 || (r == 0 && (drops & 1) == 1)) + if (r == Round::Up || (r == Round::Even && (drops & 1) == 1)) { if (drops >= kMaxRep) { @@ -817,7 +823,7 @@ Number::operator+=(Number const& y) --xe; } g.doRoundDown(xn, xm, xe, minMantissa); - if (range.cuspRoundingFixEnabled == MantissaRange::CuspRoundingFix::Enabled) + if (range.cuspRoundingFixEnabled == MantissaRange::CuspRoundingFix::Enabled && xm != 0) { // make a new guard Guard g; diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index 620ea798bc..034b6b1983 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -179,33 +179,34 @@ public: testcase << "test_add " << to_string(scale); using Case = std::tuple; - auto const cSmall = std::to_array( - {{Number{1'000'000'000'000'000, -15}, - Number{6'555'555'555'555'555, -29}, - Number{1'000'000'000'000'066, -15}, - __LINE__}, - {Number{-1'000'000'000'000'000, -15}, - Number{-6'555'555'555'555'555, -29}, - Number{-1'000'000'000'000'066, -15}, - __LINE__}, - {Number{-1'000'000'000'000'000, -15}, - Number{6'555'555'555'555'555, -29}, - Number{-9'999'999'999'999'344, -16}, - __LINE__}, - {Number{-6'555'555'555'555'555, -29}, - Number{1'000'000'000'000'000, -15}, - Number{9'999'999'999'999'344, -16}, - __LINE__}, - {Number{}, Number{5}, Number{5}, __LINE__}, - {Number{5}, Number{}, Number{5}, __LINE__}, - {Number{5'555'555'555'555'555, -32768}, - Number{-5'555'555'555'555'554, -32768}, - Number{0}, - __LINE__}, - {Number{-9'999'999'999'999'999, -31}, - Number{1'000'000'000'000'000, -15}, - Number{9'999'999'999'999'990, -16}, - __LINE__}}); + auto const cSmall = std::to_array({ + {Number{1'000'000'000'000'000, -15}, + Number{6'555'555'555'555'555, -29}, + Number{1'000'000'000'000'066, -15}, + __LINE__}, + {Number{-1'000'000'000'000'000, -15}, + Number{-6'555'555'555'555'555, -29}, + Number{-1'000'000'000'000'066, -15}, + __LINE__}, + {Number{-1'000'000'000'000'000, -15}, + Number{6'555'555'555'555'555, -29}, + Number{-9'999'999'999'999'344, -16}, + __LINE__}, + {Number{-6'555'555'555'555'555, -29}, + Number{1'000'000'000'000'000, -15}, + Number{9'999'999'999'999'344, -16}, + __LINE__}, + {Number{}, Number{5}, Number{5}, __LINE__}, + {Number{5}, Number{}, Number{5}, __LINE__}, + {Number{5'555'555'555'555'555, -32768}, + Number{-5'555'555'555'555'554, -32768}, + Number{0}, + __LINE__}, + {Number{-9'999'999'999'999'999, -31}, + Number{1'000'000'000'000'000, -15}, + Number{9'999'999'999'999'990, -16}, + __LINE__}, + }); auto const cLarge = std::to_array( // Note that items with extremely large mantissas need to be // calculated, because otherwise they overflow uint64. Items from C @@ -2021,13 +2022,27 @@ public: BigInt const exact = toBigInt(a) + toBigInt(b); BigInt const stored = toBigInt(sum); + BigInt const diff = stored - exact; - log << "\n exact a + b = " << exact.str() << "\n TowardsZero = " << stored.str() - << "\n"; + log << "\n a = " << a << "\n b = " << b + << "\n exact a + b = " << exact.str() << "\n TowardsZero = " << stored.str() + << "\n difference = " << diff.str() << "\n"; log.flush(); - if (scale != MantissaRange::MantissaScale::LargeLegacy) + if (scale == MantissaRange::MantissaScale::Small) + { BEAST_EXPECT(stored == exact); + } + else if (scale == MantissaRange::MantissaScale::LargeLegacy) + { + BEAST_EXPECT(stored > exact); + } + else + { + BEAST_EXPECT(stored < exact); + BEAST_EXPECT(diff < 0); + BEAST_EXPECT(-diff < pow10(sum.exponent())); + } } }