Skip to content

Commit

Permalink
[libc] Make BigInt bitwise shift consistent with regular integral s…
Browse files Browse the repository at this point in the history
…emantics. (#87874)

This patch removes the test for cases where the shift operand is greater
or equal to the bit width of the number. This is done for two reasons,
first it makes `BigInt` consistent with regular integral bitwise shift
semantics, and second it makes the shift operation faster. The shift
operation is on the critical path for `exp` and `log` operations, see
#86137 (comment).
  • Loading branch information
gchatelet committed Apr 6, 2024
1 parent fc7087b commit b88a1dd
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 17 deletions.
6 changes: 4 additions & 2 deletions libc/src/__support/FPUtil/dyadic_float.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,15 +122,17 @@ template <size_t Bits> struct DyadicFloat {

int exp_lo = exp_hi - static_cast<int>(PRECISION) - 1;

MantissaType m_hi(mantissa >> shift);
MantissaType m_hi =
shift >= MantissaType::BITS ? MantissaType(0) : mantissa >> shift;

T d_hi = FPBits<T>::create_value(
sign, exp_hi,
(static_cast<output_bits_t>(m_hi) & FPBits<T>::SIG_MASK) |
IMPLICIT_MASK)
.get_val();

MantissaType round_mask = MantissaType(1) << (shift - 1);
MantissaType round_mask =
shift > MantissaType::BITS ? 0 : MantissaType(1) << (shift - 1);
MantissaType sticky_mask = round_mask - MantissaType(1);

bool round_bit = !(mantissa & round_mask).is_zero();
Expand Down
10 changes: 4 additions & 6 deletions libc/src/__support/big_int.h
Original file line number Diff line number Diff line change
Expand Up @@ -249,18 +249,14 @@ LIBC_INLINE constexpr bool is_negative(cpp::array<word, N> &array) {
enum Direction { LEFT, RIGHT };

// A bitwise shift on an array of elements.
// TODO: Make the result UB when 'offset' is greater or equal to the number of
// bits in 'array'. This will allow for better code performance.
// 'offset' must be less than TOTAL_BITS (i.e., sizeof(word) * CHAR_BIT * N)
// otherwise the behavior is undefined.
template <Direction direction, bool is_signed, typename word, size_t N>
LIBC_INLINE constexpr cpp::array<word, N> shift(cpp::array<word, N> array,
size_t offset) {
static_assert(direction == LEFT || direction == RIGHT);
constexpr size_t WORD_BITS = cpp::numeric_limits<word>::digits;
constexpr size_t TOTAL_BITS = N * WORD_BITS;
if (LIBC_UNLIKELY(offset == 0))
return array;
if (LIBC_UNLIKELY(offset >= TOTAL_BITS))
return {};
#ifdef LIBC_TYPES_HAS_INT128
if constexpr (TOTAL_BITS == 128) {
using type = cpp::conditional_t<is_signed, __int128_t, __uint128_t>;
Expand All @@ -272,6 +268,8 @@ LIBC_INLINE constexpr cpp::array<word, N> shift(cpp::array<word, N> array,
return cpp::bit_cast<cpp::array<word, N>>(tmp);
}
#endif
if (LIBC_UNLIKELY(offset == 0))
return array;
const bool is_neg = is_signed && is_negative(array);
constexpr auto at = [](size_t index) -> int {
// reverse iteration when direction == LEFT.
Expand Down
10 changes: 1 addition & 9 deletions libc/test/src/__support/big_int_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ TYPED_TEST(LlvmLibcUIntClassTest, Masks, Types) {

TYPED_TEST(LlvmLibcUIntClassTest, CountBits, Types) {
if constexpr (!T::SIGNED) {
for (size_t i = 0; i <= T::BITS; ++i) {
for (size_t i = 0; i < T::BITS; ++i) {
const auto l_one = T::all_ones() << i; // 0b111...000
const auto r_one = T::all_ones() >> i; // 0b000...111
const int zeros = i;
Expand Down Expand Up @@ -559,10 +559,6 @@ TEST(LlvmLibcUIntClassTest, ShiftLeftTests) {
LL_UInt128 result5({0, 0x2468ace000000000});
EXPECT_EQ((val2 << 100), result5);

LL_UInt128 result6({0, 0});
EXPECT_EQ((val2 << 128), result6);
EXPECT_EQ((val2 << 256), result6);

LL_UInt192 val3({1, 0, 0});
LL_UInt192 result7({0, 1, 0});
EXPECT_EQ((val3 << 64), result7);
Expand All @@ -589,10 +585,6 @@ TEST(LlvmLibcUIntClassTest, ShiftRightTests) {
LL_UInt128 result5({0x0000000001234567, 0});
EXPECT_EQ((val2 >> 100), result5);

LL_UInt128 result6({0, 0});
EXPECT_EQ((val2 >> 128), result6);
EXPECT_EQ((val2 >> 256), result6);

LL_UInt128 v1({0x1111222233334444, 0xaaaabbbbccccdddd});
LL_UInt128 r1({0xaaaabbbbccccdddd, 0});
EXPECT_EQ((v1 >> 64), r1);
Expand Down

0 comments on commit b88a1dd

Please sign in to comment.