Skip to content

Conversation

kazutakahirata
Copy link
Contributor

This patch "inlines" PackedVectorBase into its sole user PackedVector.
The two variants of PackedVectorBase are dispatched with "if
constexpr". getValue and setValue are now non-static methods of
PackedVector.

We could further simplify getValue and setValue by storing signed
integers as two's complement, but that's a change for another day.
This patch focuses on the code organization, like removing the
template trick and inheritance and making the two methods non-static.

This patch "inlines" PackedVectorBase into its sole user PackedVector.
The two variants of PackedVectorBase are dispatched with "if
constexpr".  getValue and setValue are now non-static methods of
PackedVector.

We could further simplify getValue and setValue by storing signed
integers as two's complement, but that's a change for another day.
This patch focuses on the code organization, like removing the
template trick and inheritance and making the two methods non-static.
@llvmbot
Copy link
Member

llvmbot commented Sep 29, 2025

@llvm/pr-subscribers-llvm-adt

Author: Kazu Hirata (kazutakahirata)

Changes

This patch "inlines" PackedVectorBase into its sole user PackedVector.
The two variants of PackedVectorBase are dispatched with "if
constexpr". getValue and setValue are now non-static methods of
PackedVector.

We could further simplify getValue and setValue by storing signed
integers as two's complement, but that's a change for another day.
This patch focuses on the code organization, like removing the
template trick and inheritance and making the two methods non-static.


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

1 Files Affected:

  • (modified) llvm/include/llvm/ADT/PackedVector.h (+36-53)
diff --git a/llvm/include/llvm/ADT/PackedVector.h b/llvm/include/llvm/ADT/PackedVector.h
index 77fcbf24b2861..09c20e39d1552 100644
--- a/llvm/include/llvm/ADT/PackedVector.h
+++ b/llvm/include/llvm/ADT/PackedVector.h
@@ -20,53 +20,6 @@
 
 namespace llvm {
 
-template <typename T, unsigned BitNum, typename BitVectorTy, bool isSigned>
-class PackedVectorBase;
-
-// This won't be necessary if we can specialize members without specializing
-// the parent template.
-template <typename T, unsigned BitNum, typename BitVectorTy>
-class PackedVectorBase<T, BitNum, BitVectorTy, false> {
-protected:
-  static T getValue(const BitVectorTy &Bits, unsigned Idx) {
-    T val = T();
-    for (unsigned i = 0; i != BitNum; ++i)
-      val = T(val | ((Bits[(Idx * BitNum) + i] ? 1UL : 0UL) << i));
-    return val;
-  }
-
-  static void setValue(BitVectorTy &Bits, unsigned Idx, T val) {
-    assert((val >> BitNum) == 0 && "value is too big");
-    for (unsigned i = 0; i != BitNum; ++i)
-      Bits[(Idx * BitNum) + i] = val & (T(1) << i);
-  }
-};
-
-template <typename T, unsigned BitNum, typename BitVectorTy>
-class PackedVectorBase<T, BitNum, BitVectorTy, true> {
-protected:
-  static T getValue(const BitVectorTy &Bits, unsigned Idx) {
-    T val = T();
-    for (unsigned i = 0; i != BitNum - 1; ++i)
-      val = T(val | ((Bits[(Idx * BitNum) + i] ? 1UL : 0UL) << i));
-    if (Bits[(Idx * BitNum) + BitNum - 1])
-      val = ~val;
-    return val;
-  }
-
-  static void setValue(BitVectorTy &Bits, unsigned Idx, T val) {
-    if (val < 0) {
-      val = ~val;
-      Bits.set((Idx * BitNum) + BitNum - 1);
-    } else {
-      Bits.reset((Idx * BitNum) + BitNum - 1);
-    }
-    assert((val >> (BitNum - 1)) == 0 && "value is too big");
-    for (unsigned i = 0; i != BitNum - 1; ++i)
-      Bits[(Idx * BitNum) + i] = val & (T(1) << i);
-  }
-};
-
 /// Store a vector of values using a specific number of bits for each
 /// value. Both signed and unsigned types can be used, e.g
 /// @code
@@ -75,16 +28,46 @@ class PackedVectorBase<T, BitNum, BitVectorTy, true> {
 /// will create a vector accepting values -2, -1, 0, 1. Any other value will hit
 /// an assertion.
 template <typename T, unsigned BitNum, typename BitVectorTy = BitVector>
-class PackedVector
-    : public PackedVectorBase<T, BitNum, BitVectorTy,
-                              std::numeric_limits<T>::is_signed> {
+class PackedVector {
   BitVectorTy Bits;
   // Keep track of the number of elements on our own.
   // We always maintain Bits.size() == NumElements * BitNum.
   // Used to avoid an integer division in size().
   unsigned NumElements = 0;
-  using base = PackedVectorBase<T, BitNum, BitVectorTy,
-                                std::numeric_limits<T>::is_signed>;
+
+  static T getValue(const BitVectorTy &Bits, unsigned Idx) {
+    if constexpr (std::numeric_limits<T>::is_signed) {
+      T val = T();
+      for (unsigned i = 0; i != BitNum - 1; ++i)
+        val = T(val | ((Bits[(Idx * BitNum) + i] ? 1UL : 0UL) << i));
+      if (Bits[(Idx * BitNum) + BitNum - 1])
+        val = ~val;
+      return val;
+    } else {
+      T val = T();
+      for (unsigned i = 0; i != BitNum; ++i)
+        val = T(val | ((Bits[(Idx * BitNum) + i] ? 1UL : 0UL) << i));
+      return val;
+    }
+  }
+
+  static void setValue(BitVectorTy &Bits, unsigned Idx, T val) {
+    if constexpr (std::numeric_limits<T>::is_signed) {
+      if (val < 0) {
+        val = ~val;
+        Bits.set((Idx * BitNum) + BitNum - 1);
+      } else {
+        Bits.reset((Idx * BitNum) + BitNum - 1);
+      }
+      assert((val >> (BitNum - 1)) == 0 && "value is too big");
+      for (unsigned i = 0; i != BitNum - 1; ++i)
+        Bits[(Idx * BitNum) + i] = val & (T(1) << i);
+    } else {
+      assert((val >> BitNum) == 0 && "value is too big");
+      for (unsigned i = 0; i != BitNum; ++i)
+        Bits[(Idx * BitNum) + i] = val & (T(1) << i);
+    }
+  }
 
 public:
   class reference {
@@ -135,7 +118,7 @@ class PackedVector
 
   reference operator[](unsigned Idx) { return reference(*this, Idx); }
 
-  T operator[](unsigned Idx) const { return base::getValue(Bits, Idx); }
+  T operator[](unsigned Idx) const { return getValue(Bits, Idx); }
 
   bool operator==(const PackedVector &RHS) const { return Bits == RHS.Bits; }
 

@kazutakahirata kazutakahirata merged commit 57f2a2e into llvm:main Sep 29, 2025
11 checks passed
@kazutakahirata kazutakahirata deleted the cleanup_20250928_ADT_PackedVector branch September 29, 2025 14:55
RiverDave pushed a commit that referenced this pull request Oct 1, 2025
This patch "inlines" PackedVectorBase into its sole user PackedVector.
The two variants of PackedVectorBase are dispatched with "if
constexpr".  getValue and setValue are now non-static methods of
PackedVector.

We could further simplify getValue and setValue by storing signed
integers as two's complement, but that's a change for another day.
This patch focuses on the code organization, like removing the
template trick and inheritance and making the two methods non-static.
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