Bitcode: Maintain block info block abbreviations in the BitstreamCursor.

[pcc]

Summary

This will be necessary in order to correctly interpret bitcode inputs
containing multiple block info blocks.

At this point BitstreamReader is not much more than a container for a
MemoryObject; once MemoryObject is removed [0] I would also like to remove
BitstreamReader and change BitstreamCursor to hold a memory buffer directly.

[0] http://lists.llvm.org/pipermail/llvm-dev/2016-October/106595.html

[mehdi_amini]

I think this is basically like "merging" responsibility from the bitstream reader and the cursor into the cursor. The cursor isn't just a "position" in the stream with the "bitstreamreader" in charge of keeping the "context".
If this is correct, I'm not against it but at least you should reflect it in the class documentation.

Also added Duncan for an extra opinion here.

[ABataev]

llvm/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp

86

%s/GetBlockName/getBlockName/g

[pcc]

Obsoleted by D26259

llvm/include/llvm/Bitcode/BitstreamReader.h

 
 namespace llvm {
 
 /// This class is used to read from an LLVM bitcode stream, maintaining
 /// information that is global to decoding the entire file. While a file is
 /// being read, multiple cursors can be independently advanced or skipped around
 /// within the file.  These are represented by the BitstreamCursor class.
 class BitstreamReader {
-public:
-  /// This contains information emitted to BLOCKINFO_BLOCK blocks. These
-  /// describe abbreviations that all blocks of the specified ID inherit.
-  struct BlockInfo {
-    unsigned BlockID;
-    std::vector<IntrusiveRefCntPtr<BitCodeAbbrev>> Abbrevs;
-    std::string Name;
-    std::vector<std::pair<unsigned, std::string> > RecordNames;
-  };
-
 private:
   std::unique_ptr<MemoryObject> BitcodeBytes;
 
-  std::vector<BlockInfo> BlockInfoRecords;
-
-  /// This is set to true if we don't care about the block/record name
-  /// information in the BlockInfo block. Only llvm-bcanalyzer uses this.
-  bool IgnoreBlockInfoNames;
-
 public:
-  BitstreamReader() : IgnoreBlockInfoNames(true) {
-  }
+  BitstreamReader() {}
 
-  BitstreamReader(const unsigned char *Start, const unsigned char *End)
-      : IgnoreBlockInfoNames(true) {
+  BitstreamReader(const unsigned char *Start, const unsigned char *End) {
     init(Start, End);
   }
 
   BitstreamReader(std::unique_ptr<MemoryObject> BitcodeBytes)
-      : BitcodeBytes(std::move(BitcodeBytes)), IgnoreBlockInfoNames(true) {}
+      : BitcodeBytes(std::move(BitcodeBytes)) {}
 
   void init(const unsigned char *Start, const unsigned char *End) {
     assert(((End-Start) & 3) == 0 &&"Bitcode stream not a multiple of 4 bytes");
     BitcodeBytes.reset(getNonStreamedMemoryObject(Start, End));
   }
 
   MemoryObject &getBitcodeBytes() { return *BitcodeBytes; }
-
-  /// This is called by clients that want block/record name information.
-  void CollectBlockInfoNames() { IgnoreBlockInfoNames = false; }
-  bool isIgnoringBlockInfoNames() { return IgnoreBlockInfoNames; }
-
-  //===--------------------------------------------------------------------===//
-  // Block Manipulation
-  //===--------------------------------------------------------------------===//
-
-  /// Return true if we've already read and processed the block info block for
-  /// this Bitstream. We only process it for the first cursor that walks over
-  /// it.
-  bool hasBlockInfoRecords() const { return !BlockInfoRecords.empty(); }
-
-  /// If there is block info for the specified ID, return it, otherwise return
-  /// null.
-  const BlockInfo *getBlockInfo(unsigned BlockID) const {
-    // Common case, the most recent entry matches BlockID.
-    if (!BlockInfoRecords.empty() && BlockInfoRecords.back().BlockID == BlockID)
-      return &BlockInfoRecords.back();
-
-    for (unsigned i = 0, e = static_cast<unsigned>(BlockInfoRecords.size());
-         i != e; ++i)
-      if (BlockInfoRecords[i].BlockID == BlockID)
-        return &BlockInfoRecords[i];
-    return nullptr;
-  }
-
-  BlockInfo &getOrCreateBlockInfo(unsigned BlockID) {
-    if (const BlockInfo *BI = getBlockInfo(BlockID))
-      return *const_cast<BlockInfo*>(BI);
-
-    // Otherwise, add a new record.
-    BlockInfoRecords.emplace_back();
-    BlockInfoRecords.back().BlockID = BlockID;
-    return BlockInfoRecords.back();
-  }
-
-  /// Takes block info from the other bitstream reader.
-  ///
-  /// This is a "take" operation because BlockInfo records are non-trivial, and
-  /// indeed rather expensive.
-  void takeBlockInfo(BitstreamReader &&Other) {
-    assert(!hasBlockInfoRecords());
-    BlockInfoRecords = std::move(Other.BlockInfoRecords);
-  }
 };
 
 /// This represents a position within a bitstream. There may be multiple
 /// independent cursors reading within one bitstream, each maintaining their
 /// own local state.
 class SimpleBitstreamCursor {
   BitstreamReader *R = nullptr;
   size_t NextChar = 0;

 };
 
 /// This represents a position within a bitcode file, implemented on top of a
 /// SimpleBitstreamCursor.
 ///
 /// Unlike iterators, BitstreamCursors are heavy-weight objects that should not
 /// be passed by value.
 class BitstreamCursor : SimpleBitstreamCursor {
+public:
+  /// This contains information emitted to BLOCKINFO_BLOCK blocks. These
+  /// describe abbreviations that all blocks of the specified ID inherit.
+  struct BlockInfo {
+    unsigned BlockID;
+    std::vector<IntrusiveRefCntPtr<BitCodeAbbrev>> Abbrevs;
+    std::string Name;
+    std::vector<std::pair<unsigned, std::string> > RecordNames;
+  };
+
+private:
+  std::vector<BlockInfo> BlockInfoRecords;
+
+  /// This is set to true if we don't care about the block/record name
+  /// information in the BlockInfo block. Only llvm-bcanalyzer uses this.
+  bool IgnoreBlockInfoNames = true;
+
+  BlockInfo &getOrCreateBlockInfo(unsigned BlockID) {
+    if (const BlockInfo *BI = getBlockInfo(BlockID))
+      return *const_cast<BlockInfo*>(BI);
+
+    // Otherwise, add a new record.
+    BlockInfoRecords.emplace_back();
+    BlockInfoRecords.back().BlockID = BlockID;
+    return BlockInfoRecords.back();
+  }
+
   // This is the declared size of code values used for the current block, in
   // bits.
   unsigned CurCodeSize = 2;
 
   /// Abbrevs installed at in this block.
   std::vector<IntrusiveRefCntPtr<BitCodeAbbrev>> CurAbbrevs;
 
   struct Block {
public:
   unsigned readRecord(unsigned AbbrevID, SmallVectorImpl<uint64_t> &Vals,
                       StringRef *Blob = nullptr);
 
   //===--------------------------------------------------------------------===//
   // Abbrev Processing
   //===--------------------------------------------------------------------===//
   void ReadAbbrevRecord();
 
+  //===--------------------------------------------------------------------===//
+  // Block Manipulation
+  //===--------------------------------------------------------------------===//
+
+  /// Read abbreviations from a block info block. This function may only be
+  /// called once for each cursor.
   bool ReadBlockInfoBlock();
+
+  /// This is called by clients that want block/record name information.
+  void CollectBlockInfoNames() { IgnoreBlockInfoNames = false; }
+
+  /// If there is block info for the specified ID, return it, otherwise return
+  /// null.
+  const BlockInfo *getBlockInfo(unsigned BlockID) const {
+    // Common case, the most recent entry matches BlockID.
+    if (!BlockInfoRecords.empty() && BlockInfoRecords.back().BlockID == BlockID)
+      return &BlockInfoRecords.back();
+
+    for (unsigned i = 0, e = static_cast<unsigned>(BlockInfoRecords.size());
+         i != e; ++i)
+      if (BlockInfoRecords[i].BlockID == BlockID)
+        return &BlockInfoRecords[i];
+    return nullptr;
+  }
+
+  /// Takes block info from the other bitstream cursor.
+  ///
+  /// This is a "take" operation because BlockInfo records are non-trivial, and
+  /// indeed rather expensive.
+  void takeBlockInfo(BitstreamCursor &&Other) {
+    assert(BlockInfoRecords.empty());
+    BlockInfoRecords = std::move(Other.BlockInfoRecords);
+  }
 };
 
 } // end llvm namespace
 
 #endif // LLVM_BITCODE_BITSTREAMREADER_H

llvm/lib/Bitcode/Reader/BitstreamReader.cpp

 /// EnterSubBlock - Having read the ENTER_SUBBLOCK abbrevid, enter
 /// the block, and return true if the block has an error.
 bool BitstreamCursor::EnterSubBlock(unsigned BlockID, unsigned *NumWordsP) {
   // Save the current block's state on BlockScope.
   BlockScope.push_back(Block(CurCodeSize));
   BlockScope.back().PrevAbbrevs.swap(CurAbbrevs);
 
   // Add the abbrevs specific to this block to the CurAbbrevs list.
-  if (const BitstreamReader::BlockInfo *Info =
-          getBitStreamReader()->getBlockInfo(BlockID)) {
+  if (const BlockInfo *Info = getBlockInfo(BlockID)) {
     CurAbbrevs.insert(CurAbbrevs.end(), Info->Abbrevs.begin(),
                       Info->Abbrevs.end());
   }
 
   // Get the codesize of this block.
   CurCodeSize = ReadVBR(bitc::CodeLenWidth);
   // We can't read more than MaxChunkSize at a time
   if (CurCodeSize > MaxChunkSize)
void BitstreamCursor::ReadAbbrevRecord() {
 
   if (Abbv->getNumOperandInfos() == 0)
     report_fatal_error("Abbrev record with no operands");
   CurAbbrevs.push_back(Abbv);
 }
 
 bool BitstreamCursor::ReadBlockInfoBlock() {
   // We expect the client to read the block info block at most once.
-  if (getBitStreamReader()->hasBlockInfoRecords())
+  if (!BlockInfoRecords.empty())
     report_fatal_error("Duplicate read of block info block");
 
   if (EnterSubBlock(bitc::BLOCKINFO_BLOCK_ID)) return true;
 
   SmallVector<uint64_t, 64> Record;
-  BitstreamReader::BlockInfo *CurBlockInfo = nullptr;
+  BlockInfo *CurBlockInfo = nullptr;
 
   // Read all the records for this module.
   while (true) {
     BitstreamEntry Entry = advanceSkippingSubblocks(AF_DontAutoprocessAbbrevs);
 
     switch (Entry.Kind) {
     case llvm::BitstreamEntry::SubBlock: // Handled for us already.
     case llvm::BitstreamEntry::Error:
while (true) {
     }
 
     // Read a record.
     Record.clear();
     switch (readRecord(Entry.ID, Record)) {
       default: break;  // Default behavior, ignore unknown content.
       case bitc::BLOCKINFO_CODE_SETBID:
         if (Record.size() < 1) return true;
-        CurBlockInfo =
-            &getBitStreamReader()->getOrCreateBlockInfo((unsigned)Record[0]);
+        CurBlockInfo = &getOrCreateBlockInfo((unsigned)Record[0]);
         break;
       case bitc::BLOCKINFO_CODE_BLOCKNAME: {
         if (!CurBlockInfo) return true;
-        if (getBitStreamReader()->isIgnoringBlockInfoNames())
+        if (IgnoreBlockInfoNames)
           break; // Ignore name.
         std::string Name;
         for (unsigned i = 0, e = Record.size(); i != e; ++i)
           Name += (char)Record[i];
         CurBlockInfo->Name = Name;
         break;
       }
       case bitc::BLOCKINFO_CODE_SETRECORDNAME: {
         if (!CurBlockInfo) return true;
-        if (getBitStreamReader()->isIgnoringBlockInfoNames())
+        if (IgnoreBlockInfoNames)
           break; // Ignore name.
         std::string Name;
         for (unsigned i = 1, e = Record.size(); i != e; ++i)
           Name += (char)Record[i];
         CurBlockInfo->RecordNames.push_back(std::make_pair((unsigned)Record[0],
                                                            Name));
         break;
       }
     }
   }
 }

llvm/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp

enum CurStreamTypeType {
   UnknownBitstream,
   LLVMIRBitstream
 };
 
 }
 
 /// GetBlockName - Return a symbolic block name if known, otherwise return
 /// null.
-static const char *GetBlockName(unsigned BlockID,
-                                const BitstreamReader &StreamFile,
+static const char *GetBlockName(unsigned BlockID, const BitstreamCursor &Stream,
                                 CurStreamTypeType CurStreamType) {
   // Standard blocks for all bitcode files.
   if (BlockID < bitc::FIRST_APPLICATION_BLOCKID) {
     if (BlockID == bitc::BLOCKINFO_BLOCK_ID)
       return "BLOCKINFO_BLOCK";
     return nullptr;
   }
 
   // Check to see if we have a blockinfo record for this block, with a name.
-  if (const BitstreamReader::BlockInfo *Info =
-        StreamFile.getBlockInfo(BlockID)) {
+  if (const BitstreamCursor::BlockInfo *Info = Stream.getBlockInfo(BlockID)) {
     if (!Info->Name.empty())
       return Info->Name.c_str();
   }
 
 
   if (CurStreamType != LLVMIRBitstream) return nullptr;
 
   switch (BlockID) {
case bitc::GLOBALVAL_SUMMARY_BLOCK_ID:
                                            return "GLOBALVAL_SUMMARY_BLOCK";
   case bitc::MODULE_STRTAB_BLOCK_ID:       return "MODULE_STRTAB_BLOCK";
   }
 }
 
 /// GetCodeName - Return a symbolic code name if known, otherwise return
 /// null.
 static const char *GetCodeName(unsigned CodeID, unsigned BlockID,
-                               const BitstreamReader &StreamFile,
+                               const BitstreamCursor &Stream,
                                CurStreamTypeType CurStreamType) {
   // Standard blocks for all bitcode files.
   if (BlockID < bitc::FIRST_APPLICATION_BLOCKID) {
     if (BlockID == bitc::BLOCKINFO_BLOCK_ID) {
       switch (CodeID) {
       default: return nullptr;
       case bitc::BLOCKINFO_CODE_SETBID:        return "SETBID";
       case bitc::BLOCKINFO_CODE_BLOCKNAME:     return "BLOCKNAME";
       case bitc::BLOCKINFO_CODE_SETRECORDNAME: return "SETRECORDNAME";
       }
     }
     return nullptr;
   }
 
   // Check to see if we have a blockinfo record for this record, with a name.
-  if (const BitstreamReader::BlockInfo *Info =
-        StreamFile.getBlockInfo(BlockID)) {
+  if (const BitstreamCursor::BlockInfo *Info = Stream.getBlockInfo(BlockID)) {
     for (unsigned i = 0, e = Info->RecordNames.size(); i != e; ++i)
       if (Info->RecordNames[i].first == CodeID)
         return Info->RecordNames[i].second.c_str();
   }
 
 
   if (CurStreamType != LLVMIRBitstream) return nullptr;
 
static bool ParseBlock(BitstreamCursor &Stream, unsigned BlockID,
   PerBlockIDStats &BlockStats = BlockIDStats[BlockID];
 
   BlockStats.NumInstances++;
 
   // BLOCKINFO is a special part of the stream.
   bool DumpRecords = Dump;
   if (BlockID == bitc::BLOCKINFO_BLOCK_ID) {
     if (Dump) outs() << Indent << "<BLOCKINFO_BLOCK/>\n";
-    if (BitstreamCursor(Stream).ReadBlockInfoBlock())
+    if (Stream.ReadBlockInfoBlock())
       return ReportError("Malformed BlockInfoBlock");
+    Stream.JumpToBit(BlockBitStart);
     // It's not really interesting to dump the contents of the blockinfo block.
     DumpRecords = false;
   }
 
   unsigned NumWords = 0;
   if (Stream.EnterSubBlock(BlockID, &NumWords))
     return ReportError("Malformed block record");
 
   // Keep it for later, when we see a MODULE_HASH record
   uint64_t BlockEntryPos = Stream.getCurrentByteNo();
 
   const char *BlockName = nullptr;
   if (DumpRecords) {
     outs() << Indent << "<";
-    if ((BlockName = GetBlockName(BlockID, *Stream.getBitStreamReader(),
-                                  CurStreamType)))
+    if ((BlockName = GetBlockName(BlockID, Stream, CurStreamType)))
       outs() << BlockName;
     else
       outs() << "UnknownBlock" << BlockID;
 
     if (NonSymbolic && BlockName)
       outs() << " BlockID=" << BlockID;
 
     outs() << " NumWords=" << NumWords
while (1) {
     if (Entry.ID != bitc::UNABBREV_RECORD) {
       BlockStats.CodeFreq[Code].NumAbbrev++;
       ++BlockStats.NumAbbreviatedRecords;
     }
 
     if (DumpRecords) {
       outs() << Indent << "  <";
       if (const char *CodeName =
-            GetCodeName(Code, BlockID, *Stream.getBitStreamReader(),
-                        CurStreamType))
+              GetCodeName(Code, BlockID, Stream, CurStreamType))
         outs() << CodeName;
       else
         outs() << "UnknownCode" << Code;
-      if (NonSymbolic &&
-          GetCodeName(Code, BlockID, *Stream.getBitStreamReader(),
-                      CurStreamType))
+      if (NonSymbolic && GetCodeName(Code, BlockID, Stream, CurStreamType))
         outs() << " codeid=" << Code;
       const BitCodeAbbrev *Abbv = nullptr;
       if (Entry.ID != bitc::UNABBREV_RECORD) {
         Abbv = Stream.getAbbrev(Entry.ID);
         outs() << " abbrevid=" << Entry.ID;
       }
 
       for (unsigned i = 0, e = Record.size(); i != e; ++i)
if (isBitcodeWrapper(BufPtr, EndBufPtr)) {
     }
 
     if (SkipBitcodeWrapperHeader(BufPtr, EndBufPtr, true))
       return ReportError("Invalid bitcode wrapper header");
   }
 
   StreamFile = BitstreamReader(BufPtr, EndBufPtr);
   Stream = BitstreamCursor(StreamFile);
-  StreamFile.CollectBlockInfoNames();
+  Stream.CollectBlockInfoNames();
 
   // Read the stream signature.
   char Signature[6];
   Signature[0] = Stream.Read(8);
   Signature[1] = Stream.Read(8);
   Signature[2] = Stream.Read(4);
   Signature[3] = Stream.Read(4);
   Signature[4] = Stream.Read(4);
while (!BlockInfoCursor.AtEndOfStream()) {
         if (BlockInfoCursor.ReadBlockInfoBlock())
           return ReportError("Malformed BlockInfoBlock in block info file");
         break;
       }
 
       BlockInfoCursor.SkipBlock();
     }
 
-    StreamFile.takeBlockInfo(std::move(BlockInfoFile));
+    Stream.takeBlockInfo(std::move(BlockInfoCursor));
   }
 
   unsigned NumTopBlocks = 0;
 
   // Parse the top-level structure.  We only allow blocks at the top-level.
   while (!Stream.AtEndOfStream()) {
     unsigned Code = Stream.ReadCode();
     if (Code != bitc::ENTER_SUBBLOCK)
static int AnalyzeBitcode() {
   outs() << "  # Toplevel Blocks: " << NumTopBlocks << "\n";
   outs() << "\n";
 
   // Emit per-block stats.
   outs() << "Per-block Summary:\n";
   for (std::map<unsigned, PerBlockIDStats>::iterator I = BlockIDStats.begin(),
        E = BlockIDStats.end(); I != E; ++I) {
     outs() << "  Block ID #" << I->first;
-    if (const char *BlockName = GetBlockName(I->first, StreamFile,
-                                             CurStreamType))
+    if (const char *BlockName = GetBlockName(I->first, Stream, CurStreamType))
       outs() << " (" << BlockName << ")";
     outs() << ":\n";
 
     const PerBlockIDStats &Stats = I->second;
     outs() << "      Num Instances: " << Stats.NumInstances << "\n";
     outs() << "         Total Size: ";
     PrintSize(Stats.NumBits);
     outs() << "\n";
if (!NoHistogram && !Stats.CodeFreq.empty()) {
         if (RecStats.NumAbbrev)
           outs() <<
               format(" %7.2f",
                      (double)RecStats.NumAbbrev/RecStats.NumInstances*100);
         else
           outs() << "        ";
 
         outs() << "  ";
-        if (const char *CodeName =
-              GetCodeName(FreqPairs[i].second, I->first, StreamFile,
-                          CurStreamType))
+        if (const char *CodeName = GetCodeName(FreqPairs[i].second, I->first,
+                                               Stream, CurStreamType))
           outs() << CodeName << "\n";
         else
           outs() << "UnknownCode" << FreqPairs[i].second << "\n";
       }
       outs() << "\n";
 
     }
   }