Skip to content

Conversation

kazutakahirata
Copy link
Contributor

This patch adds llvm::bit_width_constexpr, a constexpr version of
llvm::bit_width.

The new function is intended to serve as a marker. When we switch to
C++20, we will most likely go through functions in llvm/ADT/bit.h and
replace them with their counterparts from . With
llvm::bit_width_constexpr, we can easily replace its use with
std::bit_width.

This patch refactors a couple of places. Specifically:

  • bitWidth in BitmaskEnum.h is replaced with the new function.

  • bitsRequired in PointerUnion.h is redefined in terms of the new
    function.

I've used Compiler Explorer to check the equivalence:

https://godbolt.org/z/1oKMK9Ez7

This patch adds llvm::bit_width_constexpr, a constexpr version of
llvm::bit_width.

The new function is intended to serve as a marker.  When we switch to
C++20, we will most likely go through functions in llvm/ADT/bit.h and
replace them with their counterparts from <bit>.  With
llvm::bit_width_constexpr, we can easily replace its use with
std::bit_width.

This patch refactors a couple of places.  Specifically:

- bitWidth in BitmaskEnum.h is replaced with the new function.

- bitsRequired in PointerUnion.h is redefined in terms of the new
  function.

I've used Compiler Explorer to check the equivalence:

https://godbolt.org/z/1oKMK9Ez7
@llvmbot
Copy link
Member

llvmbot commented Oct 3, 2025

@llvm/pr-subscribers-llvm-adt

Author: Kazu Hirata (kazutakahirata)

Changes

This patch adds llvm::bit_width_constexpr, a constexpr version of
llvm::bit_width.

The new function is intended to serve as a marker. When we switch to
C++20, we will most likely go through functions in llvm/ADT/bit.h and
replace them with their counterparts from <bit>. With
llvm::bit_width_constexpr, we can easily replace its use with
std::bit_width.

This patch refactors a couple of places. Specifically:

  • bitWidth in BitmaskEnum.h is replaced with the new function.

  • bitsRequired in PointerUnion.h is redefined in terms of the new
    function.

I've used Compiler Explorer to check the equivalence:

https://godbolt.org/z/1oKMK9Ez7


Full diff: https://github.com/llvm/llvm-project/pull/161775.diff

4 Files Affected:

  • (modified) llvm/include/llvm/ADT/BitmaskEnum.h (+2-5)
  • (modified) llvm/include/llvm/ADT/PointerUnion.h (+1-1)
  • (modified) llvm/include/llvm/ADT/bit.h (+17)
  • (modified) llvm/unittests/ADT/BitTest.cpp (+23)
diff --git a/llvm/include/llvm/ADT/BitmaskEnum.h b/llvm/include/llvm/ADT/BitmaskEnum.h
index 7214f25b0aa10..d464cbcfcdffd 100644
--- a/llvm/include/llvm/ADT/BitmaskEnum.h
+++ b/llvm/include/llvm/ADT/BitmaskEnum.h
@@ -14,6 +14,7 @@
 #include <utility>
 
 #include "llvm/ADT/STLForwardCompat.h"
+#include "llvm/ADT/bit.h"
 #include "llvm/Support/MathExtras.h"
 
 /// LLVM_MARK_AS_BITMASK_ENUM lets you opt in an individual enum type so you can
@@ -138,10 +139,6 @@ template <typename E> constexpr std::underlying_type_t<E> Underlying(E Val) {
   return U;
 }
 
-constexpr unsigned bitWidth(uint64_t Value) {
-  return Value ? 1 + bitWidth(Value >> 1) : 0;
-}
-
 template <typename E, typename = std::enable_if_t<is_bitmask_enum<E>::value>>
 constexpr bool operator!(E Val) {
   return Val == static_cast<E>(0);
@@ -220,7 +217,7 @@ e &operator>>=(e &lhs, e rhs) {
 // Enable bitmask enums in namespace ::llvm and all nested namespaces.
 LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
 template <typename E, typename = std::enable_if_t<is_bitmask_enum<E>::value>>
-constexpr unsigned BitWidth = BitmaskEnumDetail::bitWidth(
+constexpr unsigned BitWidth = llvm::bit_width_constexpr(
     uint64_t{llvm::to_underlying(E::LLVM_BITMASK_LARGEST_ENUMERATOR)});
 
 } // namespace llvm
diff --git a/llvm/include/llvm/ADT/PointerUnion.h b/llvm/include/llvm/ADT/PointerUnion.h
index ca0e1ed3d49d1..7b66177373011 100644
--- a/llvm/include/llvm/ADT/PointerUnion.h
+++ b/llvm/include/llvm/ADT/PointerUnion.h
@@ -31,7 +31,7 @@ namespace pointer_union_detail {
   /// Determine the number of bits required to store integers with values < n.
   /// This is ceil(log2(n)).
   constexpr int bitsRequired(unsigned n) {
-    return n > 1 ? 1 + bitsRequired((n + 1) / 2) : 0;
+    return n == 0 ? 0 : llvm::bit_width_constexpr(n - 1);
   }
 
   template <typename... Ts> constexpr int lowBitsAvailable() {
diff --git a/llvm/include/llvm/ADT/bit.h b/llvm/include/llvm/ADT/bit.h
index 67c0a1c3300fa..e419ea975c02e 100644
--- a/llvm/include/llvm/ADT/bit.h
+++ b/llvm/include/llvm/ADT/bit.h
@@ -292,6 +292,23 @@ template <typename T> [[nodiscard]] int bit_width(T Value) {
   return std::numeric_limits<T>::digits - llvm::countl_zero(Value);
 }
 
+/// Returns the number of bits needed to represent Value if Value is nonzero.
+/// Returns 0 otherwise.
+///
+/// A constexpr version of bit_width.
+///
+/// Ex. bit_width_constexpr(5) == 3.
+template <typename T> [[nodiscard]] constexpr int bit_width_constexpr(T Value) {
+  static_assert(std::is_unsigned_v<T>,
+                "Only unsigned integral types are allowed.");
+  int Width = 0;
+  while (Value > 0) {
+    Value >>= 1;
+    Width++;
+  }
+  return Width;
+}
+
 /// Returns the largest integral power of two no greater than Value if Value is
 /// nonzero.  Returns 0 otherwise.
 ///
diff --git a/llvm/unittests/ADT/BitTest.cpp b/llvm/unittests/ADT/BitTest.cpp
index 88ae36c44bdb9..eaed4e1fe327d 100644
--- a/llvm/unittests/ADT/BitTest.cpp
+++ b/llvm/unittests/ADT/BitTest.cpp
@@ -247,6 +247,29 @@ TEST(BitTest, BitWidth) {
   EXPECT_EQ(64, llvm::bit_width(uint64_t(0xffffffffffffffffull)));
 }
 
+TEST(BitTest, BitWidthConstexpr) {
+  static_assert(llvm::bit_width_constexpr(0u) == 0);
+  static_assert(llvm::bit_width_constexpr(1u) == 1);
+  static_assert(llvm::bit_width_constexpr(2u) == 2);
+  static_assert(llvm::bit_width_constexpr(3u) == 2);
+  static_assert(llvm::bit_width_constexpr(4u) == 3);
+  static_assert(llvm::bit_width_constexpr(5u) == 3);
+  static_assert(llvm::bit_width_constexpr(6u) == 3);
+  static_assert(llvm::bit_width_constexpr(7u) == 3);
+  static_assert(llvm::bit_width_constexpr(8u) == 4);
+
+  static_assert(llvm::bit_width_constexpr(255u) == 8);
+  static_assert(llvm::bit_width_constexpr(256u) == 9);
+  static_assert(llvm::bit_width_constexpr(257u) == 9);
+
+  static_assert(
+      llvm::bit_width_constexpr(std::numeric_limits<uint16_t>::max()) == 16);
+  static_assert(
+      llvm::bit_width_constexpr(std::numeric_limits<uint32_t>::max()) == 32);
+  static_assert(
+      llvm::bit_width_constexpr(std::numeric_limits<uint64_t>::max()) == 64);
+}
+
 TEST(BitTest, CountlZero) {
   uint8_t Z8 = 0;
   uint16_t Z16 = 0;

@kazutakahirata kazutakahirata merged commit 30f7c5d into llvm:main Oct 4, 2025
9 checks passed
@kazutakahirata kazutakahirata deleted the cleanup_20251002_ADT_bit_width_constexpr branch October 4, 2025 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants