[llvm] 095cad6 - [ADT] Simplify Bitfields.h (NFC) (#163035)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 13 08:41:04 PDT 2025
Author: Kazu Hirata
Date: 2025-10-13T08:41:00-07:00
New Revision: 095cad6add16df3f6273f5b24293e48a08e3230e
URL: https://github.com/llvm/llvm-project/commit/095cad6add16df3f6273f5b24293e48a08e3230e
DIFF: https://github.com/llvm/llvm-project/commit/095cad6add16df3f6273f5b24293e48a08e3230e.diff
LOG: [ADT] Simplify Bitfields.h (NFC) (#163035)
BitPatterns and Compressor collectively provide three main features:
- check the value range
- truncate the value before going into the bitfield
- sign-extend the signed bitfield
This patch retains the range check as separate function checkValue
while inlining the rest into their respective callers, update() and
extract().
Added:
Modified:
llvm/include/llvm/ADT/Bitfields.h
llvm/unittests/ADT/BitFieldsTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/ADT/Bitfields.h b/llvm/include/llvm/ADT/Bitfields.h
index 4064d716f8a77..1af276145ba48 100644
--- a/llvm/include/llvm/ADT/Bitfields.h
+++ b/llvm/include/llvm/ADT/Bitfields.h
@@ -86,89 +86,43 @@
#include <limits> // numeric_limits
#include <type_traits>
+#include "llvm/Support/MathExtras.h"
+
namespace llvm {
namespace bitfields_details {
-/// A struct defining useful bit patterns for n-bits integer types.
-template <typename T, unsigned Bits> struct BitPatterns {
- /// Bit patterns are forged using the equivalent `Unsigned` type because of
- /// undefined operations over signed types (e.g. Bitwise shift operators).
- /// Moreover same size casting from unsigned to signed is well defined but not
- /// the other way around.
- using Unsigned = std::make_unsigned_t<T>;
- static_assert(sizeof(Unsigned) == sizeof(T), "Types must have same size");
-
- static constexpr unsigned TypeBits = sizeof(Unsigned) * CHAR_BIT;
- static_assert(TypeBits >= Bits, "n-bit must fit in T");
-
- /// e.g. with TypeBits == 8 and Bits == 6.
- static constexpr Unsigned AllZeros = Unsigned(0); // 00000000
- static constexpr Unsigned AllOnes = ~Unsigned(0); // 11111111
- static constexpr Unsigned Umin = AllZeros; // 00000000
- static constexpr Unsigned Umax = AllOnes >> (TypeBits - Bits); // 00111111
- static constexpr Unsigned SignBitMask = Unsigned(1) << (Bits - 1); // 00100000
- static constexpr Unsigned Smax = Umax >> 1U; // 00011111
- static constexpr Unsigned Smin = ~Smax; // 11100000
- static constexpr Unsigned SignExtend = Unsigned(Smin << 1U); // 11000000
-};
-
-/// `Compressor` is used to manipulate the bits of a (possibly signed) integer
-/// type so it can be packed and unpacked into a `bits` sized integer,
-/// `Compressor` is specialized on signed-ness so no runtime cost is incurred.
-/// The `pack` method also checks that the passed in `UserValue` is valid.
-template <typename T, unsigned Bits, bool = std::is_unsigned<T>::value>
-struct Compressor {
- static_assert(std::is_unsigned<T>::value, "T must be unsigned");
- using BP = BitPatterns<T, Bits>;
-
- static T pack(T UserValue, T UserMaxValue) {
- assert(UserValue <= UserMaxValue && "value is too big");
- assert(UserValue <= BP::Umax && "value is too big");
- return UserValue;
- }
-
- static T unpack(T StorageValue) { return StorageValue; }
-};
-
-template <typename T, unsigned Bits> struct Compressor<T, Bits, false> {
- static_assert(std::is_signed<T>::value, "T must be signed");
- using BP = BitPatterns<T, Bits>;
-
- static T pack(T UserValue, T UserMaxValue) {
- assert(UserValue <= UserMaxValue && "value is too big");
- assert(UserValue <= T(BP::Smax) && "value is too big");
- assert(UserValue >= T(BP::Smin) && "value is too small");
- if (UserValue < 0)
- UserValue &= ~BP::SignExtend;
- return UserValue;
- }
-
- static T unpack(T StorageValue) {
- if (StorageValue >= T(BP::SignBitMask))
- StorageValue |= BP::SignExtend;
- return StorageValue;
- }
-};
-
/// Impl is where Bifield description and Storage are put together to interact
/// with values.
template <typename Bitfield, typename StorageType> struct Impl {
static_assert(std::is_unsigned<StorageType>::value,
"Storage must be unsigned");
using IntegerType = typename Bitfield::IntegerType;
- using C = Compressor<IntegerType, Bitfield::Bits>;
- using BP = BitPatterns<StorageType, Bitfield::Bits>;
static constexpr size_t StorageBits = sizeof(StorageType) * CHAR_BIT;
static_assert(Bitfield::FirstBit <= StorageBits, "Data must fit in mask");
static_assert(Bitfield::LastBit <= StorageBits, "Data must fit in mask");
- static constexpr StorageType Mask = BP::Umax << Bitfield::Shift;
+ static constexpr StorageType LowMask =
+ maskTrailingOnes<StorageType>(Bitfield::Bits);
+ static constexpr StorageType Mask = LowMask << Bitfield::Shift;
+
+ /// Validates that `UserValue` fits within the bitfield's range.
+ static void checkValue(IntegerType UserValue, IntegerType UserMaxValue) {
+ assert(UserValue <= UserMaxValue && "value is too big");
+ if constexpr (std::is_unsigned_v<IntegerType>) {
+ assert(isUInt<Bitfield::Bits>(UserValue) && "value is too big");
+ } else {
+ static_assert(std::is_signed_v<IntegerType>,
+ "IntegerType must be signed");
+ assert(isInt<Bitfield::Bits>(UserValue) && "value is out of range");
+ }
+ }
/// Checks `UserValue` is within bounds and packs it between `FirstBit` and
/// `LastBit` of `Packed` leaving the rest unchanged.
static void update(StorageType &Packed, IntegerType UserValue) {
- const StorageType StorageValue = C::pack(UserValue, Bitfield::UserMaxValue);
+ checkValue(UserValue, Bitfield::UserMaxValue);
+ const StorageType StorageValue = UserValue & LowMask;
Packed &= ~Mask;
Packed |= StorageValue << Bitfield::Shift;
}
@@ -177,7 +131,9 @@ template <typename Bitfield, typename StorageType> struct Impl {
/// an`IntegerType`.
static IntegerType extract(StorageType Packed) {
const StorageType StorageValue = (Packed & Mask) >> Bitfield::Shift;
- return C::unpack(StorageValue);
+ if constexpr (std::is_signed_v<IntegerType>)
+ return SignExtend64<Bitfield::Bits>(StorageValue);
+ return StorageValue;
}
/// Interprets bits between `FirstBit` and `LastBit` of `Packed` as
diff --git a/llvm/unittests/ADT/BitFieldsTest.cpp b/llvm/unittests/ADT/BitFieldsTest.cpp
index 3062d5d7f293c..ae541feda90a3 100644
--- a/llvm/unittests/ADT/BitFieldsTest.cpp
+++ b/llvm/unittests/ADT/BitFieldsTest.cpp
@@ -247,8 +247,8 @@ TEST(BitfieldsTest, ValueTooBigBounded) {
Bitfield::set<A>(Storage, 0);
Bitfield::set<A>(Storage, -1);
Bitfield::set<A>(Storage, -2);
- EXPECT_DEBUG_DEATH(Bitfield::set<A>(Storage, 2), "value is too big");
- EXPECT_DEBUG_DEATH(Bitfield::set<A>(Storage, -3), "value is too small");
+ EXPECT_DEBUG_DEATH(Bitfield::set<A>(Storage, 2), "value is out of range");
+ EXPECT_DEBUG_DEATH(Bitfield::set<A>(Storage, -3), "value is out of range");
}
#endif
More information about the llvm-commits
mailing list