Skip to content

Commit 6f1ce2b

Browse files
[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.
1 parent 8ad5a21 commit 6f1ce2b

File tree

2 files changed

+35
-3
lines changed

2 files changed

+35
-3
lines changed

llvm/include/llvm/ADT/Bitset.h

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,15 @@ class Bitset {
4747
for (size_t I = 0; I != B.size(); ++I)
4848
Bits[I] = B[I];
4949
} else {
50-
for (size_t I = 0; I != B.size(); ++I) {
50+
unsigned BitsToAssign = NumBits;
51+
for (size_t I = 0; I != B.size() && BitsToAssign; ++I) {
5152
uint64_t Elt = B[I];
52-
Bits[2 * I] = static_cast<uint32_t>(Elt);
53-
Bits[2 * I + 1] = static_cast<uint32_t>(Elt >> 32);
53+
// On a 32-bit system the storage type will be 32-bit, so we may only
54+
// need half of a uint64_t.
55+
for (size_t offset = 0; offset != 2 && BitsToAssign; ++offset) {
56+
Bits[2 * I + offset] = static_cast<uint32_t>(Elt >> (32 * offset));
57+
BitsToAssign = BitsToAssign >= 32 ? BitsToAssign - 32 : 0;
58+
}
5459
}
5560
}
5661
}

llvm/unittests/ADT/BitsetTest.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,41 @@ class TestBitsetUInt64Array : public Bitset<NumBits> {
3131

3232
return true;
3333
}
34+
35+
void verifyStorageSize(size_t elements_64_bit, size_t elements_32_bit) {
36+
if constexpr (sizeof(uintptr_t) == sizeof(uint64_t))
37+
EXPECT_EQ(sizeof(*this), elements_64_bit * sizeof(uintptr_t));
38+
else
39+
EXPECT_EQ(sizeof(*this), elements_32_bit * sizeof(uintptr_t));
40+
}
3441
};
3542

3643
TEST(BitsetTest, Construction) {
3744
std::array<uint64_t, 2> TestVals = {0x123456789abcdef3, 0x1337d3a0b22c24};
3845
TestBitsetUInt64Array<96> Test(TestVals);
3946
EXPECT_TRUE(Test.verifyValue(TestVals));
47+
Test.verifyStorageSize(2, 3);
4048

4149
TestBitsetUInt64Array<65> Test1(TestVals);
4250
EXPECT_TRUE(Test1.verifyValue(TestVals));
51+
Test1.verifyStorageSize(2, 3);
52+
53+
std::array<uint64_t, 1> TestSingleVal = {0x12345678abcdef99};
54+
55+
TestBitsetUInt64Array<64> Test64(TestSingleVal);
56+
EXPECT_TRUE(Test64.verifyValue(TestSingleVal));
57+
Test64.verifyStorageSize(1, 2);
58+
59+
TestBitsetUInt64Array<30> Test30(TestSingleVal);
60+
EXPECT_TRUE(Test30.verifyValue(TestSingleVal));
61+
Test30.verifyStorageSize(1, 1);
62+
63+
TestBitsetUInt64Array<32> Test32(TestSingleVal);
64+
EXPECT_TRUE(Test32.verifyValue(TestSingleVal));
65+
Test32.verifyStorageSize(1, 1);
66+
67+
TestBitsetUInt64Array<33> Test33(TestSingleVal);
68+
EXPECT_TRUE(Test33.verifyValue(TestSingleVal));
69+
Test33.verifyStorageSize(1, 2);
4370
}
4471
} // namespace

0 commit comments

Comments
 (0)