Simplify BitVector code
authorserge-sans-paille <sguelton@redhat.com>
Tue, 13 Apr 2021 14:14:32 +0000 (16:14 +0200)
committerserge-sans-paille <sguelton@redhat.com>
Fri, 16 Apr 2021 20:48:33 +0000 (22:48 +0200)
Instead of managing memory by hand, delegate it to std::vector. This makes the
code much simpler, and also avoids repeatedly computing the storage size.

According to valgrind --tool=callgrind, this also slightly decreases the
instruction count, but by a small margin.

This is a recommit of 82f0e3d3ea6bf927e3397b2fb423abbc5821a30f with one usage
fixed in llvm/lib/CodeGen/RegisterScavenging.cpp.

Not the slight API change: BitVector::clear() now has the same behavior as any
other container: it does not free memory, but indeed sets the size of the
BitVector to 0. It is thus incorrect to access its content right afterwards, a
scenario which wasn't enforced in previous implementation.

Differential Revision: https://reviews.llvm.org/D100387

llvm/include/llvm/ADT/BitVector.h
llvm/lib/CodeGen/RegisterScavenging.cpp

index 080f961..21f4860 100644 (file)
@@ -79,14 +79,16 @@ class BitVector {
   static_assert(BITWORD_SIZE == 64 || BITWORD_SIZE == 32,
                 "Unsupported word size");
 
-  MutableArrayRef<BitWord> Bits; // Actual bits.
-  unsigned Size;                 // Size of bitvector in bits.
+  using Storage = std::vector<BitWord>;
+
+  Storage Bits;  // Actual bits.
+  unsigned Size; // Size of bitvector in bits.
 
 public:
   typedef unsigned size_type;
+
   // Encapsulation of a single bit.
   class reference {
-    friend class BitVector;
 
     BitWord *WordRef;
     unsigned BitPos;
@@ -136,33 +138,12 @@ public:
 
   /// BitVector ctor - Creates a bitvector of specified number of bits. All
   /// bits are initialized to the specified value.
-  explicit BitVector(unsigned s, bool t = false) : Size(s) {
-    size_t Capacity = NumBitWords(s);
-    Bits = allocate(Capacity);
-    init_words(Bits, t);
+  explicit BitVector(unsigned s, bool t = false)
+      : Bits(NumBitWords(s), 0 - (BitWord)t), Size(s) {
     if (t)
       clear_unused_bits();
   }
 
-  /// BitVector copy ctor.
-  BitVector(const BitVector &RHS) : Size(RHS.size()) {
-    if (Size == 0) {
-      Bits = MutableArrayRef<BitWord>();
-      return;
-    }
-
-    size_t Capacity = NumBitWords(RHS.size());
-    Bits = allocate(Capacity);
-    std::memcpy(Bits.data(), RHS.Bits.data(), Capacity * sizeof(BitWord));
-  }
-
-  BitVector(BitVector &&RHS) : Bits(RHS.Bits), Size(RHS.Size) {
-    RHS.Bits = MutableArrayRef<BitWord>();
-    RHS.Size = 0;
-  }
-
-  ~BitVector() { std::free(Bits.data()); }
-
   /// empty - Tests whether there are no bits in this bitvector.
   bool empty() const { return Size == 0; }
 
@@ -172,17 +153,14 @@ public:
   /// count - Returns the number of bits which are set.
   size_type count() const {
     unsigned NumBits = 0;
-    for (unsigned i = 0; i < NumBitWords(size()); ++i)
-      NumBits += countPopulation(Bits[i]);
+    for (auto Bit : Bits)
+      NumBits += countPopulation(Bit);
     return NumBits;
   }
 
   /// any - Returns true if any bit is set.
   bool any() const {
-    for (unsigned i = 0; i < NumBitWords(size()); ++i)
-      if (Bits[i] != 0)
-        return true;
-    return false;
+    return any_of(Bits, [](BitWord Bit) { return Bit != 0; });
   }
 
   /// all - Returns true if all bits are set.
@@ -345,46 +323,31 @@ public:
     return find_last_unset_in(0, PriorTo);
   }
 
-  /// clear - Removes all bits from the bitvector. Does not change capacity.
+  /// clear - Removes all bits from the bitvector.
   void clear() {
     Size = 0;
+    Bits.clear();
   }
 
   /// resize - Grow or shrink the bitvector.
   void resize(unsigned N, bool t = false) {
-    if (N > getBitCapacity()) {
-      unsigned OldCapacity = Bits.size();
-      grow(N);
-      init_words(Bits.drop_front(OldCapacity), t);
-    }
-
-    // Set any old unused bits that are now included in the BitVector. This
-    // may set bits that are not included in the new vector, but we will clear
-    // them back out below.
-    if (N > Size)
-      set_unused_bits(t);
-
-    // Update the size, and clear out any bits that are now unused
-    unsigned OldSize = Size;
+    set_unused_bits(t);
     Size = N;
-    if (t || N < OldSize)
-      clear_unused_bits();
+    Bits.resize(NumBitWords(N), 0 - BitWord(t));
+    clear_unused_bits();
   }
 
-  void reserve(unsigned N) {
-    if (N > getBitCapacity())
-      grow(N);
-  }
+  void reserve(unsigned N) { Bits.reserve(NumBitWords(N)); }
 
   // Set, reset, flip
   BitVector &set() {
-    init_words(Bits, true);
+    init_words(true);
     clear_unused_bits();
     return *this;
   }
 
   BitVector &set(unsigned Idx) {
-    assert(Bits.data() && "Bits never allocated");
+    assert(Idx < Size && "access in bound");
     Bits[Idx / BITWORD_SIZE] |= BitWord(1) << (Idx % BITWORD_SIZE);
     return *this;
   }
@@ -419,7 +382,7 @@ public:
   }
 
   BitVector &reset() {
-    init_words(Bits, false);
+    init_words(false);
     return *this;
   }
 
@@ -458,8 +421,8 @@ public:
   }
 
   BitVector &flip() {
-    for (unsigned i = 0; i < NumBitWords(size()); ++i)
-      Bits[i] = ~Bits[i];
+    for (auto &Bit : Bits)
+      Bit = ~Bit;
     clear_unused_bits();
     return *this;
   }
@@ -504,8 +467,8 @@ public:
 
   /// Test if any common bits are set.
   bool anyCommon(const BitVector &RHS) const {
-    unsigned ThisWords = NumBitWords(size());
-    unsigned RHSWords  = NumBitWords(RHS.size());
+    unsigned ThisWords = Bits.size();
+    unsigned RHSWords = RHS.Bits.size();
     for (unsigned i = 0, e = std::min(ThisWords, RHSWords); i != e; ++i)
       if (Bits[i] & RHS.Bits[i])
         return true;
@@ -516,18 +479,16 @@ public:
   bool operator==(const BitVector &RHS) const {
     if (size() != RHS.size())
       return false;
-    unsigned NumWords = NumBitWords(size());
-    return Bits.take_front(NumWords) == RHS.Bits.take_front(NumWords);
+    unsigned NumWords = Bits.size();
+    return std::equal(Bits.begin(), Bits.begin() + NumWords, RHS.Bits.begin());
   }
 
-  bool operator!=(const BitVector &RHS) const {
-    return !(*this == RHS);
-  }
+  bool operator!=(const BitVector &RHS) const { return !(*this == RHS); }
 
   /// Intersection, union, disjoint union.
   BitVector &operator&=(const BitVector &RHS) {
-    unsigned ThisWords = NumBitWords(size());
-    unsigned RHSWords  = NumBitWords(RHS.size());
+    unsigned ThisWords = Bits.size();
+    unsigned RHSWords = RHS.Bits.size();
     unsigned i;
     for (i = 0; i != std::min(ThisWords, RHSWords); ++i)
       Bits[i] &= RHS.Bits[i];
@@ -543,10 +504,9 @@ public:
 
   /// reset - Reset bits that are set in RHS. Same as *this &= ~RHS.
   BitVector &reset(const BitVector &RHS) {
-    unsigned ThisWords = NumBitWords(size());
-    unsigned RHSWords  = NumBitWords(RHS.size());
-    unsigned i;
-    for (i = 0; i != std::min(ThisWords, RHSWords); ++i)
+    unsigned ThisWords = Bits.size();
+    unsigned RHSWords = RHS.Bits.size();
+    for (unsigned i = 0; i != std::min(ThisWords, RHSWords); ++i)
       Bits[i] &= ~RHS.Bits[i];
     return *this;
   }
@@ -554,8 +514,8 @@ public:
   /// test - Check if (This - RHS) is zero.
   /// This is the same as reset(RHS) and any().
   bool test(const BitVector &RHS) const {
-    unsigned ThisWords = NumBitWords(size());
-    unsigned RHSWords  = NumBitWords(RHS.size());
+    unsigned ThisWords = Bits.size();
+    unsigned RHSWords = RHS.Bits.size();
     unsigned i;
     for (i = 0; i != std::min(ThisWords, RHSWords); ++i)
       if ((Bits[i] & ~RHS.Bits[i]) != 0)
@@ -576,7 +536,7 @@ public:
                [&Arg](auto const &BV) { return Arg.size() == BV; }) &&
            "consistent sizes");
     Out.resize(Arg.size());
-    for (size_t i = 0, e = Out.NumBitWords(Arg.size()); i != e; ++i)
+    for (size_t i = 0, e = Arg.Bits.size(); i != e; ++i)
       Out.Bits[i] = f(Arg.Bits[i], Args.Bits[i]...);
     Out.clear_unused_bits();
     return Out;
@@ -585,7 +545,7 @@ public:
   BitVector &operator|=(const BitVector &RHS) {
     if (size() < RHS.size())
       resize(RHS.size());
-    for (size_t i = 0, e = NumBitWords(RHS.size()); i != e; ++i)
+    for (size_t i = 0, e = RHS.Bits.size(); i != e; ++i)
       Bits[i] |= RHS.Bits[i];
     return *this;
   }
@@ -593,7 +553,7 @@ public:
   BitVector &operator^=(const BitVector &RHS) {
     if (size() < RHS.size())
       resize(RHS.size());
-    for (size_t i = 0, e = NumBitWords(RHS.size()); i != e; ++i)
+    for (size_t i = 0, e = RHS.Bits.size(); i != e; ++i)
       Bits[i] ^= RHS.Bits[i];
     return *this;
   }
@@ -603,7 +563,7 @@ public:
     if (LLVM_UNLIKELY(empty() || N == 0))
       return *this;
 
-    unsigned NumWords = NumBitWords(Size);
+    unsigned NumWords = Bits.size();
     assert(NumWords >= 1);
 
     wordShr(N / BITWORD_SIZE);
@@ -652,7 +612,7 @@ public:
     if (LLVM_UNLIKELY(empty() || N == 0))
       return *this;
 
-    unsigned NumWords = NumBitWords(Size);
+    unsigned NumWords = Bits.size();
     assert(NumWords >= 1);
 
     wordShl(N / BITWORD_SIZE);
@@ -697,53 +657,6 @@ public:
     return *this;
   }
 
-  // Assignment operator.
-  const BitVector &operator=(const BitVector &RHS) {
-    if (this == &RHS) return *this;
-
-    Size = RHS.size();
-
-    // Handle tombstone when the BitVector is a key of a DenseHash.
-    if (RHS.isInvalid()) {
-      std::free(Bits.data());
-      Bits = None;
-      return *this;
-    }
-
-    unsigned RHSWords = NumBitWords(Size);
-    if (Size <= getBitCapacity()) {
-      if (Size)
-        std::memcpy(Bits.data(), RHS.Bits.data(), RHSWords * sizeof(BitWord));
-      clear_unused_bits();
-      return *this;
-    }
-
-    // Grow the bitvector to have enough elements.
-    unsigned NewCapacity = RHSWords;
-    assert(NewCapacity > 0 && "negative capacity?");
-    auto NewBits = allocate(NewCapacity);
-    std::memcpy(NewBits.data(), RHS.Bits.data(), NewCapacity * sizeof(BitWord));
-
-    // Destroy the old bits.
-    std::free(Bits.data());
-    Bits = NewBits;
-
-    return *this;
-  }
-
-  const BitVector &operator=(BitVector &&RHS) {
-    if (this == &RHS) return *this;
-
-    std::free(Bits.data());
-    Bits = RHS.Bits;
-    Size = RHS.Size;
-
-    RHS.Bits = MutableArrayRef<BitWord>();
-    RHS.Size = 0;
-
-    return *this;
-  }
-
   void swap(BitVector &RHS) {
     std::swap(Bits, RHS.Bits);
     std::swap(Size, RHS.Size);
@@ -755,9 +668,7 @@ public:
   }
   bool isInvalid() const { return Size == (unsigned)-1; }
 
-  ArrayRef<BitWord> getData() const {
-    return Bits.take_front(NumBitWords(size()));
-  }
+  ArrayRef<BitWord> getData() const { return {&Bits[0], Bits.size()}; }
 
   //===--------------------------------------------------------------------===//
   // Portable bit mask operations.
@@ -807,23 +718,21 @@ private:
   /// Example:
   ///   Words = [0xBBBBAAAA, 0xDDDDFFFF, 0x00000000, 0xDDDD0000]
   /// represents a BitVector where 0xBBBBAAAA contain the least significant
-  /// bits.  So if we want to shift the BitVector left by 2 words, we need to
-  /// turn this into 0x00000000 0x00000000 0xBBBBAAAA 0xDDDDFFFF by using a
+  /// bits.  So if we want to shift the BitVector left by 2 words, we need
+  /// to turn this into 0x00000000 0x00000000 0xBBBBAAAA 0xDDDDFFFF by using a
   /// memmove which moves right, not left.
   void wordShl(uint32_t Count) {
     if (Count == 0)
       return;
 
-    uint32_t NumWords = NumBitWords(Size);
-
-    auto Src = Bits.take_front(NumWords).drop_back(Count);
-    auto Dest = Bits.take_front(NumWords).drop_front(Count);
+    uint32_t NumWords = Bits.size();
 
     // Since we always move Word-sized chunks of data with src and dest both
     // aligned to a word-boundary, we don't need to worry about endianness
     // here.
-    std::memmove(Dest.begin(), Src.begin(), Dest.size() * sizeof(BitWord));
-    std::memset(Bits.data(), 0, Count * sizeof(BitWord));
+    std::copy(Bits.begin(), Bits.begin() + NumWords - Count,
+              Bits.begin() + Count);
+    std::fill(Bits.begin(), Bits.begin() + Count, 0);
     clear_unused_bits();
   }
 
@@ -834,20 +743,10 @@ private:
     if (Count == 0)
       return;
 
-    uint32_t NumWords = NumBitWords(Size);
-
-    auto Src = Bits.take_front(NumWords).drop_front(Count);
-    auto Dest = Bits.take_front(NumWords).drop_back(Count);
-    assert(Dest.size() == Src.size());
+    uint32_t NumWords = Bits.size();
 
-    std::memmove(Dest.begin(), Src.begin(), Dest.size() * sizeof(BitWord));
-    std::memset(Dest.end(), 0, Count * sizeof(BitWord));
-  }
-
-  MutableArrayRef<BitWord> allocate(size_t NumWords) {
-    BitWord *RawBits = static_cast<BitWord *>(
-        safe_malloc(NumWords * sizeof(BitWord)));
-    return MutableArrayRef<BitWord>(RawBits, NumWords);
+    std::copy(Bits.begin() + Count, Bits.begin() + NumWords, Bits.begin());
+    std::fill(Bits.begin() + NumWords - Count, Bits.begin() + NumWords, 0);
   }
 
   int next_unset_in_word(int WordIndex, BitWord Word) const {
@@ -861,19 +760,13 @@ private:
 
   // Set the unused bits in the high words.
   void set_unused_bits(bool t = true) {
-    //  Set high words first.
-    unsigned UsedWords = NumBitWords(Size);
-    if (Bits.size() > UsedWords)
-      init_words(Bits.drop_front(UsedWords), t);
-
     //  Then set any stray high bits of the last used word.
-    unsigned ExtraBits = Size % BITWORD_SIZE;
-    if (ExtraBits) {
+    if (unsigned ExtraBits = Size % BITWORD_SIZE) {
       BitWord ExtraBitMask = ~BitWord(0) << ExtraBits;
       if (t)
-        Bits[UsedWords-1] |= ExtraBitMask;
+        Bits.back() |= ExtraBitMask;
       else
-        Bits[UsedWords-1] &= ~ExtraBitMask;
+        Bits.back() &= ~ExtraBitMask;
     }
   }
 
@@ -882,18 +775,8 @@ private:
     set_unused_bits(false);
   }
 
-  void grow(unsigned NewSize) {
-    size_t NewCapacity = std::max<size_t>(NumBitWords(NewSize), Bits.size() * 2);
-    assert(NewCapacity > 0 && "realloc-ing zero space");
-    BitWord *NewBits = static_cast<BitWord *>(
-        safe_realloc(Bits.data(), NewCapacity * sizeof(BitWord)));
-    Bits = MutableArrayRef<BitWord>(NewBits, NewCapacity);
-    clear_unused_bits();
-  }
-
-  void init_words(MutableArrayRef<BitWord> B, bool t) {
-    if (B.size() > 0)
-      memset(B.data(), 0 - (int)t, B.size() * sizeof(BitWord));
+  void init_words(bool t) {
+    std::fill(Bits.begin(), Bits.end(), 0 - (BitWord)t);
   }
 
   template<bool AddBits, bool InvertMask>
@@ -934,7 +817,7 @@ inline size_t capacity_in_bytes(const BitVector &X) {
 }
 
 template <> struct DenseMapInfo<BitVector> {
-  static inline BitVector getEmptyKey() { return BitVector(); }
+  static inline BitVector getEmptyKey() { return {}; }
   static inline BitVector getTombstoneKey() {
     BitVector V;
     V.invalid();
@@ -954,10 +837,7 @@ template <> struct DenseMapInfo<BitVector> {
 
 namespace std {
   /// Implement std::swap in terms of BitVector swap.
-  inline void
-  swap(llvm::BitVector &LHS, llvm::BitVector &RHS) {
-    LHS.swap(RHS);
-  }
+inline void swap(llvm::BitVector &LHS, llvm::BitVector &RHS) { LHS.swap(RHS); }
 } // end namespace std
 
 #endif // LLVM_ADT_BITVECTOR_H
index 1f23073..2ec4915 100644 (file)
@@ -119,7 +119,7 @@ void RegScavenger::determineKillsAndDefs() {
   DefRegUnits.reset();
   for (const MachineOperand &MO : MI.operands()) {
     if (MO.isRegMask()) {
-      TmpRegUnits.clear();
+      TmpRegUnits.reset();
       for (unsigned RU = 0, RUEnd = TRI->getNumRegUnits(); RU != RUEnd; ++RU) {
         for (MCRegUnitRootIterator RURI(RU, TRI); RURI.isValid(); ++RURI) {
           if (MO.clobbersPhysReg(*RURI)) {