From fcfe008db25ef14fdb4dc0e1620bd03c0065b840 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 29 Jul 2024 16:00:34 -0400 Subject: [PATCH] feefrac fuzz: use arith_uint256 instead of ad-hoc multiply Rather than use an ad-hoc reimplementation of wide multiplication inside the fuzz test, reuse arith_uint256, which already has this. It's larger than what we need here, but performance isn't a concern in this test, and it does what we need. --- src/test/fuzz/feefrac.cpp | 67 +++++++++++---------------------------- 1 file changed, 19 insertions(+), 48 deletions(-) diff --git a/src/test/fuzz/feefrac.cpp b/src/test/fuzz/feefrac.cpp index 2c7553360e6..6363eb530eb 100644 --- a/src/test/fuzz/feefrac.cpp +++ b/src/test/fuzz/feefrac.cpp @@ -2,6 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include #include #include #include @@ -13,43 +14,22 @@ namespace { -/** Compute a * b, represented in 4x32 bits, highest limb first. */ -std::array Mul128(uint64_t a, uint64_t b) +/** The maximum absolute value of an int64_t, as an arith_uint256 (2^63). */ +const auto MAX_ABS_INT64 = arith_uint256{1} << 63; + +/** Construct an arith_uint256 whose value equals abs(x). */ +arith_uint256 Abs256(int64_t x) { - std::array ret{0, 0, 0, 0}; - - /** Perform ret += v << (32 * pos), at 128-bit precision. */ - auto add_fn = [&](uint64_t v, int pos) { - uint64_t accum{0}; - for (int i = 0; i + pos < 4; ++i) { - // Add current value at limb pos in ret. - accum += ret[3 - pos - i]; - // Add low or high half of v. - if (i == 0) accum += v & 0xffffffff; - if (i == 1) accum += v >> 32; - // Store lower half of result in limb pos in ret. - ret[3 - pos - i] = accum & 0xffffffff; - // Leave carry in accum. - accum >>= 32; - } - // Make sure no overflow. - assert(accum == 0); - }; - - // Multiply the 4 individual limbs (schoolbook multiply, with base 2^32). - add_fn((a & 0xffffffff) * (b & 0xffffffff), 0); - add_fn((a >> 32) * (b & 0xffffffff), 1); - add_fn((a & 0xffffffff) * (b >> 32), 1); - add_fn((a >> 32) * (b >> 32), 2); - return ret; -} - -/* comparison helper for std::array */ -std::strong_ordering compare_arrays(const std::array& a, const std::array& b) { - for (size_t i = 0; i < a.size(); ++i) { - if (a[i] != b[i]) return a[i] <=> b[i]; + if (x >= 0) { + // For positive numbers, pass through the value. + return arith_uint256{static_cast(x)}; + } else if (x > std::numeric_limits::min()) { + // For negative numbers, negate first. + return arith_uint256{static_cast(-x)}; + } else { + // Special case for x == -2^63 (for which -x results in integer overflow). + return MAX_ABS_INT64; } - return std::strong_ordering::equal; } std::strong_ordering MulCompare(int64_t a1, int64_t a2, int64_t b1, int64_t b2) @@ -59,23 +39,14 @@ std::strong_ordering MulCompare(int64_t a1, int64_t a2, int64_t b1, int64_t b2) int sign_b = (b1 == 0 ? 0 : b1 < 0 ? -1 : 1) * (b2 == 0 ? 0 : b2 < 0 ? -1 : 1); if (sign_a != sign_b) return sign_a <=> sign_b; - // Compute absolute values. - uint64_t abs_a1 = static_cast(a1), abs_a2 = static_cast(a2); - uint64_t abs_b1 = static_cast(b1), abs_b2 = static_cast(b2); - // Use (~x + 1) instead of the equivalent (-x) to silence the linter; mod 2^64 behavior is - // intentional here. - if (a1 < 0) abs_a1 = ~abs_a1 + 1; - if (a2 < 0) abs_a2 = ~abs_a2 + 1; - if (b1 < 0) abs_b1 = ~abs_b1 + 1; - if (b2 < 0) abs_b2 = ~abs_b2 + 1; + // Compute absolute values of products. + auto mul_abs_a = Abs256(a1) * Abs256(a2), mul_abs_b = Abs256(b1) * Abs256(b2); // Compute products of absolute values. - auto mul_abs_a = Mul128(abs_a1, abs_a2); - auto mul_abs_b = Mul128(abs_b1, abs_b2); if (sign_a < 0) { - return compare_arrays(mul_abs_b, mul_abs_a); + return mul_abs_b <=> mul_abs_a; } else { - return compare_arrays(mul_abs_a, mul_abs_b); + return mul_abs_a <=> mul_abs_b; } }