summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar Abseil Team <absl-team@google.com>2021-10-11 15:32:11 -0700
committerGravatar dinord <dino.radakovich@gmail.com>2021-10-12 10:57:14 -0400
commitd6f40f4e1f424677a88eee1dade4f3ae2ea5f743 (patch)
tree16f6e40aa9d543de1f9cac5820784808e33ef841
parent084aa074c6e3994471fcdb8f542d3a24916792de (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.inc12
-rw-r--r--absl/numeric/int128_test.cc18
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));