gnucash master: Multiple changes pushed
John Ralls
jralls at code.gnucash.org
Sat Apr 8 12:10:33 EDT 2017
Updated via https://github.com/Gnucash/gnucash/commit/2035806d (commit)
via https://github.com/Gnucash/gnucash/commit/fd6234f5 (commit)
via https://github.com/Gnucash/gnucash/commit/a467d0d3 (commit)
via https://github.com/Gnucash/gnucash/commit/de1c56b5 (commit)
via https://github.com/Gnucash/gnucash/commit/e55b7861 (commit)
from https://github.com/Gnucash/gnucash/commit/6e7334fe (commit)
commit 2035806db7bb33c6e8e9e641d84b2386424f6658
Author: John Ralls <jralls at ceridwen.us>
Date: Fri Apr 7 12:39:47 2017 -0700
Some instrumentation output for how much the numerator or denominator is shifted.
diff --git a/src/libqof/qof/gnc-rational.cpp b/src/libqof/qof/gnc-rational.cpp
index e409b84..1e0f7a1 100644
--- a/src/libqof/qof/gnc-rational.cpp
+++ b/src/libqof/qof/gnc-rational.cpp
@@ -215,6 +215,7 @@ GncRational::round_to_numeric() const
--ll_bits;
}
}
+ std::cout << "Rounded with " << ll_bits << " bits.\n";
return new_v;
}
auto quot(m_den / m_num);
@@ -236,6 +237,7 @@ GncRational::round_to_numeric() const
continue;
}
GncRational new_rational(num, den);
+ std::cout << "Divisor converted with " << ll_bits << "bits.\n";
return new_rational;
}
new_v = convert<RoundType::half_down>(m_den / divisor);
@@ -245,6 +247,7 @@ GncRational::round_to_numeric() const
new_v = GncRational();
}
}
+ std::cout << "Divisor rounded with " << ll_bits << "bits.\n";
return new_v;
}
commit fd6234f58fd436b6c31aa3b4f0f98aa008ea5aea
Author: John Ralls <jralls at ceridwen.us>
Date: Fri Apr 7 12:38:55 2017 -0700
Better manage truncation in GncRational::round_to_numeric.
Mike Alexander brought this up with a test case that failed to round down
sufficiently; he found that reducing the rounding denominator by 2 sufficed
to make his test case pass.
In fact the sizing of the replacement denominator by shifting the larger of
the numerator or denominator by an arbitrary 62 bits was not correct most
of the time, so instead we begin with a shift of the full lower leg worth,
try to do the conversion, and if the conversion is still “big†shift the
larger value one more and try the operation again, repeating until the
result will fit in a GncNumeric.
diff --git a/src/libqof/qof/gnc-rational.cpp b/src/libqof/qof/gnc-rational.cpp
index e5878a9..e409b84 100644
--- a/src/libqof/qof/gnc-rational.cpp
+++ b/src/libqof/qof/gnc-rational.cpp
@@ -149,6 +149,8 @@ GncRational::prepare_conversion (GncInt128 new_denom) const
auto red_conv = conversion.reduce();
GncInt128 old_num(m_num);
auto new_num = old_num * red_conv.num();
+ if (new_num.isOverflow())
+ throw std::overflow_error("Conversion overflow");
auto rem = new_num % red_conv.denom();
new_num /= red_conv.denom();
return {new_num, red_conv.denom(), rem};
@@ -181,6 +183,7 @@ GncRational::reduce() const
GncRational
GncRational::round_to_numeric() const
{
+ unsigned int ll_bits = GncInt128::legbits;
if (m_num.isZero())
return GncRational(); //Default constructor makes 0/1
if (!(m_num.isBig() || m_den.isBig()))
@@ -195,25 +198,53 @@ GncRational::round_to_numeric() const
<< "GncNumeric. Its integer value is too large.\n";
throw std::overflow_error(msg.str());
}
- GncRational new_v(*this);
- new_v = new_v.convert<RoundType::half_down>(m_den / (m_num.abs() >> 62));
+ GncRational new_v;
+ while (new_v.num().isZero())
+ {
+ try
+ {
+ new_v = convert<RoundType::half_down>(m_den / (m_num.abs() >> ll_bits));
+ if (new_v.is_big())
+ {
+ --ll_bits;
+ new_v = GncRational();
+ }
+ }
+ catch(const std::overflow_error& err)
+ {
+ --ll_bits;
+ }
+ }
return new_v;
}
auto quot(m_den / m_num);
if (quot.isBig())
return GncRational(); //Smaller than can be represented as a GncNumeric
- auto divisor = m_den >> 62;
- if (m_num.isBig())
+ GncRational new_v;
+ while (new_v.num().isZero())
{
- GncInt128 oldnum(m_num), num, rem;
- oldnum.div(divisor, num, rem);
- auto den = m_den / divisor;
- num += rem * 2 >= den ? 1 : 0;
- GncRational new_rational(num, den);
- return new_rational;
+ auto divisor = m_den >> ll_bits;
+ if (m_num.isBig())
+ {
+ GncInt128 oldnum(m_num), num, rem;
+ oldnum.div(divisor, num, rem);
+ auto den = m_den / divisor;
+ num += rem * 2 >= den ? 1 : 0;
+ if (num.isBig() || den.isBig())
+ {
+ --ll_bits;
+ continue;
+ }
+ GncRational new_rational(num, den);
+ return new_rational;
+ }
+ new_v = convert<RoundType::half_down>(m_den / divisor);
+ if (new_v.is_big())
+ {
+ --ll_bits;
+ new_v = GncRational();
+ }
}
- GncRational new_v(*this);
- new_v = new_v.convert<RoundType::half_down>(m_den / divisor);
return new_v;
}
diff --git a/src/libqof/qof/gnc-rational.hpp b/src/libqof/qof/gnc-rational.hpp
index 59cf685..c8ba2c3 100644
--- a/src/libqof/qof/gnc-rational.hpp
+++ b/src/libqof/qof/gnc-rational.hpp
@@ -150,9 +150,9 @@ public:
}
/** Numerator accessor */
- GncInt128 num() { return m_num; }
+ GncInt128 num() const noexcept { return m_num; }
/** Denominator accessor */
- GncInt128 denom() { return m_den; }
+ GncInt128 denom() const noexcept { return m_den; }
/** @defgroup gnc_rational_mutators
* @{
* Standard mutating arithmetic operators.
@@ -275,5 +275,11 @@ inline GncRational operator/(GncInt128 a, GncRational b)
{
return GncRational(a, 1) / b;
}
+
+inline std::ostream& operator<<(std::ostream& stream, const GncRational& val) noexcept
+{
+ stream << val.num() << "/" << val.denom();
+ return stream;
+}
/** @} */
#endif //__GNC_RATIONAL_HPP__
diff --git a/src/libqof/qof/test/gtest-gnc-int128.cpp b/src/libqof/qof/test/gtest-gnc-int128.cpp
index 946063f..869b33d 100644
--- a/src/libqof/qof/test/gtest-gnc-int128.cpp
+++ b/src/libqof/qof/test/gtest-gnc-int128.cpp
@@ -581,6 +581,7 @@ TEST(qofint128_functions, pow)
EXPECT_EQ (GncInt128(1), little.pow(0));
EXPECT_EQ (GncInt128(0), GncInt128(0).pow(123));
+ auto really_big = big * big;
EXPECT_EQ (big * big, big.pow(2));
EXPECT_EQ (GncInt128(UINT64_C(66326033898754),
UINT64_C(10251549987585143605)), little.pow(7));
@@ -590,3 +591,17 @@ TEST(qofint128_functions, pow)
auto over = minus.pow(9);
EXPECT_TRUE(over.isOverflow());
}
+
+TEST(qofint128_functions, shift)
+{
+ GncInt128 a (UINT64_C(0xabcdabcd), UINT64_C(0xfe89fe89fe89fe89), 0);
+ EXPECT_EQ(GncInt128(UINT64_C(0xabcdabcdfe89), UINT64_C(0xfe89fe89fe890000), 0), a << 16);
+ EXPECT_EQ(GncInt128(UINT64_C(0xabcd), UINT64_C(0xabcdfe89fe89fe89), 0), a >> 16);
+ EXPECT_EQ(GncInt128(UINT64_C(0x1e89fe89fe89fe89), UINT64_C(0), 0), a << 64);
+ EXPECT_EQ(GncInt128(UINT64_C(0), UINT64_C(0xabcdabcd), 0), a >> 64);
+ GncInt128 b (UINT64_C(0xabcdabcd), UINT64_C(0xfe89fe89fe89fe89), 1);
+ EXPECT_EQ(GncInt128(UINT64_C(0xabcdabcdfe89), UINT64_C(0xfe89fe89fe890000), 1), b << 16);
+ EXPECT_EQ(GncInt128(UINT64_C(0xabcd), UINT64_C(0xabcdfe89fe89fe89), 1), b >> 16);
+ EXPECT_EQ(GncInt128(UINT64_C(0x1e89fe89fe89fe89), UINT64_C(0), 1), b << 64);
+ EXPECT_EQ(GncInt128(UINT64_C(0), UINT64_C(0xabcdabcd), 1), b >> 64);
+}
diff --git a/src/libqof/qof/test/gtest-gnc-numeric.cpp b/src/libqof/qof/test/gtest-gnc-numeric.cpp
index c4633ea..5237445 100644
--- a/src/libqof/qof/test/gtest-gnc-numeric.cpp
+++ b/src/libqof/qof/test/gtest-gnc-numeric.cpp
@@ -332,17 +332,22 @@ TEST(gncnumeric_operators, test_multiplication)
GncNumeric a(123456789987654321, 1000000000);
GncNumeric b(65432198765432198, 100000000);
GncNumeric c = a * b;
- EXPECT_EQ (4604488056206217807, c.num());
- EXPECT_EQ (57, c.denom());
+ EXPECT_EQ (9208976112412435614, c.num());
+ EXPECT_EQ (114, c.denom());
a *= b;
- EXPECT_EQ (4604488056206217807, a.num());
- EXPECT_EQ (57, a.denom());
+ EXPECT_EQ (9208976112412435614, a.num());
+ EXPECT_EQ (114, a.denom());
GncNumeric d(215815575996, 269275978715);
GncNumeric e(1002837599929, 1);
GncNumeric f, g;
EXPECT_NO_THROW(f = d * e);
EXPECT_NO_THROW(g = f.convert<RoundType::half_up>(1));
+ GncNumeric h(133499999999, 136499999999);
+ GncNumeric i(60000000003, 100000000);
+ GncNumeric j;
+ EXPECT_NO_THROW(j = gnc_numeric_mul(h, i, 100000000, GNC_HOW_RND_ROUND));
+ EXPECT_EQ(58681318684, j.num());
}
diff --git a/src/libqof/qof/test/gtest-gnc-rational.cpp b/src/libqof/qof/test/gtest-gnc-rational.cpp
index 8b002b8..fa6c2d9 100644
--- a/src/libqof/qof/test/gtest-gnc-rational.cpp
+++ b/src/libqof/qof/test/gtest-gnc-rational.cpp
@@ -149,25 +149,31 @@ rounding_predicate(GncInt128 expected, GncInt128 result)
TEST(gncrational_functions, test_round_to_numeric)
{
std::default_random_engine dre;
- std::uniform_int_distribution<int64_t> di{INT64_C(0x100000000000),
- INT64_C(0x7fffffffffffff)};
+ std::uniform_int_distribution<int64_t> di{INT64_C(0x1000000000000),
+ INT64_C(0x7ffffffffffffff)};
static const int reps{100};
for (auto i = 0; i < reps; ++i)
{
GncRational a(di(dre), di(dre));
GncRational b(di(dre), 100);
- auto c = a * b;
- auto expected = c;
- expected = expected.convert<RoundType::bankers>(100);
- auto rounded = c.round_to_numeric();
- rounded = rounded.convert<RoundType::bankers>(100);
- if (rounded.is_big())
+ try {
+ auto c = a * b;
+ auto expected = c;
+ expected = expected.convert<RoundType::bankers>(100);
+ auto rounded = c.round_to_numeric();
+ rounded = rounded.convert<RoundType::bankers>(100);
+ if (rounded.is_big())
+ {
+ --i;
+ continue;
+ }
+ EXPECT_PRED2(rounding_predicate, expected.num(), rounded.num());
+ EXPECT_TRUE(rounded.valid());
+ }
+ catch (const std::overflow_error& err)
{
- --i;
- continue;
+ std::cerr << "Overflow error from " << a << " * " << b << ".\n";
}
- EXPECT_PRED2(rounding_predicate, expected.num(), rounded.num());
- EXPECT_TRUE(rounded.valid());
}
}
commit a467d0d397ada9ea056c536b16bd901947b3dfb7
Author: John Ralls <jralls at ceridwen.us>
Date: Fri Apr 7 12:29:04 2017 -0700
Fix GncInt128 maxbits to account for the flag bits.
diff --git a/src/libqof/qof/gnc-int128.cpp b/src/libqof/qof/gnc-int128.cpp
index 4ac0017..b3c5c91 100644
--- a/src/libqof/qof/gnc-int128.cpp
+++ b/src/libqof/qof/gnc-int128.cpp
@@ -501,7 +501,12 @@ GncInt128::operator*= (const GncInt128& b) noexcept
}
unsigned int abits {bits()}, bbits {b.bits()};
- if (abits + bbits > maxbits)
+ /* If the product of the high bytes < 7fff then the result will have abits +
+ * bbits -1 bits and won't actually overflow. It's not worth the effort to
+ * work that out for this corner-case, so we'll take the risk and calculate
+ * the product and catch the overflow later.
+ */
+ if (abits + bbits - 1 > maxbits)
{
flags |= overflow;
m_hi = set_flags(m_hi, flags);
diff --git a/src/libqof/qof/gnc-int128.hpp b/src/libqof/qof/gnc-int128.hpp
index bb99336..1306ead 100644
--- a/src/libqof/qof/gnc-int128.hpp
+++ b/src/libqof/qof/gnc-int128.hpp
@@ -67,9 +67,10 @@ class GncInt128
uint64_t m_lo;
public:
+ static const unsigned int flagbits = 3;
static const unsigned int numlegs = 2;
static const unsigned int legbits = 64;
- static const unsigned int maxbits = legbits * numlegs;
+ static const unsigned int maxbits = legbits * numlegs - flagbits;
enum // Values for m_flags
{
commit de1c56b53d62df0d6f0f86dbfe22b043f530f132
Author: John Ralls <jralls at ceridwen.us>
Date: Fri Apr 7 12:27:59 2017 -0700
Fix carrying to the wrong end of the lower leg in left shift.
diff --git a/src/libqof/qof/gnc-int128.cpp b/src/libqof/qof/gnc-int128.cpp
index 28fcb16..4ac0017 100644
--- a/src/libqof/qof/gnc-int128.cpp
+++ b/src/libqof/qof/gnc-int128.cpp
@@ -384,7 +384,7 @@ GncInt128::operator<<= (unsigned int i) noexcept
auto hi = get_num(m_hi);
if (i < legbits)
{
- uint64_t carry {(m_lo & (((UINT64_C(1) << i) - 1) << (legbits - i)))};
+ uint64_t carry {(m_lo & (((UINT64_C(1) << i) - 1) << (legbits - i))) >> (legbits - i)};
m_lo <<= i;
hi <<= i;
hi += carry;
commit e55b78614cc6e124eb72a214f2ab8d6da7cb1a9e
Author: John Ralls <jralls at ceridwen.us>
Date: Fri Apr 7 12:26:51 2017 -0700
Fix a magic number in GncInt128.
diff --git a/src/libqof/qof/gnc-int128.cpp b/src/libqof/qof/gnc-int128.cpp
index fdb87e2..28fcb16 100644
--- a/src/libqof/qof/gnc-int128.cpp
+++ b/src/libqof/qof/gnc-int128.cpp
@@ -40,6 +40,7 @@ extern "C"
*/
namespace {
+ static const unsigned int upper_num_bits = 61;
static const unsigned int sublegs = GncInt128::numlegs * 2;
static const unsigned int sublegbits = GncInt128::legbits / 2;
static const uint64_t sublegmask = (UINT64_C(1) << sublegbits) - 1;
@@ -50,12 +51,12 @@ namespace {
*/
static inline uint64_t set_flags(uint64_t leg, uint8_t flags)
{
- auto flag_part = static_cast<uint64_t>(flags) << 61;
+ auto flag_part = static_cast<uint64_t>(flags) << upper_num_bits;
return flag_part + (leg & nummask);
}
static inline uint8_t get_flags(uint64_t leg)
{
- return (leg & flagmask) >> 61;
+ return (leg & flagmask) >> upper_num_bits;
}
static inline uint64_t get_num(uint64_t leg)
{
Summary of changes:
src/libqof/qof/gnc-int128.cpp | 14 +++++---
src/libqof/qof/gnc-int128.hpp | 3 +-
src/libqof/qof/gnc-rational.cpp | 58 +++++++++++++++++++++++-------
src/libqof/qof/gnc-rational.hpp | 10 ++++--
src/libqof/qof/test/gtest-gnc-int128.cpp | 15 ++++++++
src/libqof/qof/test/gtest-gnc-numeric.cpp | 13 ++++---
src/libqof/qof/test/gtest-gnc-rational.cpp | 30 +++++++++-------
7 files changed, 108 insertions(+), 35 deletions(-)
More information about the gnucash-changes
mailing list