Skip to content

[libc] Remove UB specializations of type traits for BigInt #84035

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 7, 2024
Merged

[libc] Remove UB specializations of type traits for BigInt #84035

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
528b7ce
[libc] Remove UB specializations of type_traits
gchatelet Mar 5, 2024
ccabea0
Fix bug on has_single_bit
gchatelet Mar 6, 2024
71a1599
implement faster count_ones count_zeros, add documentation
gchatelet Mar 6, 2024
48b0bc8
Remove non standard bit functions specializations, add documentation …
gchatelet Mar 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions libc/src/__support/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ add_header_library(
HDRS
integer_to_string.h
DEPENDS
.uint
libc.src.__support.common
libc.src.__support.CPP.algorithm
libc.src.__support.CPP.limits
Expand Down
112 changes: 69 additions & 43 deletions libc/src/__support/CPP/bit.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,14 @@ namespace LIBC_NAMESPACE::cpp {

// This implementation of bit_cast requires trivially-constructible To, to avoid
// UB in the implementation.
template <
typename To, typename From,
typename = cpp::enable_if_t<sizeof(To) == sizeof(From) &&
cpp::is_trivially_constructible<To>::value &&
cpp::is_trivially_copyable<To>::value &&
cpp::is_trivially_copyable<From>::value>>
LIBC_INLINE constexpr To bit_cast(const From &from) {
template <typename To, typename From>
LIBC_INLINE constexpr cpp::enable_if_t<
(sizeof(To) == sizeof(From)) &&
cpp::is_trivially_constructible<To>::value &&
cpp::is_trivially_copyable<To>::value &&
cpp::is_trivially_copyable<From>::value,
To>
bit_cast(const From &from) {
MSAN_UNPOISON(&from, sizeof(From));
#if LIBC_HAS_BUILTIN(__builtin_bit_cast)
return __builtin_bit_cast(To, from);
Expand All @@ -51,8 +52,10 @@ LIBC_INLINE constexpr To bit_cast(const From &from) {
#endif // LIBC_HAS_BUILTIN(__builtin_bit_cast)
}

template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
[[nodiscard]] LIBC_INLINE constexpr bool has_single_bit(T value) {
template <typename T>
[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>,
bool>
has_single_bit(T value) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a side question: Is there a simple way to make these template functions be like:

has_single_bit(T value)   when sizeof(T) <= 8
has_single_bit(const T &value) when sizeof(T) > 8

Especially if this pattern can be applied to many functions easily somehow.
Last time I experimented with DoubleDouble and DyadicFloat<128> and it did provide measurable performance improvements.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All our functions are inline. This means that the compiler can change how the value is passed to the function. That is, we don't care about passing by value or ref.

The only thing we need to make sure is that the type is [[trivial_abi]] (blog article), otherwise the compiler is forced to pass by ref to be able to call the destructor when there is one. This is true even if the signature of the function passes the argument by value. So technically we'd need to annotate BigInt and DoubleDouble with [[clang::trivial_abi]] (or its GCC equivalent). We can also static_assert<cpp::is_trivial_v<T>> for DoubleDouble and DyadicFloat<128>.

return (value != 0) && ((value & (value - 1)) == 0);
}

Expand All @@ -70,8 +73,9 @@ template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
/// Only unsigned integral types are allowed.
///
/// Returns cpp::numeric_limits<T>::digits on an input of 0.
template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
[[nodiscard]] LIBC_INLINE constexpr int countr_zero(T value) {
template <typename T>
[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, int>
countr_zero(T value) {
if (!value)
return cpp::numeric_limits<T>::digits;
if (value & 0x1)
Expand Down Expand Up @@ -103,8 +107,9 @@ ADD_SPECIALIZATION(countr_zero, unsigned long long, __builtin_ctzll)
/// Only unsigned integral types are allowed.
///
/// Returns cpp::numeric_limits<T>::digits on an input of 0.
template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
[[nodiscard]] LIBC_INLINE constexpr int countl_zero(T value) {
template <typename T>
[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, int>
countl_zero(T value) {
if (!value)
return cpp::numeric_limits<T>::digits;
// Bisection method.
Expand Down Expand Up @@ -135,8 +140,9 @@ ADD_SPECIALIZATION(countl_zero, unsigned long long, __builtin_clzll)
/// Only unsigned integral types are allowed.
///
/// Returns cpp::numeric_limits<T>::digits on an input of all ones.
template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
[[nodiscard]] LIBC_INLINE constexpr int countl_one(T value) {
template <typename T>
[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, int>
countl_one(T value) {
return cpp::countl_zero<T>(~value);
}

Expand All @@ -147,26 +153,29 @@ template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
/// Only unsigned integral types are allowed.
///
/// Returns cpp::numeric_limits<T>::digits on an input of all ones.
template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
[[nodiscard]] LIBC_INLINE constexpr int countr_one(T value) {
template <typename T>
[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, int>
countr_one(T value) {
return cpp::countr_zero<T>(~value);
}

/// Returns the number of bits needed to represent value if value is nonzero.
/// Returns 0 otherwise.
///
/// Ex. bit_width(5) == 3.
template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
[[nodiscard]] LIBC_INLINE constexpr int bit_width(T value) {
template <typename T>
[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, int>
bit_width(T value) {
return cpp::numeric_limits<T>::digits - cpp::countl_zero(value);
}

/// Returns the largest integral power of two no greater than value if value is
/// nonzero. Returns 0 otherwise.
///
/// Ex. bit_floor(5) == 4.
template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
[[nodiscard]] LIBC_INLINE constexpr T bit_floor(T value) {
template <typename T>
[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, T>
bit_floor(T value) {
if (!value)
return 0;
return T(1) << (cpp::bit_width(value) - 1);
Expand All @@ -179,8 +188,9 @@ template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
///
/// The return value is undefined if the input is larger than the largest power
/// of two representable in T.
template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
[[nodiscard]] LIBC_INLINE constexpr T bit_ceil(T value) {
template <typename T>
[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, T>
bit_ceil(T value) {
if (value < 2)
return 1;
return T(1) << cpp::bit_width<T>(value - 1u);
Expand All @@ -190,28 +200,31 @@ template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
// from https://blog.regehr.org/archives/1063.

// Forward-declare rotr so that rotl can use it.
template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
[[nodiscard]] LIBC_INLINE constexpr T rotr(T value, int rotate);
template <typename T>
[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, T>
rotr(T value, int rotate);

template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
[[nodiscard]] LIBC_INLINE constexpr T rotl(T value, int rotate) {
template <typename T>
[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, T>
rotl(T value, int rotate) {
constexpr unsigned N = cpp::numeric_limits<T>::digits;
rotate = rotate % N;
if (!rotate)
return value;
if (rotate < 0)
return cpp::rotr(value, -rotate);
return cpp::rotr<T>(value, -rotate);
return (value << rotate) | (value >> (N - rotate));
}

template <typename T, typename>
[[nodiscard]] LIBC_INLINE constexpr T rotr(T value, int rotate) {
template <typename T>
[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, T>
rotr(T value, int rotate) {
constexpr unsigned N = cpp::numeric_limits<T>::digits;
rotate = rotate % N;
if (!rotate)
return value;
if (rotate < 0)
return cpp::rotl(value, -rotate);
return cpp::rotl<T>(value, -rotate);
return (value >> rotate) | (value << (N - rotate));
}

Expand All @@ -226,33 +239,44 @@ LIBC_INLINE constexpr To bit_or_static_cast(const From &from) {
}
}

template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
[[nodiscard]] LIBC_INLINE constexpr int first_leading_zero(T value) {
// TODO: remove from 'bit.h' as it is not a standard function.
template <typename T>
[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, int>
first_leading_zero(T value) {
return value == cpp::numeric_limits<T>::max() ? 0 : countl_one(value) + 1;
}

template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
[[nodiscard]] LIBC_INLINE constexpr int first_leading_one(T value) {
// TODO: remove from 'bit.h' as it is not a standard function.
template <typename T>
[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, int>
first_leading_one(T value) {
return first_leading_zero(static_cast<T>(~value));
}

template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
[[nodiscard]] LIBC_INLINE constexpr int first_trailing_zero(T value) {
// TODO: remove from 'bit.h' as it is not a standard function.
template <typename T>
[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, int>
first_trailing_zero(T value) {
return value == cpp::numeric_limits<T>::max()
? 0
: countr_zero(static_cast<T>(~value)) + 1;
}

template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
[[nodiscard]] LIBC_INLINE constexpr int first_trailing_one(T value) {
// TODO: remove from 'bit.h' as it is not a standard function.
template <typename T>
[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, int>
first_trailing_one(T value) {
return value == cpp::numeric_limits<T>::max() ? 0 : countr_zero(value) + 1;
}

/// Count number of 1's aka population count or hamming weight.
///
/// Only unsigned integral types are allowed.
template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
[[nodiscard]] LIBC_INLINE constexpr int count_ones(T value) {
// TODO: rename as 'popcount' to follow the standard
// https://en.cppreference.com/w/cpp/numeric/popcount
template <typename T>
[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, int>
count_ones(T value) {
int count = 0;
for (int i = 0; i != cpp::numeric_limits<T>::digits; ++i)
if ((value >> i) & 0x1)
Expand All @@ -272,8 +296,10 @@ ADD_SPECIALIZATION(unsigned long long, __builtin_popcountll)
// TODO: 128b specializations?
#undef ADD_SPECIALIZATION

template <typename T, typename = cpp::enable_if_t<cpp::is_unsigned_v<T>>>
[[nodiscard]] LIBC_INLINE constexpr int count_zeros(T value) {
// TODO: remove from 'bit.h' as it is not a standard function.
template <typename T>
[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, int>
count_zeros(T value) {
return count_ones<T>(static_cast<T>(~value));
}

Expand Down
Loading