diff options
author | Abseil Team <absl-team@google.com> | 2021-10-11 15:32:11 -0700 |
---|---|---|
committer | dinord <dino.radakovich@gmail.com> | 2021-10-12 10:57:14 -0400 |
commit | d6f40f4e1f424677a88eee1dade4f3ae2ea5f743 (patch) | |
tree | 16f6e40aa9d543de1f9cac5820784808e33ef841 | |
parent | 084aa074c6e3994471fcdb8f542d3a24916792de (diff) |
Export of internal Abseil changes
--
566c422ef23a01a71ca7a02d15a3f23ab5135f47 by Abseil Team <absl-team@google.com>:
Fix a bug in the right shift of negative numbers for the no_intrinsic code path
If the right shift was greater than or equal to 64 bits the the most significant 64 bit would always be initialized to zero. The code now initializes the most significant int64 to all ones if the architecture does arithmetic rights shifts and the number was negative.
PiperOrigin-RevId: 402407698
GitOrigin-RevId: 566c422ef23a01a71ca7a02d15a3f23ab5135f47
Change-Id: I01c57a87a91a2c89f62bb952661e3a5dcdca6234
-rw-r--r-- | absl/numeric/int128_no_intrinsic.inc | 12 | ||||
-rw-r--r-- | absl/numeric/int128_test.cc | 18 |
2 files changed, 27 insertions, 3 deletions
diff --git a/absl/numeric/int128_no_intrinsic.inc b/absl/numeric/int128_no_intrinsic.inc index 66f6809f..8834804c 100644 --- a/absl/numeric/int128_no_intrinsic.inc +++ b/absl/numeric/int128_no_intrinsic.inc @@ -279,7 +279,7 @@ constexpr int128 operator^(int128 lhs, int128 rhs) { } constexpr int128 operator<<(int128 lhs, int amount) { - // uint64_t shifts of >= 64 are undefined, so we need some special-casing. + // int64_t shifts of >= 64 are undefined, so we need some special-casing. return amount >= 64 ? MakeInt128( static_cast<int64_t>(Int128Low64(lhs) << (amount - 64)), 0) @@ -292,10 +292,16 @@ constexpr int128 operator<<(int128 lhs, int amount) { } constexpr int128 operator>>(int128 lhs, int amount) { - // uint64_t shifts of >= 64 are undefined, so we need some special-casing. + // int64_t shifts of >= 64 are undefined, so we need some special-casing. + // The (Int128High64(lhs) >> 32) >> 32 "trick" causes the the most significant + // int64 to be inititialized with all zeros or all ones correctly. It takes + // into account whether the number is negative or positive, and whether the + // current architecture does arithmetic or logical right shifts for negative + // numbers. return amount >= 64 ? MakeInt128( - 0, static_cast<uint64_t>(Int128High64(lhs) >> (amount - 64))) + (Int128High64(lhs) >> 32) >> 32, + static_cast<uint64_t>(Int128High64(lhs) >> (amount - 64))) : amount == 0 ? lhs : MakeInt128(Int128High64(lhs) >> amount, diff --git a/absl/numeric/int128_test.cc b/absl/numeric/int128_test.cc index c445d89a..dd9425d7 100644 --- a/absl/numeric/int128_test.cc +++ b/absl/numeric/int128_test.cc @@ -239,6 +239,24 @@ TEST(Uint128, AllTests) { EXPECT_EQ(absl::Uint128Max(), absl::kuint128max); } +TEST(Int128, RightShiftOfNegativeNumbers) { + absl::int128 minus_six = -6; + absl::int128 minus_three = -3; + absl::int128 minus_two = -2; + absl::int128 minus_one = -1; + if ((-6 >> 1) == -3) { + // Right shift is arithmetic (sign propagates) + EXPECT_EQ(minus_six >> 1, minus_three); + EXPECT_EQ(minus_six >> 2, minus_two); + EXPECT_EQ(minus_six >> 65, minus_one); + } else { + // Right shift is logical (zeros shifted in at MSB) + EXPECT_EQ(minus_six >> 1, absl::int128(absl::uint128(minus_six) >> 1)); + EXPECT_EQ(minus_six >> 2, absl::int128(absl::uint128(minus_six) >> 2)); + EXPECT_EQ(minus_six >> 65, absl::int128(absl::uint128(minus_six) >> 65)); + } +} + TEST(Uint128, ConversionTests) { EXPECT_TRUE(absl::MakeUint128(1, 0)); |