From c1c993b3e8912e6fe97a716a00c281b8ae7aac90 Mon Sep 17 00:00:00 2001 From: Vedant Kumar Date: Thu, 19 Mar 2020 16:21:52 -0700 Subject: [PATCH] [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 --- include/llvm/ADT/CoalescingBitVector.h | 60 ++++++++++------------- lib/CodeGen/LiveDebugValues.cpp | 20 +++++--- unittests/ADT/CoalescingBitVectorTest.cpp | 11 ----- 3 files changed, 38 insertions(+), 53 deletions(-) diff --git a/include/llvm/ADT/CoalescingBitVector.h b/include/llvm/ADT/CoalescingBitVector.h index 6fc81b31264..34352daeea1 100644 --- a/include/llvm/ADT/CoalescingBitVector.h +++ b/include/llvm/ADT/CoalescingBitVector.h @@ -21,7 +21,6 @@ #include #include -#include 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 class CoalescingBitVector { static_assert(std::is_unsigned::value, "Index must be an unsigned integer."); @@ -55,13 +53,13 @@ public: /// Construct by passing in a CoalescingBitVector::Allocator /// reference. CoalescingBitVector(Allocator &Alloc) - : Alloc(&Alloc), Intervals(std::make_unique(Alloc)) {} + : Alloc(&Alloc), Intervals(Alloc) {} /// \name Copy/move constructors and assignment operators. /// @{ CoalescingBitVector(const ThisT &Other) - : Alloc(Other.Alloc), Intervals(std::make_unique(*Other.Alloc)) { + : Alloc(Other.Alloc), Intervals(*Other.Alloc) { set(Other); } @@ -71,27 +69,21 @@ public: 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 @@ public: /// 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 @@ public: /// 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 @@ public: /// 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 @@ public: 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 @@ public: 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 @@ public: // 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 @@ public: } }; - 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 @@ public: 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 @@ public: #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 &Overlaps) const { - for (IntervalMapOverlaps I(*Intervals, *Other.Intervals); + for (IntervalMapOverlaps 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 @@ private: } Allocator *Alloc; - std::unique_ptr Intervals; + MapT Intervals; }; } // namespace llvm diff --git a/lib/CodeGen/LiveDebugValues.cpp b/lib/CodeGen/LiveDebugValues.cpp index a013c419b7c..89c3bcf52cd 100644 --- a/lib/CodeGen/LiveDebugValues.cpp +++ b/lib/CodeGen/LiveDebugValues.cpp @@ -482,7 +482,8 @@ private: } }; - using VarLocInMBB = SmallDenseMap; + using VarLocInMBB = + SmallDenseMap>; struct TransferDebugPair { MachineInstr *TransferInst; ///< Instruction where this transfer occurs. LocIndex LocationID; ///< Location number for the transfer dest. @@ -573,15 +574,17 @@ private: SmallVectorImpl &UsedRegs) const; VarLocSet &getVarLocsInMBB(const MachineBasicBlock *MBB, VarLocInMBB &Locs) { - auto Result = Locs.try_emplace(MBB, Alloc); - return Result.first->second; + std::unique_ptr &VLS = Locs[MBB]; + if (!VLS) + VLS = std::make_unique(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(*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(Alloc); for (auto &MI : MBB) { if (MI.isDebugValue()) diff --git a/unittests/ADT/CoalescingBitVectorTest.cpp b/unittests/ADT/CoalescingBitVectorTest.cpp index 02a5bde2bfa..9536efe742a 100644 --- a/unittests/ADT/CoalescingBitVectorTest.cpp +++ b/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);