From: Zachary Turner Date: Fri, 14 Dec 2018 18:21:20 +0000 (+0000) Subject: [ADT] Fix bugs in SmallBitVector. X-Git-Tag: llvmorg-8.0.0-rc1~2088 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=1eb3aae24778dbb7882508f6289b7e5703841e2f;p=platform%2Fupstream%2Fllvm.git [ADT] Fix bugs in SmallBitVector. Fixes: * find_last/find_last_unset - off-by-one error * Compound assignment ops and operator== when mixing big/small modes Patch by Brad Moody Differential Revision: https://reviews.llvm.org/D54933 llvm-svn: 349173 --- diff --git a/llvm/include/llvm/ADT/SmallBitVector.h b/llvm/include/llvm/ADT/SmallBitVector.h index f86bebd..0a73dbd 100644 --- a/llvm/include/llvm/ADT/SmallBitVector.h +++ b/llvm/include/llvm/ADT/SmallBitVector.h @@ -92,10 +92,6 @@ public: }; private: - bool isSmall() const { - return X & uintptr_t(1); - } - BitVector *getPointer() const { assert(!isSmall()); return reinterpret_cast(X); @@ -186,6 +182,8 @@ public: return make_range(set_bits_begin(), set_bits_end()); } + bool isSmall() const { return X & uintptr_t(1); } + /// Tests whether there are no bits in this bitvector. bool empty() const { return isSmall() ? getSmallSize() == 0 : getPointer()->empty(); @@ -242,7 +240,7 @@ public: uintptr_t Bits = getSmallBits(); if (Bits == 0) return -1; - return NumBaseBits - countLeadingZeros(Bits); + return NumBaseBits - countLeadingZeros(Bits) - 1; } return getPointer()->find_last(); } @@ -265,7 +263,9 @@ public: return -1; uintptr_t Bits = getSmallBits(); - return NumBaseBits - countLeadingOnes(Bits); + // Set unused bits. + Bits |= ~uintptr_t(0) << getSmallSize(); + return NumBaseBits - countLeadingOnes(Bits) - 1; } return getPointer()->find_last_unset(); } @@ -487,10 +487,17 @@ public: bool operator==(const SmallBitVector &RHS) const { if (size() != RHS.size()) return false; - if (isSmall()) + if (isSmall() && RHS.isSmall()) return getSmallBits() == RHS.getSmallBits(); - else + else if (!isSmall() && !RHS.isSmall()) return *getPointer() == *RHS.getPointer(); + else { + for (size_t i = 0, e = size(); i != e; ++i) { + if ((*this)[i] != RHS[i]) + return false; + } + return true; + } } bool operator!=(const SmallBitVector &RHS) const { @@ -498,16 +505,19 @@ public: } // Intersection, union, disjoint union. + // FIXME BitVector::operator&= does not resize the LHS but this does SmallBitVector &operator&=(const SmallBitVector &RHS) { resize(std::max(size(), RHS.size())); - if (isSmall()) + if (isSmall() && RHS.isSmall()) setSmallBits(getSmallBits() & RHS.getSmallBits()); - else if (!RHS.isSmall()) + else if (!isSmall() && !RHS.isSmall()) getPointer()->operator&=(*RHS.getPointer()); else { - SmallBitVector Copy = RHS; - Copy.resize(size()); - getPointer()->operator&=(*Copy.getPointer()); + size_t i, e; + for (i = 0, e = std::min(size(), RHS.size()); i != e; ++i) + (*this)[i] = test(i) && RHS.test(i); + for (e = size(); i != e; ++i) + reset(i); } return *this; } @@ -547,28 +557,26 @@ public: SmallBitVector &operator|=(const SmallBitVector &RHS) { resize(std::max(size(), RHS.size())); - if (isSmall()) + if (isSmall() && RHS.isSmall()) setSmallBits(getSmallBits() | RHS.getSmallBits()); - else if (!RHS.isSmall()) + else if (!isSmall() && !RHS.isSmall()) getPointer()->operator|=(*RHS.getPointer()); else { - SmallBitVector Copy = RHS; - Copy.resize(size()); - getPointer()->operator|=(*Copy.getPointer()); + for (size_t i = 0, e = RHS.size(); i != e; ++i) + (*this)[i] = test(i) || RHS.test(i); } return *this; } SmallBitVector &operator^=(const SmallBitVector &RHS) { resize(std::max(size(), RHS.size())); - if (isSmall()) + if (isSmall() && RHS.isSmall()) setSmallBits(getSmallBits() ^ RHS.getSmallBits()); - else if (!RHS.isSmall()) + else if (!isSmall() && !RHS.isSmall()) getPointer()->operator^=(*RHS.getPointer()); else { - SmallBitVector Copy = RHS; - Copy.resize(size()); - getPointer()->operator^=(*Copy.getPointer()); + for (size_t i = 0, e = RHS.size(); i != e; ++i) + (*this)[i] = test(i) != RHS.test(i); } return *this; } diff --git a/llvm/unittests/ADT/BitVectorTest.cpp b/llvm/unittests/ADT/BitVectorTest.cpp index e7ab2f3..84290b3 100644 --- a/llvm/unittests/ADT/BitVectorTest.cpp +++ b/llvm/unittests/ADT/BitVectorTest.cpp @@ -182,13 +182,8 @@ TYPED_TEST(BitVectorTest, TrivialOperation) { EXPECT_TRUE(Vec.empty()); } -TYPED_TEST(BitVectorTest, SimpleFindOps) { - // Test finding in an empty BitVector. +TYPED_TEST(BitVectorTest, SimpleFindOpsMultiWord) { TypeParam A; - EXPECT_EQ(-1, A.find_first()); - EXPECT_EQ(-1, A.find_last()); - EXPECT_EQ(-1, A.find_first_unset()); - EXPECT_EQ(-1, A.find_last_unset()); // Test finding next set and unset bits in a BitVector with multiple words A.resize(100); @@ -232,12 +227,33 @@ TYPED_TEST(BitVectorTest, SimpleFindOps) { EXPECT_EQ(0, A.find_first_unset()); EXPECT_EQ(99, A.find_last_unset()); EXPECT_EQ(99, A.find_next_unset(98)); +} + +// Check if a SmallBitVector is in small mode. This check is used in tests +// that run for both SmallBitVector and BitVector. This check doesn't apply +// to BitVector so we provide an overload that returns true to get the tests +// to compile. +static bool SmallBitVectorIsSmallMode(const SmallBitVector &bv) { + return bv.isSmall(); +} +static bool SmallBitVectorIsSmallMode(const BitVector &) { return true; } + +// These tests are intended to exercise the single-word case of BitVector +// and the small-mode case of SmallBitVector. +TYPED_TEST(BitVectorTest, SimpleFindOpsSingleWord) { + // Test finding in an empty BitVector. + TypeParam A; + ASSERT_TRUE(SmallBitVectorIsSmallMode(A)); + EXPECT_EQ(-1, A.find_first()); + EXPECT_EQ(-1, A.find_last()); + EXPECT_EQ(-1, A.find_first_unset()); + EXPECT_EQ(-1, A.find_last_unset()); - // Also test with a vector that is small enough to fit in 1 word. A.resize(20); A.set(3); A.set(4); A.set(16); + ASSERT_TRUE(SmallBitVectorIsSmallMode(A)); EXPECT_EQ(16, A.find_last()); EXPECT_EQ(3, A.find_first()); EXPECT_EQ(3, A.find_next(1)); @@ -431,6 +447,8 @@ TYPED_TEST(BitVectorTest, CompoundAssignment) { A &= B; EXPECT_FALSE(A.test(2)); EXPECT_FALSE(A.test(7)); + EXPECT_TRUE(A.test(4)); + EXPECT_TRUE(A.test(5)); EXPECT_EQ(2U, A.count()); EXPECT_EQ(50U, A.size()); @@ -444,6 +462,242 @@ TYPED_TEST(BitVectorTest, CompoundAssignment) { EXPECT_EQ(100U, A.size()); } +// Test SmallBitVector operations with mixed big/small representations +TYPED_TEST(BitVectorTest, MixedBigSmall) { + { + TypeParam Big; + TypeParam Small; + + Big.reserve(100); + Big.resize(20); + Small.resize(10); + + Small.set(0); + Small.set(1); + Big.set(0); + Big.set(2); + Big.set(16); + + Small &= Big; + EXPECT_TRUE(Small.test(0)); + EXPECT_EQ(1u, Small.count()); + // FIXME BitVector and SmallBitVector behave differently here. + // SmallBitVector resizes the LHS to max(LHS.size(), RHS.size()) + // but BitVector does not. + // EXPECT_EQ(20u, Small.size()); + } + + { + TypeParam Big; + TypeParam Small; + + Big.reserve(100); + Big.resize(20); + Small.resize(10); + + Small.set(0); + Small.set(1); + Big.set(0); + Big.set(2); + Big.set(16); + + Big &= Small; + EXPECT_TRUE(Big.test(0)); + EXPECT_EQ(1u, Big.count()); + // FIXME BitVector and SmallBitVector behave differently here. + // SmallBitVector resizes the LHS to max(LHS.size(), RHS.size()) + // but BitVector does not. + // EXPECT_EQ(20u, Big.size()); + } + + { + TypeParam Big; + TypeParam Small; + + Big.reserve(100); + Big.resize(20); + Small.resize(10); + + Small.set(0); + Small.set(1); + Big.set(0); + Big.set(2); + Big.set(16); + + Small |= Big; + EXPECT_TRUE(Small.test(0)); + EXPECT_TRUE(Small.test(1)); + EXPECT_TRUE(Small.test(2)); + EXPECT_TRUE(Small.test(16)); + EXPECT_EQ(4u, Small.count()); + EXPECT_EQ(20u, Small.size()); + } + + { + TypeParam Big; + TypeParam Small; + + Big.reserve(100); + Big.resize(20); + Small.resize(10); + + Small.set(0); + Small.set(1); + Big.set(0); + Big.set(2); + Big.set(16); + + Big |= Small; + EXPECT_TRUE(Big.test(0)); + EXPECT_TRUE(Big.test(1)); + EXPECT_TRUE(Big.test(2)); + EXPECT_TRUE(Big.test(16)); + EXPECT_EQ(4u, Big.count()); + EXPECT_EQ(20u, Big.size()); + } + + { + TypeParam Big; + TypeParam Small; + + Big.reserve(100); + Big.resize(20); + Small.resize(10); + + Small.set(0); + Small.set(1); + Big.set(0); + Big.set(2); + Big.set(16); + + Small ^= Big; + EXPECT_TRUE(Small.test(1)); + EXPECT_TRUE(Small.test(2)); + EXPECT_TRUE(Small.test(16)); + EXPECT_EQ(3u, Small.count()); + EXPECT_EQ(20u, Small.size()); + } + + { + TypeParam Big; + TypeParam Small; + + Big.reserve(100); + Big.resize(20); + Small.resize(10); + + Small.set(0); + Small.set(1); + Big.set(0); + Big.set(2); + Big.set(16); + + Big ^= Small; + EXPECT_TRUE(Big.test(1)); + EXPECT_TRUE(Big.test(2)); + EXPECT_TRUE(Big.test(16)); + EXPECT_EQ(3u, Big.count()); + EXPECT_EQ(20u, Big.size()); + } + + { + TypeParam Big; + TypeParam Small; + + Big.reserve(100); + Big.resize(20); + Small.resize(10); + + Small.set(0); + Small.set(1); + Big.set(0); + Big.set(2); + Big.set(16); + + Small.reset(Big); + EXPECT_TRUE(Small.test(1)); + EXPECT_EQ(1u, Small.count()); + EXPECT_EQ(10u, Small.size()); + } + + { + TypeParam Big; + TypeParam Small; + + Big.reserve(100); + Big.resize(20); + Small.resize(10); + + Small.set(0); + Small.set(1); + Big.set(0); + Big.set(2); + Big.set(16); + + Big.reset(Small); + EXPECT_TRUE(Big.test(2)); + EXPECT_TRUE(Big.test(16)); + EXPECT_EQ(2u, Big.count()); + EXPECT_EQ(20u, Big.size()); + } + + { + TypeParam Big; + TypeParam Small; + + Big.reserve(100); + Big.resize(10); + Small.resize(10); + + Small.set(0); + Small.set(1); + Big.set(0); + + EXPECT_FALSE(Big == Small); + EXPECT_FALSE(Small == Big); + Big.set(1); + EXPECT_TRUE(Big == Small); + EXPECT_TRUE(Small == Big); + } + + { + TypeParam Big; + TypeParam Small; + + Big.reserve(100); + Big.resize(20); + Small.resize(10); + + Small.set(0); + Big.set(1); + + EXPECT_FALSE(Small.anyCommon(Big)); + EXPECT_FALSE(Big.anyCommon(Small)); + Big.set(0); + EXPECT_TRUE(Small.anyCommon(Big)); + EXPECT_TRUE(Big.anyCommon(Small)); + } + + { + TypeParam Big; + TypeParam Small; + + Big.reserve(100); + Big.resize(10); + Small.resize(10); + + Small.set(0); + Small.set(1); + Big.set(0); + + EXPECT_TRUE(Small.test(Big)); + EXPECT_FALSE(Big.test(Small)); + Big.set(1); + EXPECT_FALSE(Small.test(Big)); + EXPECT_FALSE(Big.test(Small)); + } +} + TYPED_TEST(BitVectorTest, ProxyIndex) { TypeParam Vec(3); EXPECT_TRUE(Vec.none());