Skip to content

Commit d6f40f4

Browse files
Abseil Teamdinord
authored andcommitted
Export of internal Abseil changes
-- 566c422ef23a01a71ca7a02d15a3f23ab5135f47 by Abseil Team <[email protected]>: 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
1 parent 084aa07 commit d6f40f4

File tree

2 files changed

+27
-3
lines changed

2 files changed

+27
-3
lines changed

absl/numeric/int128_no_intrinsic.inc

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ constexpr int128 operator^(int128 lhs, int128 rhs) {
279279
}
280280

281281
constexpr int128 operator<<(int128 lhs, int amount) {
282-
// uint64_t shifts of >= 64 are undefined, so we need some special-casing.
282+
// int64_t shifts of >= 64 are undefined, so we need some special-casing.
283283
return amount >= 64
284284
? MakeInt128(
285285
static_cast<int64_t>(Int128Low64(lhs) << (amount - 64)), 0)
@@ -292,10 +292,16 @@ constexpr int128 operator<<(int128 lhs, int amount) {
292292
}
293293

294294
constexpr int128 operator>>(int128 lhs, int amount) {
295-
// uint64_t shifts of >= 64 are undefined, so we need some special-casing.
295+
// int64_t shifts of >= 64 are undefined, so we need some special-casing.
296+
// The (Int128High64(lhs) >> 32) >> 32 "trick" causes the the most significant
297+
// int64 to be inititialized with all zeros or all ones correctly. It takes
298+
// into account whether the number is negative or positive, and whether the
299+
// current architecture does arithmetic or logical right shifts for negative
300+
// numbers.
296301
return amount >= 64
297302
? MakeInt128(
298-
0, static_cast<uint64_t>(Int128High64(lhs) >> (amount - 64)))
303+
(Int128High64(lhs) >> 32) >> 32,
304+
static_cast<uint64_t>(Int128High64(lhs) >> (amount - 64)))
299305
: amount == 0
300306
? lhs
301307
: MakeInt128(Int128High64(lhs) >> amount,

absl/numeric/int128_test.cc

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,24 @@ TEST(Uint128, AllTests) {
239239
EXPECT_EQ(absl::Uint128Max(), absl::kuint128max);
240240
}
241241

242+
TEST(Int128, RightShiftOfNegativeNumbers) {
243+
absl::int128 minus_six = -6;
244+
absl::int128 minus_three = -3;
245+
absl::int128 minus_two = -2;
246+
absl::int128 minus_one = -1;
247+
if ((-6 >> 1) == -3) {
248+
// Right shift is arithmetic (sign propagates)
249+
EXPECT_EQ(minus_six >> 1, minus_three);
250+
EXPECT_EQ(minus_six >> 2, minus_two);
251+
EXPECT_EQ(minus_six >> 65, minus_one);
252+
} else {
253+
// Right shift is logical (zeros shifted in at MSB)
254+
EXPECT_EQ(minus_six >> 1, absl::int128(absl::uint128(minus_six) >> 1));
255+
EXPECT_EQ(minus_six >> 2, absl::int128(absl::uint128(minus_six) >> 2));
256+
EXPECT_EQ(minus_six >> 65, absl::int128(absl::uint128(minus_six) >> 65));
257+
}
258+
}
259+
242260
TEST(Uint128, ConversionTests) {
243261
EXPECT_TRUE(absl::MakeUint128(1, 0));
244262

0 commit comments

Comments
 (0)