From b7f397b4102ed48a75556ba54c7e1fea7601951a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 4 Mar 2021 11:48:18 +0100 Subject: [PATCH 1/2] test: Enhance shift tests --- test/unittests/test_intx.cpp | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/test/unittests/test_intx.cpp b/test/unittests/test_intx.cpp index 5c2fc001..165b0f27 100644 --- a/test/unittests/test_intx.cpp +++ b/test/unittests/test_intx.cpp @@ -341,21 +341,31 @@ TYPED_TEST(uint_test, shift_one_bit) { for (unsigned shift = 0; shift < sizeof(TypeParam) * 8; ++shift) { - auto x = TypeParam{1}; - auto y = x << shift; - auto z = y >> shift; - EXPECT_EQ(x, z) << "shift: " << shift; + SCOPED_TRACE(shift); + constexpr auto x = TypeParam{1}; + + const auto a = x << shift; + const auto b = shl_loop(x, shift); + EXPECT_EQ(b, a); + + EXPECT_EQ(x, a >> shift); } } -TYPED_TEST(uint_test, shift_loop_one_bit) +TYPED_TEST(uint_test, shift_left_overflow) { - for (unsigned shift = 0; shift < sizeof(TypeParam) * 8; ++shift) + const auto x = ~TypeParam{}; + + for (unsigned n = 0; n <= sizeof(TypeParam) * 7; ++n) + { + const auto sh = x >> n; + EXPECT_EQ(x << sh, 0) << "n=" << n; + } + + for (unsigned n = 0; n <= sizeof(TypeParam) * 7; ++n) { - auto x = TypeParam{1}; - auto y = shl_loop(x, shift); - auto z = y >> shift; - EXPECT_EQ(x, z) << "shift: " << shift; + const auto sh = TypeParam{sizeof(TypeParam) * 8} << n; + EXPECT_EQ(x << sh, 0) << "n=" << n; } } @@ -367,6 +377,7 @@ TYPED_TEST(uint_test, shift_overflow) EXPECT_EQ(value >> TypeParam{sh}, 0); EXPECT_EQ(value << sh, 0); EXPECT_EQ(value << TypeParam{sh}, 0); + EXPECT_EQ(shl_loop(value, sh), 0); } TYPED_TEST(uint_test, not_of_zero) From 97ec68d23114e37afe09a410eee75d9b3f10e264 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 4 Mar 2021 16:49:02 +0100 Subject: [PATCH 2/2] Change shift amount param from unsigned to uint64_t This simplifies reduction from any intx::uint to the shift amount param because we can just use the lowest word for this. --- include/intx/int128.hpp | 12 ++++++------ include/intx/intx.hpp | 31 ++++++++++++++++--------------- test/unittests/test_intx.cpp | 2 +- 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/include/intx/int128.hpp b/include/intx/int128.hpp index 41cf85a8..3eae231e 100644 --- a/include/intx/int128.hpp +++ b/include/intx/int128.hpp @@ -323,7 +323,7 @@ inline constexpr uint128 operator^(uint128 x, uint128 y) noexcept return {x.hi ^ y.hi, x.lo ^ y.lo}; } -inline constexpr uint128 operator<<(uint128 x, unsigned shift) noexcept +inline constexpr uint128 operator<<(uint128 x, uint64_t shift) noexcept { return (shift < 64) ? // Find the part moved from lo to hi. @@ -338,11 +338,11 @@ inline constexpr uint128 operator<<(uint128 x, unsigned shift) noexcept inline constexpr uint128 operator<<(uint128 x, uint128 shift) noexcept { if (shift < 128) - return x << unsigned(shift); + return x << static_cast(shift); return 0; } -inline constexpr uint128 operator>>(uint128 x, unsigned shift) noexcept +inline constexpr uint128 operator>>(uint128 x, uint64_t shift) noexcept { return (shift < 64) ? // Find the part moved from lo to hi. @@ -357,7 +357,7 @@ inline constexpr uint128 operator>>(uint128 x, unsigned shift) noexcept inline constexpr uint128 operator>>(uint128 x, uint128 shift) noexcept { if (shift < 128) - return x >> unsigned(shift); + return x >> static_cast(shift); return 0; } @@ -452,12 +452,12 @@ inline constexpr uint128& operator^=(uint128& x, uint128 y) noexcept return x = x ^ y; } -inline constexpr uint128& operator<<=(uint128& x, unsigned shift) noexcept +inline constexpr uint128& operator<<=(uint128& x, uint64_t shift) noexcept { return x = x << shift; } -inline constexpr uint128& operator>>=(uint128& x, unsigned shift) noexcept +inline constexpr uint128& operator>>=(uint128& x, uint64_t shift) noexcept { return x = x >> shift; } diff --git a/include/intx/intx.hpp b/include/intx/intx.hpp index 53079cd1..e027409e 100644 --- a/include/intx/intx.hpp +++ b/include/intx/intx.hpp @@ -261,7 +261,7 @@ inline constexpr uint operator~(const uint& x) noexcept } template -inline constexpr uint operator<<(const uint& x, unsigned shift) noexcept +inline constexpr uint operator<<(const uint& x, uint64_t shift) noexcept { constexpr auto num_bits = N; constexpr auto half_bits = num_bits / 2; @@ -301,25 +301,26 @@ inline Target narrow_cast(const Int& x) noexcept } template -inline constexpr uint operator>>(const uint& x, unsigned shift) noexcept +inline constexpr uint operator>>(const uint& x, uint64_t shift) noexcept { - constexpr auto half_bits = N / 2; + constexpr auto num_bits = N; + constexpr auto half_bits = num_bits / 2; if (shift < half_bits) { - auto hi = x.hi >> shift; + const auto hi = x.hi >> shift; // Find the part moved from hi to lo. // To avoid invalid shift left, // split them into 2 valid shifts by (lshift - 1) and 1. - unsigned lshift = half_bits - shift; - auto hi_overflow = (x.hi << (lshift - 1)) << 1; - auto lo_part = x.lo >> shift; - auto lo = lo_part | hi_overflow; + const auto lshift = half_bits - shift; + const auto hi_overflow = (x.hi << (lshift - 1)) << 1; + const auto lo_part = x.lo >> shift; + const auto lo = lo_part | hi_overflow; return {hi, lo}; } - if (shift < num_bits(x)) + if (shift < num_bits) return {0, x.hi >> (shift - half_bits)}; return 0; @@ -331,7 +332,7 @@ template operator<<(const uint& x, const T& shift) noexcept { if (shift < T{sizeof(x) * 8}) - return x << static_cast(shift); + return x << static_cast(shift); return 0; } @@ -340,12 +341,12 @@ template operator>>(const uint& x, const T& shift) noexcept { if (shift < T{sizeof(x) * 8}) - return x >> static_cast(shift); + return x >> static_cast(shift); return 0; } template -inline constexpr uint& operator>>=(uint& x, unsigned shift) noexcept +inline constexpr uint& operator>>=(uint& x, uint64_t shift) noexcept { return x = x >> shift; } @@ -388,15 +389,15 @@ inline const uint8_t* as_bytes(const uint& x) noexcept /// Implementation of shift left as a loop. /// This one is slower than the one using "split" strategy. template -inline uint shl_loop(const uint& x, unsigned shift) +inline uint shl_loop(const uint& x, uint64_t shift) { auto r = uint{}; constexpr unsigned word_bits = sizeof(uint64_t) * 8; constexpr size_t num_words = sizeof(uint) / sizeof(uint64_t); auto rw = as_words(r); auto words = as_words(x); - unsigned s = shift % word_bits; - unsigned skip = shift / word_bits; + const auto s = shift % word_bits; + const auto skip = shift / word_bits; uint64_t carry = 0; for (size_t i = 0; i < (num_words - skip); ++i) diff --git a/test/unittests/test_intx.cpp b/test/unittests/test_intx.cpp index 165b0f27..d0444ff3 100644 --- a/test/unittests/test_intx.cpp +++ b/test/unittests/test_intx.cpp @@ -371,7 +371,7 @@ TYPED_TEST(uint_test, shift_left_overflow) TYPED_TEST(uint_test, shift_overflow) { - unsigned sh = sizeof(TypeParam) * 8; + const uint64_t sh = sizeof(TypeParam) * 8; const auto value = ~TypeParam{}; EXPECT_EQ(value >> sh, 0); EXPECT_EQ(value >> TypeParam{sh}, 0);