[llvm] 4716ebb - [ADT] CoalescingBitVector: Avoid initial heap allocation, NFC

Vedant Kumar via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 20 12:18:34 PDT 2020


Author: Vedant Kumar
Date: 2020-03-20T12:18:25-07:00
New Revision: 4716ebb823e4a3953d7ea803db1949ff699b96c8

URL: https://github.com/llvm/llvm-project/commit/4716ebb823e4a3953d7ea803db1949ff699b96c8
DIFF: https://github.com/llvm/llvm-project/commit/4716ebb823e4a3953d7ea803db1949ff699b96c8.diff

LOG: [ADT] CoalescingBitVector: Avoid initial heap allocation, NFC

Avoid making a heap allocation when constructing a CoalescingBitVector.

This reduces time spent in LiveDebugValues when compiling sqlite3 by
700ms (0.5% of the total User Time).

rdar://60046261

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

Added: 
    

Modified: 
    llvm/include/llvm/ADT/CoalescingBitVector.h
    llvm/lib/CodeGen/LiveDebugValues.cpp
    llvm/unittests/ADT/CoalescingBitVectorTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/ADT/CoalescingBitVector.h b/llvm/include/llvm/ADT/CoalescingBitVector.h
index 6fc81b312645..34352daeea15 100644
--- a/llvm/include/llvm/ADT/CoalescingBitVector.h
+++ b/llvm/include/llvm/ADT/CoalescingBitVector.h
@@ -21,7 +21,6 @@
 
 #include <algorithm>
 #include <initializer_list>
-#include <memory>
 
 namespace llvm {
 
@@ -34,8 +33,7 @@ namespace llvm {
 /// performance for non-sequential find() operations.
 ///
 /// \tparam IndexT - The type of the index into the bitvector.
-/// \tparam N - The first N coalesced intervals of set bits are stored in-place
-/// (in the initial heap allocation).
+/// \tparam N - The first N coalesced intervals of set bits are stored in-place.
 template <typename IndexT, unsigned N = 16> class CoalescingBitVector {
   static_assert(std::is_unsigned<IndexT>::value,
                 "Index must be an unsigned integer.");
@@ -55,13 +53,13 @@ template <typename IndexT, unsigned N = 16> class CoalescingBitVector {
   /// Construct by passing in a CoalescingBitVector<IndexT>::Allocator
   /// reference.
   CoalescingBitVector(Allocator &Alloc)
-      : Alloc(&Alloc), Intervals(std::make_unique<MapT>(Alloc)) {}
+      : Alloc(&Alloc), Intervals(Alloc) {}
 
   /// \name Copy/move constructors and assignment operators.
   /// @{
 
   CoalescingBitVector(const ThisT &Other)
-      : Alloc(Other.Alloc), Intervals(std::make_unique<MapT>(*Other.Alloc)) {
+      : Alloc(Other.Alloc), Intervals(*Other.Alloc) {
     set(Other);
   }
 
@@ -71,27 +69,21 @@ template <typename IndexT, unsigned N = 16> class CoalescingBitVector {
     return *this;
   }
 
-  CoalescingBitVector(ThisT &&Other)
-      : Alloc(Other.Alloc), Intervals(std::move(Other.Intervals)) {}
-
-  ThisT &operator=(ThisT &&Other) {
-    Alloc = Other.Alloc;
-    Intervals = std::move(Other.Intervals);
-    return *this;
-  }
+  CoalescingBitVector(ThisT &&Other) = delete;
+  ThisT &operator=(ThisT &&Other) = delete;
 
   /// @}
 
   /// Clear all the bits.
-  void clear() { Intervals->clear(); }
+  void clear() { Intervals.clear(); }
 
   /// Check whether no bits are set.
-  bool empty() const { return Intervals->empty(); }
+  bool empty() const { return Intervals.empty(); }
 
   /// Count the number of set bits.
   unsigned count() const {
     unsigned Bits = 0;
-    for (auto It = Intervals->begin(), End = Intervals->end(); It != End; ++It)
+    for (auto It = Intervals.begin(), End = Intervals.end(); It != End; ++It)
       Bits += 1 + It.stop() - It.start();
     return Bits;
   }
@@ -112,7 +104,7 @@ template <typename IndexT, unsigned N = 16> class CoalescingBitVector {
   /// This method does /not/ support setting already-set bits, see \ref set
   /// for the rationale. For a safe set union operation, use \ref operator|=.
   void set(const ThisT &Other) {
-    for (auto It = Other.Intervals->begin(), End = Other.Intervals->end();
+    for (auto It = Other.Intervals.begin(), End = Other.Intervals.end();
          It != End; ++It)
       insert(It.start(), It.stop());
   }
@@ -125,8 +117,8 @@ template <typename IndexT, unsigned N = 16> class CoalescingBitVector {
 
   /// Check whether the bit at \p Index is set.
   bool test(IndexT Index) const {
-    const auto It = Intervals->find(Index);
-    if (It == Intervals->end())
+    const auto It = Intervals.find(Index);
+    if (It == Intervals.end())
       return false;
     assert(It.stop() >= Index && "Interval must end after Index");
     return It.start() <= Index;
@@ -140,8 +132,8 @@ template <typename IndexT, unsigned N = 16> class CoalescingBitVector {
 
   /// Reset the bit at \p Index. Supports resetting an already-unset bit.
   void reset(IndexT Index) {
-    auto It = Intervals->find(Index);
-    if (It == Intervals->end())
+    auto It = Intervals.find(Index);
+    if (It == Intervals.end())
       return;
 
     // Split the interval containing Index into up to two parts: one from
@@ -169,7 +161,7 @@ template <typename IndexT, unsigned N = 16> class CoalescingBitVector {
     getOverlaps(RHS, Overlaps);
 
     // Insert the non-overlapping parts of all the intervals from RHS.
-    for (auto It = RHS.Intervals->begin(), End = RHS.Intervals->end();
+    for (auto It = RHS.Intervals.begin(), End = RHS.Intervals.end();
          It != End; ++It) {
       IndexT Start = It.start();
       IndexT Stop = It.stop();
@@ -205,7 +197,7 @@ template <typename IndexT, unsigned N = 16> class CoalescingBitVector {
       IndexT OlapStart, OlapStop;
       std::tie(OlapStart, OlapStop) = Overlap;
 
-      auto It = Intervals->find(OlapStart);
+      auto It = Intervals.find(OlapStart);
       IndexT CurrStart = It.start();
       IndexT CurrStop = It.stop();
       assert(CurrStart <= OlapStart && OlapStop <= CurrStop &&
@@ -227,14 +219,14 @@ template <typename IndexT, unsigned N = 16> class CoalescingBitVector {
     // We cannot just use std::equal because it checks the dereferenced values
     // of an iterator pair for equality, not the iterators themselves. In our
     // case that results in comparison of the (unused) IntervalMap values.
-    auto ItL = Intervals->begin();
-    auto ItR = RHS.Intervals->begin();
-    while (ItL != Intervals->end() && ItR != RHS.Intervals->end() &&
+    auto ItL = Intervals.begin();
+    auto ItR = RHS.Intervals.begin();
+    while (ItL != Intervals.end() && ItR != RHS.Intervals.end() &&
            ItL.start() == ItR.start() && ItL.stop() == ItR.stop()) {
       ++ItL;
       ++ItR;
     }
-    return ItL == Intervals->end() && ItR == RHS.Intervals->end();
+    return ItL == Intervals.end() && ItR == RHS.Intervals.end();
   }
 
   bool operator!=(const ThisT &RHS) const { return !operator==(RHS); }
@@ -324,15 +316,15 @@ template <typename IndexT, unsigned N = 16> class CoalescingBitVector {
     }
   };
 
-  const_iterator begin() const { return const_iterator(Intervals->begin()); }
+  const_iterator begin() const { return const_iterator(Intervals.begin()); }
 
   const_iterator end() const { return const_iterator(); }
 
   /// Return an iterator pointing to the first set bit AT, OR AFTER, \p Index.
   /// If no such set bit exists, return end(). This is like std::lower_bound.
   const_iterator find(IndexT Index) const {
-    auto UnderlyingIt = Intervals->find(Index);
-    if (UnderlyingIt == Intervals->end())
+    auto UnderlyingIt = Intervals.find(Index);
+    if (UnderlyingIt == Intervals.end())
       return end();
     auto It = const_iterator(UnderlyingIt);
     It.advanceTo(Index);
@@ -341,7 +333,7 @@ template <typename IndexT, unsigned N = 16> class CoalescingBitVector {
 
   void print(raw_ostream &OS) const {
     OS << "{";
-    for (auto It = Intervals->begin(), End = Intervals->end(); It != End;
+    for (auto It = Intervals.begin(), End = Intervals.end(); It != End;
          ++It) {
       OS << "[" << It.start();
       if (It.start() != It.stop())
@@ -362,13 +354,13 @@ template <typename IndexT, unsigned N = 16> class CoalescingBitVector {
 #endif
 
 private:
-  void insert(IndexT Start, IndexT End) { Intervals->insert(Start, End, 0); }
+  void insert(IndexT Start, IndexT End) { Intervals.insert(Start, End, 0); }
 
   /// Record the overlaps between \p this and \p Other in \p Overlaps. Return
   /// true if there is any overlap.
   bool getOverlaps(const ThisT &Other,
                    SmallVectorImpl<IntervalT> &Overlaps) const {
-    for (IntervalMapOverlaps<MapT, MapT> I(*Intervals, *Other.Intervals);
+    for (IntervalMapOverlaps<MapT, MapT> I(Intervals, Other.Intervals);
          I.valid(); ++I)
       Overlaps.emplace_back(I.start(), I.stop());
     assert(std::is_sorted(Overlaps.begin(), Overlaps.end(),
@@ -409,7 +401,7 @@ template <typename IndexT, unsigned N = 16> class CoalescingBitVector {
   }
 
   Allocator *Alloc;
-  std::unique_ptr<MapT> Intervals;
+  MapT Intervals;
 };
 
 } // namespace llvm

diff  --git a/llvm/lib/CodeGen/LiveDebugValues.cpp b/llvm/lib/CodeGen/LiveDebugValues.cpp
index a013c419b7c7..89c3bcf52cdd 100644
--- a/llvm/lib/CodeGen/LiveDebugValues.cpp
+++ b/llvm/lib/CodeGen/LiveDebugValues.cpp
@@ -482,7 +482,8 @@ class LiveDebugValues : public MachineFunctionPass {
     }
   };
 
-  using VarLocInMBB = SmallDenseMap<const MachineBasicBlock *, VarLocSet>;
+  using VarLocInMBB =
+      SmallDenseMap<const MachineBasicBlock *, std::unique_ptr<VarLocSet>>;
   struct TransferDebugPair {
     MachineInstr *TransferInst; ///< Instruction where this transfer occurs.
     LocIndex LocationID;        ///< Location number for the transfer dest.
@@ -573,15 +574,17 @@ class LiveDebugValues : public MachineFunctionPass {
                    SmallVectorImpl<uint32_t> &UsedRegs) const;
 
   VarLocSet &getVarLocsInMBB(const MachineBasicBlock *MBB, VarLocInMBB &Locs) {
-    auto Result = Locs.try_emplace(MBB, Alloc);
-    return Result.first->second;
+    std::unique_ptr<VarLocSet> &VLS = Locs[MBB];
+    if (!VLS)
+      VLS = std::make_unique<VarLocSet>(Alloc);
+    return *VLS.get();
   }
 
   const VarLocSet &getVarLocsInMBB(const MachineBasicBlock *MBB,
                                    const VarLocInMBB &Locs) const {
     auto It = Locs.find(MBB);
     assert(It != Locs.end() && "MBB not in map");
-    return It->second;
+    return *It->second.get();
   }
 
   /// Tests whether this instruction is a spill to a stack location.
@@ -1479,10 +1482,11 @@ bool LiveDebugValues::join(
 
     // Just copy over the Out locs to incoming locs for the first visited
     // predecessor, and for all other predecessors join the Out locs.
+    VarLocSet &OutLocVLS = *OL->second.get();
     if (!NumVisited)
-      InLocsT = OL->second;
+      InLocsT = OutLocVLS;
     else
-      InLocsT &= OL->second;
+      InLocsT &= OutLocVLS;
 
     LLVM_DEBUG({
       if (!InLocsT.empty()) {
@@ -1554,7 +1558,7 @@ void LiveDebugValues::flushPendingLocs(VarLocInMBB &PendingInLocs,
   for (auto &Iter : PendingInLocs) {
     // Map is keyed on a constant pointer, unwrap it so we can insert insts.
     auto &MBB = const_cast<MachineBasicBlock &>(*Iter.first);
-    VarLocSet &Pending = Iter.second;
+    VarLocSet &Pending = *Iter.second.get();
 
     for (uint64_t ID : Pending) {
       // The ID location is live-in to MBB -- work out what kind of machine
@@ -1703,7 +1707,7 @@ bool LiveDebugValues::ExtendRanges(MachineFunction &MF) {
 
   // Initialize per-block structures and scan for fragment overlaps.
   for (auto &MBB : MF) {
-    PendingInLocs.try_emplace(&MBB, Alloc);
+    PendingInLocs[&MBB] = std::make_unique<VarLocSet>(Alloc);
 
     for (auto &MI : MBB) {
       if (MI.isDebugValue())

diff  --git a/llvm/unittests/ADT/CoalescingBitVectorTest.cpp b/llvm/unittests/ADT/CoalescingBitVectorTest.cpp
index 02a5bde2bfac..9536efe742a2 100644
--- a/llvm/unittests/ADT/CoalescingBitVectorTest.cpp
+++ b/llvm/unittests/ADT/CoalescingBitVectorTest.cpp
@@ -77,17 +77,6 @@ TEST(CoalescingBitVector, Copy) {
   EXPECT_TRUE(elementsMatch(BV2, {0}));
 }
 
-TEST(CoalescingBitVector, Move) {
-  UBitVec::Allocator Alloc;
-  UBitVec BV1(Alloc);
-  BV1.set(0);
-  UBitVec BV2 = std::move(BV1);
-  EXPECT_TRUE(elementsMatch(BV2, {0}));
-  BV2.set(5);
-  BV1 = std::move(BV2);
-  EXPECT_TRUE(elementsMatch(BV1, {0, 5}));
-}
-
 TEST(CoalescingBitVectorTest, Iterators) {
   UBitVec::Allocator Alloc;
   UBitVec BV(Alloc);


        


More information about the llvm-commits mailing list