[libc] Fix forward missing BigInt specialization of mask_leading_ones / mask_trailing_ones
#84325
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…nes` / `mask_trailing_ones` llvm#84299 broke the arm32 build, this patch fixes it forward.
|
@llvm/pr-subscribers-libc Author: Guillaume Chatelet (gchatelet) Changes#84299 broke the arm32 build, this patch fixes it forward. Full diff: https://github.com/llvm/llvm-project/pull/84325.diff 5 Files Affected:
diff --git a/libc/src/__support/UInt.h b/libc/src/__support/UInt.h
index b3d8f00b9a01a5..44291218fb7bc0 100644
--- a/libc/src/__support/UInt.h
+++ b/libc/src/__support/UInt.h
@@ -1054,6 +1054,52 @@ rotr(T value, int rotate) {
return (value >> rotate) | (value << (N - rotate));
}
+// Specialization of mask_trailing_ones ('math_extras.h') for BigInt.
+template <typename T, size_t count>
+LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_big_int_v<T>, T>
+mask_trailing_ones() {
+ static_assert(!T::SIGNED);
+ if (count == 0)
+ return T();
+ constexpr unsigned T_BITS = CHAR_BIT * sizeof(T);
+ static_assert(count <= T_BITS && "Invalid bit index");
+ T out;
+ size_t lo_index = 0;
+ for (auto &word : out.val) {
+ if (count < lo_index)
+ word = 0;
+ else if (count > lo_index + T::WORD_SIZE)
+ word = -1;
+ else
+ word = mask_trailing_ones<T::word_type, count % T::WORD_SIZE>();
+ lo_index += T::WORD_SIZE;
+ }
+ return out;
+}
+
+// Specialization of mask_leading_ones ('math_extras.h') for BigInt.
+template <typename T, size_t count>
+LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_big_int_v<T>, T>
+mask_leading_ones() {
+ static_assert(!T::SIGNED);
+ if (count == 0)
+ return T();
+ constexpr unsigned T_BITS = CHAR_BIT * sizeof(T);
+ static_assert(count <= T_BITS && "Invalid bit index");
+ T out;
+ size_t lo_index = 0;
+ for (auto &word : out.val) {
+ if (count < lo_index)
+ word = -1;
+ else if (count > lo_index + T::WORD_SIZE)
+ word = 0;
+ else
+ word = mask_leading_ones<T::word_type, count % T::WORD_SIZE>();
+ lo_index += T::WORD_SIZE;
+ }
+ return out;
+}
+
} // namespace LIBC_NAMESPACE::cpp
#endif // LLVM_LIBC_SRC___SUPPORT_UINT_H
diff --git a/libc/src/__support/math_extras.h b/libc/src/__support/math_extras.h
index 7a89fbb11b2a9e..c6b458ddecdabf 100644
--- a/libc/src/__support/math_extras.h
+++ b/libc/src/__support/math_extras.h
@@ -20,21 +20,18 @@ namespace LIBC_NAMESPACE {
// Create a bitmask with the count right-most bits set to 1, and all other bits
// set to 0. Only unsigned types are allowed.
template <typename T, size_t count>
-LIBC_INLINE constexpr T mask_trailing_ones() {
- static_assert(cpp::is_unsigned_v<T>);
+LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, T>
+mask_trailing_ones() {
constexpr unsigned T_BITS = CHAR_BIT * sizeof(T);
static_assert(count <= T_BITS && "Invalid bit index");
- // It's important not to initialize T with -1, since T may be BigInt which
- // will take -1 as a uint64_t and only initialize the low 64 bits.
- constexpr T ALL_ZEROES(0);
- constexpr T ALL_ONES(~ALL_ZEROES); // bitwise NOT performs integer promotion.
- return count == 0 ? 0 : (ALL_ONES >> (T_BITS - count));
+ return count == 0 ? 0 : (T(-1) >> (T_BITS - count));
}
// Create a bitmask with the count left-most bits set to 1, and all other bits
// set to 0. Only unsigned types are allowed.
template <typename T, size_t count>
-LIBC_INLINE constexpr T mask_leading_ones() {
+LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, T>
+mask_leading_ones() {
constexpr T MASK(mask_trailing_ones<T, CHAR_BIT * sizeof(T) - count>());
return T(~MASK); // bitwise NOT performs integer promotion.
}
diff --git a/libc/test/src/__support/CMakeLists.txt b/libc/test/src/__support/CMakeLists.txt
index 8c861b576f9b1b..adbacb9728ccd4 100644
--- a/libc/test/src/__support/CMakeLists.txt
+++ b/libc/test/src/__support/CMakeLists.txt
@@ -27,7 +27,9 @@ add_libc_test(
SRCS
math_extras_test.cpp
DEPENDS
+ libc.src.__support.integer_literals
libc.src.__support.math_extras
+ libc.src.__support.uint128
)
add_libc_test(
diff --git a/libc/test/src/__support/math_extras_test.cpp b/libc/test/src/__support/math_extras_test.cpp
index e55d995592cc1c..7e293268729ec7 100644
--- a/libc/test/src/__support/math_extras_test.cpp
+++ b/libc/test/src/__support/math_extras_test.cpp
@@ -6,34 +6,47 @@
//
//===----------------------------------------------------------------------===//
+#include "src/__support/UInt128.h" // UInt128
+#include "src/__support/integer_literals.h"
#include "src/__support/math_extras.h"
#include "test/UnitTest/Test.h"
namespace LIBC_NAMESPACE {
TEST(LlvmLibcBlockMathExtrasTest, mask_trailing_ones) {
- EXPECT_EQ(uint8_t(0), (mask_leading_ones<uint8_t, 0>()));
- EXPECT_EQ(uint8_t(0), (mask_trailing_ones<uint8_t, 0>()));
- EXPECT_EQ(uint16_t(0), (mask_leading_ones<uint16_t, 0>()));
- EXPECT_EQ(uint16_t(0), (mask_trailing_ones<uint16_t, 0>()));
- EXPECT_EQ(uint32_t(0), (mask_leading_ones<uint32_t, 0>()));
- EXPECT_EQ(uint32_t(0), (mask_trailing_ones<uint32_t, 0>()));
- EXPECT_EQ(uint64_t(0), (mask_leading_ones<uint64_t, 0>()));
- EXPECT_EQ(uint64_t(0), (mask_trailing_ones<uint64_t, 0>()));
-
- EXPECT_EQ(uint32_t(0x00000003), (mask_trailing_ones<uint32_t, 2>()));
- EXPECT_EQ(uint32_t(0xC0000000), (mask_leading_ones<uint32_t, 2>()));
-
- EXPECT_EQ(uint32_t(0x000007FF), (mask_trailing_ones<uint32_t, 11>()));
- EXPECT_EQ(uint32_t(0xFFE00000), (mask_leading_ones<uint32_t, 11>()));
-
- EXPECT_EQ(uint32_t(0xFFFFFFFF), (mask_trailing_ones<uint32_t, 32>()));
- EXPECT_EQ(uint32_t(0xFFFFFFFF), (mask_leading_ones<uint32_t, 32>()));
- EXPECT_EQ(uint64_t(0xFFFFFFFFFFFFFFFF), (mask_trailing_ones<uint64_t, 64>()));
- EXPECT_EQ(uint64_t(0xFFFFFFFFFFFFFFFF), (mask_leading_ones<uint64_t, 64>()));
-
- EXPECT_EQ(uint64_t(0x0000FFFFFFFFFFFF), (mask_trailing_ones<uint64_t, 48>()));
- EXPECT_EQ(uint64_t(0xFFFFFFFFFFFF0000), (mask_leading_ones<uint64_t, 48>()));
+ EXPECT_EQ(0_u8, (mask_leading_ones<uint8_t, 0>()));
+ EXPECT_EQ(0_u8, (mask_trailing_ones<uint8_t, 0>()));
+ EXPECT_EQ(0_u16, (mask_leading_ones<uint16_t, 0>()));
+ EXPECT_EQ(0_u16, (mask_trailing_ones<uint16_t, 0>()));
+ EXPECT_EQ(0_u32, (mask_leading_ones<uint32_t, 0>()));
+ EXPECT_EQ(0_u32, (mask_trailing_ones<uint32_t, 0>()));
+ EXPECT_EQ(0_u64, (mask_leading_ones<uint64_t, 0>()));
+ EXPECT_EQ(0_u64, (mask_trailing_ones<uint64_t, 0>()));
+
+ EXPECT_EQ(0x00000003_u32, (mask_trailing_ones<uint32_t, 2>()));
+ EXPECT_EQ(0xC0000000_u32, (mask_leading_ones<uint32_t, 2>()));
+
+ EXPECT_EQ(0x000007FF_u32, (mask_trailing_ones<uint32_t, 11>()));
+ EXPECT_EQ(0xFFE00000_u32, (mask_leading_ones<uint32_t, 11>()));
+
+ EXPECT_EQ(0xFFFFFFFF_u32, (mask_trailing_ones<uint32_t, 32>()));
+ EXPECT_EQ(0xFFFFFFFF_u32, (mask_leading_ones<uint32_t, 32>()));
+ EXPECT_EQ(0xFFFFFFFFFFFFFFFF_u64, (mask_trailing_ones<uint64_t, 64>()));
+ EXPECT_EQ(0xFFFFFFFFFFFFFFFF_u64, (mask_leading_ones<uint64_t, 64>()));
+
+ EXPECT_EQ(0x0000FFFFFFFFFFFF_u64, (mask_trailing_ones<uint64_t, 48>()));
+ EXPECT_EQ(0xFFFFFFFFFFFF0000_u64, (mask_leading_ones<uint64_t, 48>()));
+
+ EXPECT_EQ(0_u128, (mask_trailing_ones<UInt128, 0>()));
+ EXPECT_EQ(0_u128, (mask_leading_ones<UInt128, 0>()));
+ EXPECT_EQ(0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF_u128,
+ (mask_trailing_ones<UInt128, 128>()));
+ EXPECT_EQ(0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF_u128,
+ (mask_leading_ones<UInt128, 128>()));
+ EXPECT_EQ(0x0000FFFFFFFFFFFFFFFFFFFFFFFFFFFF_u128,
+ (mask_trailing_ones<UInt128, 112>()));
+ EXPECT_EQ(0xFFFFFFFFFFFFFFFFFFFFFFFFFFFF0000_u128,
+ (mask_leading_ones<UInt128, 112>()));
}
} // namespace LIBC_NAMESPACE
diff --git a/utils/bazel/llvm-project-overlay/libc/test/src/__support/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/test/src/__support/BUILD.bazel
index 8e94a84f586f4c..19d4c7869799a0 100644
--- a/utils/bazel/llvm-project-overlay/libc/test/src/__support/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/test/src/__support/BUILD.bazel
@@ -13,7 +13,11 @@ licenses(["notice"])
libc_test(
name = "math_extras_test",
srcs = ["math_extras_test.cpp"],
- deps = ["//libc:__support_math_extras"],
+ deps = [
+ "//libc:__support_integer_literals",
+ "//libc:__support_math_extras",
+ "//libc:__support_uint128",
+ ],
)
# This test is currently disabled because of an issue in
|
lntue
reviewed
Mar 7, 2024
libc/src/__support/UInt.h
Outdated
| static_assert(count <= T_BITS && "Invalid bit index"); | ||
| using word_type = typename T::word_type; | ||
| T out; | ||
| const int chunk_index_containing_bit = static_cast<int>(count / T::WORD_SIZE); |
There was a problem hiding this comment.
If so, don't forget to capitalize the identifiers so that this doesn't regress the readability lint.
lntue
reviewed
Mar 7, 2024
libc/src/__support/UInt.h
Outdated
| static_assert(count <= T_BITS && "Invalid bit index"); | ||
| using word_type = typename T::word_type; | ||
| T out; | ||
| const int chunk_index_containing_bit = |
nickdesaulniers
approved these changes
Mar 7, 2024
lntue
approved these changes
Mar 7, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#84299 broke the arm32 build, this patch fixes it forward.