Make BitCodeAbbrev ownership explicit using shared_ptr rather than IntrusiveRefCntPtr

If this is a problem for anyone (shared_ptr is two pointers in size,
whereas IntrusiveRefCntPtr is 1 - and the ref count control block that
make_shared adds is probably larger than the one int in RefCountedBase)
I'd prefer to address this by adding a lower-overhead version of
shared_ptr (possibly refactoring IntrusiveRefCntPtr into such a thing)
to avoid the intrusiveness - this allows memory ownership to remain
orthogonal to types and at least to me, seems to make code easier to
understand (since no implicit ownership acquisition can happen).

llvm-svn: 291006

Author: dwblaikie

llvm/include/llvm/Bitcode/BitCodes.h

@@ -166,11 +166,8 @@ template <> struct isPodLike<BitCodeAbbrevOp> { static const bool value=true; };
 /// BitCodeAbbrev - This class represents an abbreviation record.  An
 /// abbreviation allows a complex record that has redundancy to be stored in a
 /// specialized format instead of the fully-general, fully-vbr, format.
-class BitCodeAbbrev : public RefCountedBase<BitCodeAbbrev> {
+class BitCodeAbbrev {
   SmallVector<BitCodeAbbrevOp, 32> OperandList;
-  // Only RefCountedBase is allowed to delete.
-  ~BitCodeAbbrev() = default;
-  friend class RefCountedBase<BitCodeAbbrev>;
 
 public:
   unsigned getNumOperandInfos() const {

llvm/include/llvm/Bitcode/BitstreamReader.h

@@ -42,7 +42,7 @@ class BitstreamBlockInfo {
   /// describe abbreviations that all blocks of the specified ID inherit.
   struct BlockInfo {
     unsigned BlockID;
-    std::vector<IntrusiveRefCntPtr<BitCodeAbbrev>> Abbrevs;
+    std::vector<std::shared_ptr<BitCodeAbbrev>> Abbrevs;
     std::string Name;
     std::vector<std::pair<unsigned, std::string> > RecordNames;
   };
@@ -316,11 +316,11 @@ class BitstreamCursor : SimpleBitstreamCursor {
   unsigned CurCodeSize = 2;
 
   /// Abbrevs installed at in this block.
-  std::vector<IntrusiveRefCntPtr<BitCodeAbbrev>> CurAbbrevs;
+  std::vector<std::shared_ptr<BitCodeAbbrev>> CurAbbrevs;
 
   struct Block {
     unsigned PrevCodeSize;
-    std::vector<IntrusiveRefCntPtr<BitCodeAbbrev>> PrevAbbrevs;
+    std::vector<std::shared_ptr<BitCodeAbbrev>> PrevAbbrevs;
 
     explicit Block(unsigned PCS) : PrevCodeSize(PCS) {}
   };

llvm/include/llvm/Bitcode/BitstreamWriter.h

@@ -43,12 +43,12 @@ class BitstreamWriter {
   unsigned BlockInfoCurBID;
 
   /// CurAbbrevs - Abbrevs installed at in this block.
-  std::vector<IntrusiveRefCntPtr<BitCodeAbbrev>> CurAbbrevs;
+  std::vector<std::shared_ptr<BitCodeAbbrev>> CurAbbrevs;
 
   struct Block {
     unsigned PrevCodeSize;
     size_t StartSizeWord;
-    std::vector<IntrusiveRefCntPtr<BitCodeAbbrev>> PrevAbbrevs;
+    std::vector<std::shared_ptr<BitCodeAbbrev>> PrevAbbrevs;
     Block(unsigned PCS, size_t SSW) : PrevCodeSize(PCS), StartSizeWord(SSW) {}
   };
 
@@ -59,7 +59,7 @@ class BitstreamWriter {
   /// These describe abbreviations that all blocks of the specified ID inherit.
   struct BlockInfo {
     unsigned BlockID;
-    std::vector<IntrusiveRefCntPtr<BitCodeAbbrev>> Abbrevs;
+    std::vector<std::shared_ptr<BitCodeAbbrev>> Abbrevs;
   };
   std::vector<BlockInfo> BlockInfoRecords;
 
@@ -469,12 +469,12 @@ class BitstreamWriter {
 
 private:
   // Emit the abbreviation as a DEFINE_ABBREV record.
-  void EncodeAbbrev(BitCodeAbbrev *Abbv) {
+  void EncodeAbbrev(const BitCodeAbbrev &Abbv) {
     EmitCode(bitc::DEFINE_ABBREV);
-    EmitVBR(Abbv->getNumOperandInfos(), 5);
-    for (unsigned i = 0, e = static_cast<unsigned>(Abbv->getNumOperandInfos());
+    EmitVBR(Abbv.getNumOperandInfos(), 5);
+    for (unsigned i = 0, e = static_cast<unsigned>(Abbv.getNumOperandInfos());
          i != e; ++i) {
-      const BitCodeAbbrevOp &Op = Abbv->getOperandInfo(i);
+      const BitCodeAbbrevOp &Op = Abbv.getOperandInfo(i);
       Emit(Op.isLiteral(), 1);
       if (Op.isLiteral()) {
         EmitVBR64(Op.getLiteralValue(), 8);
@@ -489,10 +489,10 @@ class BitstreamWriter {
 
   /// EmitAbbrev - This emits an abbreviation to the stream.  Note that this
   /// method takes ownership of the specified abbrev.
-  unsigned EmitAbbrev(BitCodeAbbrev *Abbv) {
+  unsigned EmitAbbrev(std::shared_ptr<BitCodeAbbrev> Abbv) {
     // Emit the abbreviation as a record.
-    EncodeAbbrev(Abbv);
-    CurAbbrevs.push_back(Abbv);
+    EncodeAbbrev(*Abbv);
+    CurAbbrevs.push_back(std::move(Abbv));
     return static_cast<unsigned>(CurAbbrevs.size())-1 +
       bitc::FIRST_APPLICATION_ABBREV;
   }
@@ -532,13 +532,13 @@ class BitstreamWriter {
 
   /// EmitBlockInfoAbbrev - Emit a DEFINE_ABBREV record for the specified
   /// BlockID.
-  unsigned EmitBlockInfoAbbrev(unsigned BlockID, BitCodeAbbrev *Abbv) {
+  unsigned EmitBlockInfoAbbrev(unsigned BlockID, std::shared_ptr<BitCodeAbbrev> Abbv) {
     SwitchToBlockID(BlockID);
-    EncodeAbbrev(Abbv);
+    EncodeAbbrev(*Abbv);
 
     // Add the abbrev to the specified block record.
     BlockInfo &Info = getOrCreateBlockInfo(BlockID);
-    Info.Abbrevs.push_back(Abbv);
+    Info.Abbrevs.push_back(std::move(Abbv));
 
     return Info.Abbrevs.size()-1+bitc::FIRST_APPLICATION_ABBREV;
   }

llvm/include/llvm/Support/FileSystem.h

@@ -829,28 +829,23 @@ class directory_iterator {
 };
 
 namespace detail {
-  /// RecDirIterState - Keeps state for the recursive_directory_iterator. It is
-  /// reference counted in order to preserve InputIterator semantics on copy.
-  struct RecDirIterState : public RefCountedBase<RecDirIterState> {
-    RecDirIterState()
-      : Level(0)
-      , HasNoPushRequest(false) {}
-
+  /// Keeps state for the recursive_directory_iterator.
+  struct RecDirIterState {
     std::stack<directory_iterator, std::vector<directory_iterator>> Stack;
-    uint16_t Level;
-    bool HasNoPushRequest;
+    uint16_t Level = 0;
+    bool HasNoPushRequest = false;
   };
 } // end namespace detail
 
 /// recursive_directory_iterator - Same as directory_iterator except for it
 /// recurses down into child directories.
 class recursive_directory_iterator {
-  IntrusiveRefCntPtr<detail::RecDirIterState> State;
+  std::shared_ptr<detail::RecDirIterState> State;
 
 public:
   recursive_directory_iterator() = default;
   explicit recursive_directory_iterator(const Twine &path, std::error_code &ec)
-      : State(new detail::RecDirIterState) {
+      : State(std::make_shared<detail::RecDirIterState>()) {
     State->Stack.push(directory_iterator(path, ec));
     if (State->Stack.top() == directory_iterator())
       State.reset();

llvm/lib/Bitcode/Reader/BitstreamReader.cpp

@@ -273,7 +273,7 @@ unsigned BitstreamCursor::readRecord(unsigned AbbrevID,
 }
 
 void BitstreamCursor::ReadAbbrevRecord() {
-  BitCodeAbbrev *Abbv = new BitCodeAbbrev();
+  auto Abbv = std::make_shared<BitCodeAbbrev>();
   unsigned NumOpInfo = ReadVBR(5);
   for (unsigned i = 0; i != NumOpInfo; ++i) {
     bool IsLiteral = Read(1);
@@ -307,7 +307,7 @@ void BitstreamCursor::ReadAbbrevRecord() {
 
   if (Abbv->getNumOperandInfos() == 0)
     report_fatal_error("Abbrev record with no operands");
-  CurAbbrevs.push_back(Abbv);
+  CurAbbrevs.push_back(std::move(Abbv));
 }
 
 Optional<BitstreamBlockInfo>