[llvm] 6f1ce2b - [llvm][ADT] Fix uint64_t array BitSet construction on 32-bit systems (#162814)

[via llvm-commits]

Author: David Spickett
Date: 2025-10-10T10:08:50Z
New Revision: 6f1ce2b25b56d0fac317d93e7b59a22afbb54931

URL: https://github.com/llvm/llvm-project/commit/6f1ce2b25b56d0fac317d93e7b59a22afbb54931
DIFF: https://github.com/llvm/llvm-project/commit/6f1ce2b25b56d0fac317d93e7b59a22afbb54931.diff

LOG: [llvm][ADT] Fix uint64_t array BitSet construction on 32-bit systems (#162814)

When the underlying storage element is 32-bit, we may only need half of
the last value in the uint64_t array. I've adjusted the constructor to
account for that.

For example, if you construct a 65 bit bitset you will need both 32-bit
halves of the first array value but only the bottom half of the second
value. The storage will only have 3 elements, so attempting to assign
the top 32-bits into the storage will fail.

This happened on our buildbot:
https://lab.llvm.org/buildbot/#/builders/154/builds/22555
(though the traceback is not useful)

Then added tests for < 32 bit sizes, and assertions for the number of
elements we decide to use. Given that the only member of BitSet is a
std::array, I think the size will be consistent across systems.

Tested on 32 and 64-bit Arm machines.

Follow up to #162703.

Added: 
    

Modified: 
    llvm/include/llvm/ADT/Bitset.h
    llvm/unittests/ADT/BitsetTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/ADT/Bitset.h b/llvm/include/llvm/ADT/Bitset.h
index 0dfeb2088dfa0..1d4cbf8306230 100644
--- a/llvm/include/llvm/ADT/Bitset.h
+++ b/llvm/include/llvm/ADT/Bitset.h
@@ -47,10 +47,15 @@ class Bitset {
       for (size_t I = 0; I != B.size(); ++I)
         Bits[I] = B[I];
     } else {
-      for (size_t I = 0; I != B.size(); ++I) {
+      unsigned BitsToAssign = NumBits;
+      for (size_t I = 0; I != B.size() && BitsToAssign; ++I) {
         uint64_t Elt = B[I];
-        Bits[2 * I] = static_cast<uint32_t>(Elt);
-        Bits[2 * I + 1] = static_cast<uint32_t>(Elt >> 32);
+        // On a 32-bit system the storage type will be 32-bit, so we may only
+        // need half of a uint64_t.
+        for (size_t offset = 0; offset != 2 && BitsToAssign; ++offset) {
+          Bits[2 * I + offset] = static_cast<uint32_t>(Elt >> (32 * offset));
+          BitsToAssign = BitsToAssign >= 32 ? BitsToAssign - 32 : 0;
+        }
       }
     }
   }

diff  --git a/llvm/unittests/ADT/BitsetTest.cpp b/llvm/unittests/ADT/BitsetTest.cpp
index 8877397b30c53..0ecd213d6a781 100644
--- a/llvm/unittests/ADT/BitsetTest.cpp
+++ b/llvm/unittests/ADT/BitsetTest.cpp
@@ -31,14 +31,41 @@ class TestBitsetUInt64Array : public Bitset<NumBits> {
 
     return true;
   }
+
+  void verifyStorageSize(size_t elements_64_bit, size_t elements_32_bit) {
+    if constexpr (sizeof(uintptr_t) == sizeof(uint64_t))
+      EXPECT_EQ(sizeof(*this), elements_64_bit * sizeof(uintptr_t));
+    else
+      EXPECT_EQ(sizeof(*this), elements_32_bit * sizeof(uintptr_t));
+  }
 };
 
 TEST(BitsetTest, Construction) {
   std::array<uint64_t, 2> TestVals = {0x123456789abcdef3, 0x1337d3a0b22c24};
   TestBitsetUInt64Array<96> Test(TestVals);
   EXPECT_TRUE(Test.verifyValue(TestVals));
+  Test.verifyStorageSize(2, 3);
 
   TestBitsetUInt64Array<65> Test1(TestVals);
   EXPECT_TRUE(Test1.verifyValue(TestVals));
+  Test1.verifyStorageSize(2, 3);
+
+  std::array<uint64_t, 1> TestSingleVal = {0x12345678abcdef99};
+
+  TestBitsetUInt64Array<64> Test64(TestSingleVal);
+  EXPECT_TRUE(Test64.verifyValue(TestSingleVal));
+  Test64.verifyStorageSize(1, 2);
+
+  TestBitsetUInt64Array<30> Test30(TestSingleVal);
+  EXPECT_TRUE(Test30.verifyValue(TestSingleVal));
+  Test30.verifyStorageSize(1, 1);
+
+  TestBitsetUInt64Array<32> Test32(TestSingleVal);
+  EXPECT_TRUE(Test32.verifyValue(TestSingleVal));
+  Test32.verifyStorageSize(1, 1);
+
+  TestBitsetUInt64Array<33> Test33(TestSingleVal);
+  EXPECT_TRUE(Test33.verifyValue(TestSingleVal));
+  Test33.verifyStorageSize(1, 2);
 }
 } // namespace