[ADT] Add Bitfield utilities - design #2

[gchatelet]

Summary

Alternative design to D81580 with the following change in API

using Bool = Bitfield<bool, 0, 1>;

// testField becomes cast to bool operator

before: if(Bool::testField(Storage))
after:  if(bitfield<Bool>(Storage).isSet())

// getField becomes cast to UserType
before: auto Value = Bool::getField(Storage);
after:  auto Value = bitfield<Bool>(Storage);

// setField becomes returning a UserType assignable object
before: Bool::setField(Storage, Value);
after:  bitfield<Bool>(Storage) = Value;

[gchatelet]

• update API documentation

[courbet]

llvm/include/llvm/ADT/Bitfields.h

144

The conversion from/to enum/bool are done in the public API outside of this template where is that ?

197

I'm actually a bit frightened by this, even with the guaranteed elision in c++, this is still dangerous: https://godbolt.org/z/t-Gzi7

The previous Bitfield::setField() approach has the same issue, but at least we're returning a reference and not a value, so the issue is more visible.

246

This is only a type, make ctor private to make it clear that it's not meant to be instantiated ?

266

This should not be static. It can be inline, but this is already implied by the template.

llvm/unittests/ADT/BitFieldsTest.cpp

22

I think @serge-sans-paille was suggesting to go one step further : do away with the bitfield function, and replace it with the ctor directly: Bool(Storage) = true.

[gchatelet]

llvm/include/llvm/ADT/Bitfields.h

144

Outdated comment, thx for noticing.
The documentation is not polished yet, I'd like to get in agreement on the API code first.
Keeping your comment not Done for now.

197

Yeah that's why I'm not fond of this version, the object semantic adds its own set of gotchas on top of the already complex functional code.

llvm/unittests/ADT/BitFieldsTest.cpp

22

Yes unfortunately this is not possible as Storage type needs to be deduced somehow.
e.g.

template<typename UserType, unsigned Offset, unsigned Size, UserType MaxValue = ...>
struct Bitfield {
  Bitfield(StorageType &Storage) // where does StorageType comes from?
};

The type would need to be part of the template arguments.
e.g.

template<typename StorageType, typename UserType, unsigned Offset, unsigned Size, UserType MaxValue = ...>
struct Bitfield {
  Bitfield(StorageType &Storage) //OK
};

But I believe this is confusing to declare (e.g. Bitfield<uint64_t, uint_8t, 0, 4>) and would require a lot of editing in case the StorageType changes.

The function is the trick to deduce StorageType automatically.

[courbet]

llvm/unittests/ADT/BitFieldsTest.cpp

22

That makes sense. Then maybe at least make the function static and part of the class ?

using Bool = Bitfield<bool, 0, 1>;
Bool::fieldOf(Storage) = true;

That's a midpoint between the current proposal:

using Bool = Bitfield<bool, 0, 1>;
bitfield<Bool>(Storage) = true;

And the previous one:

using Bool = Bitfield<bool, 0, 1>;
Bool::setField(Storage, true);

[gchatelet]

llvm/unittests/ADT/BitFieldsTest.cpp

22

@jfb would you have some time to give your thoughts on which approach is best? (I've added you as a reviewer since you seemed interested in abstractions when I introduced Align.)
@serge-sans-paille what do you think of the API you suggested ?

[gchatelet]

rebase

[serge-sans-paille]

Additional thoughts: even though there wasn't a lot of answers to your initial thread on llvm-dev, I'd post a followup with a few examples of the two approaches to gather some feedback from the community. I personally like this one, obviously :-)

llvm/unittests/ADT/BitFieldsTest.cpp

22

@gchatelet : I really like the syntax eye candy. That looks very readable to me. thanks for going as far as implementing an alternate version.

[gchatelet]

Dropped in favor of D82454

llvm/include/llvm/ADT/Bitfields.h

+//===-- llvm/ADT/Bitfield.h - Get and Set bits in an integer ---*- C++ -*--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// This file implements methods to test, set and extract typed bits from packed
+/// unsigned integers.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_ADT_BITFIELDS_H
+#define LLVM_ADT_BITFIELDS_H
+
+#include <cassert>
+#include <climits> // CHAR_BIT
+#include <cstddef> // size_t
+#include <cstdint> // uintXX_t
+#include <limits>  // numeric_limits
+#include <type_traits>
+
+/// Helpers to pack / unpack typed bitfields into an unsigned integer.
+/// e.g.
+///
+///  uint8_t Storage = 0;
+///
+///  // Store and retrieve a single bit as bool.
+///  using Bool = Bitfield<bool, 0, 1>;
+///  bitfield<Bool>(Storage) = true;
+///  EXPECT_EQ(Storage, 0b00000001);
+///  //                          ^
+///  EXPECT_EQ(bitfield<Bool>(Storage), true);
+///
+///  // Store and retrieve a 2 bit typed enum.
+///  // Note: enum underlying type must be unsigned.
+///  enum class SuitEnum : uint8_t { CLUBS, DIAMONDS, HEARTS, SPADES };
+///  // Note: enum maximum value needs to be passed in.
+///  using Suit = Bitfield<SuitEnum, 1, 2, SuitEnum::SPADES>;
+///  bitfield<Suit>(Storage) = SuitEnum::HEARTS;
+///  EXPECT_EQ(Storage, 0b00000101);
+///  //                        ^^
+///  EXPECT_EQ(bitfield<Suit>(Storage), SuitEnum::HEARTS);
+///
+///  // Store and retrieve a 5 bit value as unsigned.
+///  using Value = Bitfield<unsigned, 3, 5>;
+///  bitfield<Value>(Storage) = 10;
+///  EXPECT_EQ(Storage, 0b01010101);
+///  //                   ^^^^^
+///  EXPECT_EQ(bitfield<Value>(Storage), 10U);
+///
+///  // Interpret the same 5 bit value as signed.
+///  using SignedValue = Bitfield<int, 3, 5>;
+///  bitfield<SignedValue>(Storage) = -2;
+///  EXPECT_EQ(Storage, 0b11110101);
+///  //                   ^^^^^
+///  EXPECT_EQ(bitfield<SignedValue>(Storage), -2);
+///
+///  // Ability to efficiently test if a field is non zero.
+///  EXPECT_TRUE(bitfield<Value>(Storage).isSet());
+///
+///  // Alter Storage changes value.
+///  Storage = 0;
+///  EXPECT_EQ(bitfield<Bool>(Storage), false);
+///  EXPECT_EQ(bitfield<Suit>(Storage), SuitEnum::CLUBS);
+///  EXPECT_EQ(bitfield<Value>(Storage), 0U);
+///  EXPECT_EQ(bitfield<SignedValue>(Storage), 0);
+///
+///  Storage = 255;
+///  EXPECT_EQ(bitfield<Bool>(Storage), true);
+///  EXPECT_EQ(bitfield<Suit>(Storage), SuitEnum::SPADES);
+///  EXPECT_EQ(bitfield<Value>(Storage), 31U);
+///  EXPECT_EQ(bitfield<SignedValue>(Storage), -1);
+
+namespace llvm {
+
+namespace 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 = typename std::make_unsigned<T>::type;
+  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 = 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 is 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 is 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;
+  }
+};
+
+/// `BitfieldProperties` stores important properties about the Bitfield.
+/// It only deals with proper integer types. The conversion from/to enum/bool
+/// are done in the public API outside of this template.
+template <typename UserType, typename T, unsigned Offset, unsigned Bits,
+          T MaxValue = std::numeric_limits<T>::max(),
+          T MinValue = std::numeric_limits<T>::min()>
+struct BitfieldProperties {
+  using IntegerType = T;
+  static_assert(std::is_integral<T>::value &&
+                    std::numeric_limits<T>::is_integer,
+                "T must be an integer type");
+
+  static constexpr size_t TypeBits = sizeof(IntegerType) * CHAR_BIT;
+  static constexpr unsigned FirstBit = Offset;
+  static constexpr unsigned LastBit = Offset + Bits;
+
+  static_assert(Bits > 0, "Bits must be non zero");
+  static_assert(Bits <= TypeBits, "Bits may not be greater than T size");
+
+  /// Impl is where Bifield description and storage are put together to interact
+  /// with values.
+  template <typename StorageType> struct Impl {
+  private:
+    static_assert(std::is_unsigned<StorageType>::value,
+                  "Storage must be unsigned");
+    using C = Compressor<IntegerType, Bits>;
+    using BP = BitPatterns<StorageType, Bits>;
+
+    static constexpr size_t StorageBits = sizeof(StorageType) * CHAR_BIT;
+    static_assert(FirstBit <= StorageBits, "Data must fit in mask");
+    static_assert(LastBit <= StorageBits, "Data must fit in mask");
+    static constexpr StorageType Mask = BP::Umax << Offset;
+
+    /// 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, MaxValue);
+      Packed &= ~Mask;
+      Packed |= StorageValue << Offset;
+    }
+
+    /// Interprets bits between `FirstBit` and `LastBit` of `Packed` as an
+    /// `IntegerType`.
+    static IntegerType extract(StorageType Packed) {
+      const StorageType StorageValue = (Packed & Mask) >> Offset;
+      return C::unpack(StorageValue);
+    }
+
+    StorageType *StoragePtr = nullptr;
+
+  public:
+    Impl(StorageType &StorageValue) : StoragePtr(&StorageValue) {}
+
+    Impl(const Impl &) = delete;
+    Impl(Impl &&) = default; // needed to return the value
+    Impl &operator=(const Impl &) = delete;
+    Impl &operator=(Impl &&) = delete;
+
+    StorageType isSet() const { return *StoragePtr & Mask; }
+
+    operator UserType() const {
+      return static_cast<UserType>(extract(*StoragePtr));
+    }
+
+    Impl &operator=(UserType UserValue) {
+      update(*StoragePtr, static_cast<IntegerType>(UserValue));
+      return *this;
+    }
+  };
+};
+
+/// `Bitfield` deals with the following type:
+/// - unsigned enums
+/// - signed and unsigned integer
+/// - `bool`
+/// Internally though we only manipulate integer with well defined and
+/// consistent semantic, this excludes typed enums and `bool` that are
+/// replaced with their unsigned counterparts.
+/// The correct type is restored in the public API.
+template <typename T, bool = std::is_enum<T>::value>
+struct ResolveUnderlyingType {
+  using type = typename std::underlying_type<T>::type;
+};
+template <typename T> struct ResolveUnderlyingType<T, false> {
+  using type = T;
+};
+template <> struct ResolveUnderlyingType<bool, false> {
+  /// In case sizeof(bool) != 1, replace `void` by an additionnal
+  /// std::conditionnal.
+  using type = std::conditional<sizeof(bool) == 1, uint8_t, void>::type;
+};
+
+} // namespace details
+
+/// A struct to hold the bitfield informations.
+/// \param T, the type of the field once in unpacked form,
+/// \param Offset, the position of the first bit,
+/// \param Size, the size of the field,
+/// \param MaxValue, For enums the maximum enum allowed.
+template <typename T, unsigned Offset, unsigned Size,
+          T MaxValue = std::is_enum<T>::value
+                           ? T(0) // coupled with static_assert below
+                           : std::numeric_limits<T>::max()>
+struct Bitfield {
+  using Type = T;
+  using IntegerType = typename details::ResolveUnderlyingType<T>::type;
+  using Properties =
+      typename details::BitfieldProperties<T, IntegerType, Offset, Size,
+                                           static_cast<IntegerType>(MaxValue)>;
+
+  static_assert(!std::is_enum<T>::value || MaxValue != T(0),
+                "Enum Bitfields must provide a MaxValue");
+  static_assert(!std::is_enum<T>::value || std::is_unsigned<IntegerType>::value,
+                "Enum must be unsigned");
+};
+
+template <typename Bitfield, typename StorageType>
+auto bitfield(StorageType &StorageValue) {
+  using Impl = typename Bitfield::Properties::template Impl<StorageType>;
+  return Impl(StorageValue);
+}
+
+/// Returns whether the two bitfields share common bits.
+template <typename A, typename B> static constexpr bool isOverlapping() {
+  using PA = typename A::Properties;
+  using PB = typename B::Properties;
+  return PA::LastBit > PB::FirstBit && PB::LastBit > PA::FirstBit;
+}
+
+} // namespace llvm
+
+#endif // LLVM_ADT_BITFIELDS_H

llvm/unittests/ADT/BitFieldsTest.cpp

+//===- llvm/unittests/ADT/BitFieldsTest.cpp - BitFields unit tests --------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ADT/Bitfields.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+namespace {
+
+TEST(BitfieldsTest, Example) {
+  uint8_t Storage = 0;
+
+  // Store and retrieve a single bit as bool.
+  using Bool = Bitfield<bool, 0, 1>;
+  bitfield<Bool>(Storage) = true;
+  EXPECT_EQ(Storage, 0b00000001);
+  //                          ^
+  EXPECT_EQ(bitfield<Bool>(Storage), true);
+
+  // Store and retrieve a 2 bit typed enum.
+  // Note: enum underlying type must be unsigned.
+  enum class SuitEnum : uint8_t { CLUBS, DIAMONDS, HEARTS, SPADES };
+  // Note: enum maximum value needs to be passed in.
+  using Suit = Bitfield<SuitEnum, 1, 2, SuitEnum::SPADES>;
+  bitfield<Suit>(Storage) = SuitEnum::HEARTS;
+  EXPECT_EQ(Storage, 0b00000101);
+  //                        ^^
+  EXPECT_EQ(bitfield<Suit>(Storage), SuitEnum::HEARTS);
+
+  // Store and retrieve a 5 bit value as unsigned.
+  using Value = Bitfield<unsigned, 3, 5>;
+  bitfield<Value>(Storage) = 10;
+  EXPECT_EQ(Storage, 0b01010101);
+  //                   ^^^^^
+  EXPECT_EQ(bitfield<Value>(Storage), 10U);
+
+  // Interpret the same 5 bit value as signed.
+  using SignedValue = Bitfield<int, 3, 5>;
+  bitfield<SignedValue>(Storage) = -2;
+  EXPECT_EQ(Storage, 0b11110101);
+  //                   ^^^^^
+  EXPECT_EQ(bitfield<SignedValue>(Storage), -2);
+
+  // Ability to efficiently test if a field is non zero.
+  EXPECT_TRUE(bitfield<Value>(Storage).isSet());
+
+  // Alter Storage changes value.
+  Storage = 0;
+  EXPECT_EQ(bitfield<Bool>(Storage), false);
+  EXPECT_EQ(bitfield<Suit>(Storage), SuitEnum::CLUBS);
+  EXPECT_EQ(bitfield<Value>(Storage), 0U);
+  EXPECT_EQ(bitfield<SignedValue>(Storage), 0);
+
+  Storage = 255;
+  EXPECT_EQ(bitfield<Bool>(Storage), true);
+  EXPECT_EQ(bitfield<Suit>(Storage), SuitEnum::SPADES);
+  EXPECT_EQ(bitfield<Value>(Storage), 31U);
+  EXPECT_EQ(bitfield<SignedValue>(Storage), -1);
+}
+
+TEST(BitfieldsTest, FirstBit) {
+  uint8_t Storage = 0;
+  using FirstBit = Bitfield<bool, 0, 1>;
+  // Set true
+  bitfield<FirstBit>(Storage) = true;
+  EXPECT_EQ(bitfield<FirstBit>(Storage), true);
+  EXPECT_EQ(Storage, 0x1ULL);
+  // Set false
+  bitfield<FirstBit>(Storage) = false;
+  EXPECT_EQ(bitfield<FirstBit>(Storage), false);
+  EXPECT_EQ(Storage, 0x0ULL);
+}
+
+TEST(BitfieldsTest, SecondBit) {
+  uint8_t Storage = 0;
+  using SecondBit = Bitfield<bool, 1, 1>;
+  // Set true
+  bitfield<SecondBit>(Storage) = true;
+  EXPECT_EQ(bitfield<SecondBit>(Storage), true);
+  EXPECT_EQ(Storage, 0x2ULL);
+  // Set false
+  bitfield<SecondBit>(Storage) = false;
+  EXPECT_EQ(bitfield<SecondBit>(Storage), false);
+  EXPECT_EQ(Storage, 0x0ULL);
+}
+
+TEST(BitfieldsTest, LastBit) {
+  uint8_t Storage = 0;
+  using LastBit = Bitfield<bool, 7, 1>;
+  // Set true
+  bitfield<LastBit>(Storage) = true;
+  EXPECT_EQ(bitfield<LastBit>(Storage), true);
+  EXPECT_EQ(Storage, 0x80ULL);
+  // Set false
+  bitfield<LastBit>(Storage) = false;
+  EXPECT_EQ(bitfield<LastBit>(Storage), false);
+  EXPECT_EQ(Storage, 0x0ULL);
+}
+
+TEST(BitfieldsTest, LastBitUint64) {
+  uint64_t Storage = 0;
+  using LastBit = Bitfield<bool, 63, 1>;
+  // Set true
+  bitfield<LastBit>(Storage) = true;
+  EXPECT_EQ(bitfield<LastBit>(Storage), true);
+  EXPECT_EQ(Storage, 0x8000000000000000ULL);
+  // Set false
+  bitfield<LastBit>(Storage) = false;
+  EXPECT_EQ(bitfield<LastBit>(Storage), false);
+  EXPECT_EQ(Storage, 0x0ULL);
+}
+
+TEST(BitfieldsTest, Enum) {
+  enum Enum : unsigned { Zero = 0, Two = 2, LAST = Two };
+
+  uint8_t Storage = 0;
+  using OrderingField = Bitfield<Enum, 1, 2, LAST>;
+  EXPECT_EQ(bitfield<OrderingField>(Storage), Zero);
+  bitfield<OrderingField>(Storage) = Two;
+  EXPECT_EQ(bitfield<OrderingField>(Storage), Two);
+  EXPECT_EQ(Storage, 0b00000100);
+  // value 2 in             ^^
+}
+
+TEST(BitfieldsTest, EnumClass) {
+  enum class Enum : unsigned { Zero = 0, Two = 2, LAST = Two };
+
+  uint8_t Storage = 0;
+  using OrderingField = Bitfield<Enum, 1, 2, Enum::LAST>;
+  EXPECT_EQ(bitfield<OrderingField>(Storage), Enum::Zero);
+  bitfield<OrderingField>(Storage) = Enum::Two;
+  EXPECT_EQ(bitfield<OrderingField>(Storage), Enum::Two);
+  EXPECT_EQ(Storage, 0b00000100);
+  // value 2 in             ^^
+}
+
+TEST(BitfieldsTest, OneBitSigned) {
+  uint8_t Storage = 0;
+  using SignedField = Bitfield<int, 1, 1>;
+  EXPECT_EQ(bitfield<SignedField>(Storage), 0);
+  EXPECT_EQ(Storage, 0b00000000);
+  // value 0 in              ^
+  bitfield<SignedField>(Storage) = -1;
+  EXPECT_EQ(bitfield<SignedField>(Storage), -1);
+  EXPECT_EQ(Storage, 0b00000010);
+  // value 1 in              ^
+}
+
+TEST(BitfieldsTest, TwoBitSigned) {
+  uint8_t Storage = 0;
+  using SignedField = Bitfield<int, 1, 2>;
+  EXPECT_EQ(bitfield<SignedField>(Storage), 0);
+  EXPECT_EQ(Storage, 0b00000000);
+  // value 0 in             ^^
+  bitfield<SignedField>(Storage) = 1;
+  EXPECT_EQ(bitfield<SignedField>(Storage), 1);
+  EXPECT_EQ(Storage, 0b00000010);
+  // value 1 in             ^^
+  bitfield<SignedField>(Storage) = -1;
+  EXPECT_EQ(bitfield<SignedField>(Storage), -1);
+  EXPECT_EQ(Storage, 0b00000110);
+  // value -1 in            ^^
+  bitfield<SignedField>(Storage) = -2;
+  EXPECT_EQ(bitfield<SignedField>(Storage), -2);
+  EXPECT_EQ(Storage, 0b00000100);
+  // value -2 in            ^^
+}
+
+TEST(BitfieldsTest, isOverlapping) {
+  //    01234567
+  // A: --------
+  // B:    ---
+  // C:  ---
+  // D:     ---
+  using A = Bitfield<unsigned, 0, 8>;
+  using B = Bitfield<unsigned, 3, 3>;
+  using C = Bitfield<unsigned, 1, 3>;
+  using D = Bitfield<unsigned, 4, 3>;
+  EXPECT_TRUE((isOverlapping<A, B>()));
+  EXPECT_TRUE((isOverlapping<A, C>()));
+  EXPECT_TRUE((isOverlapping<A, B>()));
+  EXPECT_TRUE((isOverlapping<A, D>()));
+
+  EXPECT_TRUE((isOverlapping<B, C>()));
+  EXPECT_TRUE((isOverlapping<B, D>()));
+  EXPECT_FALSE((isOverlapping<C, D>()));
+}
+
+TEST(BitfieldsTest, FullUint64) {
+  uint64_t Storage = 0;
+  using Value = Bitfield<uint64_t, 0, 64>;
+  bitfield<Value>(Storage) = -1ULL;
+  EXPECT_EQ(bitfield<Value>(Storage), -1ULL);
+  bitfield<Value>(Storage) = 0ULL;
+  EXPECT_EQ(bitfield<Value>(Storage), 0ULL);
+}
+
+TEST(BitfieldsTest, FullInt64) {
+  uint64_t Storage = 0;
+  using Value = Bitfield<int64_t, 0, 64>;
+  bitfield<Value>(Storage) = -1;
+  EXPECT_EQ(bitfield<Value>(Storage), -1);
+  bitfield<Value>(Storage) = 0;
+  EXPECT_EQ(bitfield<Value>(Storage), 0);
+}
+
+#ifdef EXPECT_DEBUG_DEATH
+
+TEST(BitfieldsTest, ValueTooBigBool) {
+  uint64_t Storage = 0;
+  using A = Bitfield<unsigned, 0, 1>;
+  bitfield<A>(Storage) = true;
+  bitfield<A>(Storage) = false;
+  EXPECT_DEBUG_DEATH(bitfield<A>(Storage) = 2, "value is too big");
+}
+
+TEST(BitfieldsTest, ValueTooBigInt) {
+  uint64_t Storage = 0;
+  using A = Bitfield<unsigned, 0, 2>;
+  bitfield<A>(Storage) = 3;
+  EXPECT_DEBUG_DEATH(bitfield<A>(Storage) = 4, "value is too big");
+  EXPECT_DEBUG_DEATH(bitfield<A>(Storage) = -1, "value is too big");
+}
+
+TEST(BitfieldsTest, ValueTooBigBounded) {
+  uint8_t Storage = 0;
+  using A = Bitfield<int, 1, 2>;
+  bitfield<A>(Storage) = 1;
+  bitfield<A>(Storage) = 0;
+  bitfield<A>(Storage) = -1;
+  bitfield<A>(Storage) = -2;
+  EXPECT_DEBUG_DEATH(bitfield<A>(Storage) = 2, "value is too big");
+  EXPECT_DEBUG_DEATH(bitfield<A>(Storage) = -3, "value is too small");
+}
+
+#endif
+
+} // namespace

llvm/unittests/ADT/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Support
   )
 
 add_llvm_unittest(ADTTests
   AnyTest.cpp
   APFloatTest.cpp
   APIntTest.cpp
   APSIntTest.cpp
   ArrayRefTest.cpp
+  BitFieldsTest.cpp
   BitmaskEnumTest.cpp
   BitVectorTest.cpp
   BreadthFirstIteratorTest.cpp
   BumpPtrListTest.cpp
   CoalescingBitVectorTest.cpp
   DAGDeltaAlgorithmTest.cpp
   DeltaAlgorithmTest.cpp
   DenseMapTest.cpp