Use a different block id for block of metadata kind records

[tejohnson]

Summary

There are currently two blocks with the METADATA_BLOCK id at module
scope. The first has the module-level metadata values (consisting of
some combination of METADATA_* record codes except for METADATA_KIND).
The second consists only of METADATA_KIND records. The latter is used
only in the METADATA_ATTACHMENT block within function blocks (for
metadata attached to instructions).

For ThinLTO we want to delay the parsing of module level metadata
until all functions have been imported from that module (there is some
bookkeeping used to suture it up when we read it during a post-pass).
However, we do need the METADATA_KIND records when parsing the function
body during importing, since those kinds are used as described above.

To simplify identification and parsing of just the block containing
the metadata kinds, use a different block id (METADATA_KIND_BLOCK_ID).
Support older bitcode without the new block id as well.

[mehdi_amini]

lib/Bitcode/Reader/BitcodeReader.cpp

409 ↗

(On Diff #40146)

Usually function parameters are SmallVectorImpl

2409 ↗

(On Diff #40146)

What kind of record is allowed here that we want to ignore?

[tejohnson]

lib/Bitcode/Reader/BitcodeReader.cpp

409 ↗

(On Diff #40146)

Will fix.

2409 ↗

(On Diff #40146)

This is consistent with all the other parsing routines that ignore unrecognized blocks and records.

[tejohnson]

• Use SmallVectorImpl for passing Record as parameter.

llvm/trunk/include/llvm/Bitcode/LLVMBitCodes.h

enum BlockIDs {
 
   TYPE_BLOCK_ID_NEW,
 
   USELIST_BLOCK_ID,
 
   MODULE_STRTAB_BLOCK_ID,
   FUNCTION_SUMMARY_BLOCK_ID,
 
-  OPERAND_BUNDLE_TAGS_BLOCK_ID
+  OPERAND_BUNDLE_TAGS_BLOCK_ID,
+
+  METADATA_KIND_BLOCK_ID
 };
 
 /// Identification block contains a string that describes the producer details,
 /// and an epoch that defines the auto-upgrade capability.
 enum IdentificationCodes {
   IDENTIFICATION_CODE_STRING = 1, // IDENTIFICATION:      [strchr x N]
   IDENTIFICATION_CODE_EPOCH = 2,  // EPOCH:               [epoch#]
 };

llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp

private:
   std::error_code rememberAndSkipFunctionBodies();
   std::error_code rememberAndSkipFunctionBody();
   /// Save the positions of the Metadata blocks and skip parsing the blocks.
   std::error_code rememberAndSkipMetadata();
   std::error_code parseFunctionBody(Function *F);
   std::error_code globalCleanup();
   std::error_code resolveGlobalAndAliasInits();
   std::error_code parseMetadata();
+  std::error_code parseMetadataKinds();
+  std::error_code parseMetadataKindRecord(SmallVectorImpl<uint64_t> &Record);
   std::error_code parseMetadataAttachment(Function &F);
   ErrorOr<std::string> parseModuleTriple();
   std::error_code parseUseLists();
   std::error_code initStream(std::unique_ptr<DataStreamer> Streamer);
   std::error_code initStreamFromBuffer();
   std::error_code initLazyStream(std::unique_ptr<DataStreamer> Streamer);
   std::error_code findFunctionInStream(
       Function *F,
case bitc::VST_CODE_BBENTRY: {
       BB->setName(StringRef(ValueName.data(), ValueName.size()));
       ValueName.clear();
       break;
     }
     }
   }
 }
 
+/// Parse a single METADATA_KIND record, inserting result in MDKindMap.
+std::error_code
+BitcodeReader::parseMetadataKindRecord(SmallVectorImpl<uint64_t> &Record) {
+  if (Record.size() < 2)
+    return error("Invalid record");
+
+  unsigned Kind = Record[0];
+  SmallString<8> Name(Record.begin() + 1, Record.end());
+
+  unsigned NewKind = TheModule->getMDKindID(Name.str());
+  if (!MDKindMap.insert(std::make_pair(Kind, NewKind)).second)
+    return error("Conflicting METADATA_KIND records");
+  return std::error_code();
+}
+
 static int64_t unrotateSign(uint64_t U) { return U & 1 ? ~(U >> 1) : U >> 1; }
 
 std::error_code BitcodeReader::parseMetadata() {
   IsMetadataMaterialized = true;
   unsigned NextMDValueNo = MDValueList.size();
 
   if (Stream.EnterSubBlock(bitc::METADATA_BLOCK_ID))
     return error("Invalid record");
while (1) {
     case bitc::METADATA_STRING: {
       std::string String(Record.begin(), Record.end());
       llvm::UpgradeMDStringConstant(String);
       Metadata *MD = MDString::get(Context, String);
       MDValueList.assignValue(MD, NextMDValueNo++);
       break;
     }
     case bitc::METADATA_KIND: {
-      if (Record.size() < 2)
-        return error("Invalid record");
+      // Support older bitcode files that had METADATA_KIND records in a
+      // block with METADATA_BLOCK_ID.
+      if (std::error_code EC = parseMetadataKindRecord(Record))
+        return EC;
+      break;
+    }
+    }
+  }
+#undef GET_OR_DISTINCT
+}
+
+/// Parse the metadata kinds out of the METADATA_KIND_BLOCK.
+std::error_code BitcodeReader::parseMetadataKinds() {
+  if (Stream.EnterSubBlock(bitc::METADATA_KIND_BLOCK_ID))
 
-      unsigned Kind = Record[0];
-      SmallString<8> Name(Record.begin()+1, Record.end());
+  SmallVector<uint64_t, 64> Record;
 
-      unsigned NewKind = TheModule->getMDKindID(Name.str());
-      if (!MDKindMap.insert(std::make_pair(Kind, NewKind)).second)
-        return error("Conflicting METADATA_KIND records");
+  // Read all the records.
+  while (1) {
+    BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
+
+    switch (Entry.Kind) {
+    case BitstreamEntry::SubBlock: // Handled for us already.
+    case BitstreamEntry::Error:
+      return error("Malformed block");
+    case BitstreamEntry::EndBlock:
+      return std::error_code();
+    case BitstreamEntry::Record:
+      // The interesting case.
+      break;
+    }
+
+    // Read a record.
+    Record.clear();
+    unsigned Code = Stream.readRecord(Entry.ID, Record);
+    switch (Code) {
+    default: // Default behavior: ignore.
+      break;
+    case bitc::METADATA_KIND: {
+      if (std::error_code EC = parseMetadataKindRecord(Record))
+        return EC;
       break;
     }
     }
   }
-#undef GET_OR_DISTINCT
 }
 
 /// Decode a signed value stored with the sign bit in the LSB for dense VBR
 /// encoding.
 uint64_t BitcodeReader::decodeSignRotatedValue(uint64_t V) {
   if ((V & 1) == 0)
     return V >> 1;
   if (V != 1)
case BitstreamEntry::SubBlock:
           if (std::error_code EC = rememberAndSkipMetadata())
             return EC;
           break;
         }
         assert(DeferredMetadataInfo.empty() && "Unexpected deferred metadata");
         if (std::error_code EC = parseMetadata())
           return EC;
         break;
+      case bitc::METADATA_KIND_BLOCK_ID:
+        if (std::error_code EC = parseMetadataKinds())
+          return EC;
+        break;
       case bitc::FUNCTION_BLOCK_ID:
         // If this is the first function body we've seen, reverse the
         // FunctionsWithBodies list.
         if (!SeenFirstFunctionBody) {
           std::reverse(FunctionsWithBodies.begin(), FunctionsWithBodies.end());
           if (std::error_code EC = globalCleanup())
             return EC;
           SeenFirstFunctionBody = true;

llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp

static void WriteModuleMetadataStore(const Module *M, BitstreamWriter &Stream) {
 
   // Write metadata kinds
   // METADATA_KIND - [n x [id, name]]
   SmallVector<StringRef, 8> Names;
   M->getMDKindNames(Names);
 
   if (Names.empty()) return;
 
-  Stream.EnterSubblock(bitc::METADATA_BLOCK_ID, 3);
+  Stream.EnterSubblock(bitc::METADATA_KIND_BLOCK_ID, 3);
 
   for (unsigned MDKindID = 0, e = Names.size(); MDKindID != e; ++MDKindID) {
     Record.push_back(MDKindID);
     StringRef KName = Names[MDKindID];
     Record.append(KName.begin(), KName.end());
 
     Stream.EmitRecord(bitc::METADATA_KIND, Record, 0);
     Record.clear();

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

static const char *GetBlockName(unsigned BlockID,
   case bitc::PARAMATTR_GROUP_BLOCK_ID: return "PARAMATTR_GROUP_BLOCK_ID";
   case bitc::TYPE_BLOCK_ID_NEW:        return "TYPE_BLOCK_ID";
   case bitc::CONSTANTS_BLOCK_ID:       return "CONSTANTS_BLOCK";
   case bitc::FUNCTION_BLOCK_ID:        return "FUNCTION_BLOCK";
   case bitc::IDENTIFICATION_BLOCK_ID:
     return "IDENTIFICATION_BLOCK_ID";
   case bitc::VALUE_SYMTAB_BLOCK_ID:    return "VALUE_SYMTAB";
   case bitc::METADATA_BLOCK_ID:        return "METADATA_BLOCK";
+  case bitc::METADATA_KIND_BLOCK_ID:   return "METADATA_KIND_BLOCK";
   case bitc::METADATA_ATTACHMENT_ID:   return "METADATA_ATTACHMENT_BLOCK";
   case bitc::USELIST_BLOCK_ID:         return "USELIST_BLOCK_ID";
   case bitc::FUNCTION_SUMMARY_BLOCK_ID:
                                        return "FUNCTION_SUMMARY_BLOCK";
   case bitc::MODULE_STRTAB_BLOCK_ID:   return "MODULE_STRTAB_BLOCK";
   }
 }
 
case bitc::METADATA_ATTACHMENT_ID:
     default:return nullptr;
       STRINGIFY_CODE(METADATA, ATTACHMENT)
     }
   case bitc::METADATA_BLOCK_ID:
     switch(CodeID) {
     default:return nullptr;
       STRINGIFY_CODE(METADATA, STRING)
       STRINGIFY_CODE(METADATA, NAME)
-      STRINGIFY_CODE(METADATA, KIND)
+      STRINGIFY_CODE(METADATA, KIND) // Older bitcode has it in a MODULE_BLOCK
       STRINGIFY_CODE(METADATA, NODE)
       STRINGIFY_CODE(METADATA, VALUE)
       STRINGIFY_CODE(METADATA, OLD_NODE)
       STRINGIFY_CODE(METADATA, OLD_FN_NODE)
       STRINGIFY_CODE(METADATA, NAMED_NODE)
       STRINGIFY_CODE(METADATA, DISTINCT_NODE)
       STRINGIFY_CODE(METADATA, LOCATION)
       STRINGIFY_CODE(METADATA, GENERIC_DEBUG)
default:return nullptr;
       STRINGIFY_CODE(METADATA, TEMPLATE_VALUE)
       STRINGIFY_CODE(METADATA, GLOBAL_VAR)
       STRINGIFY_CODE(METADATA, LOCAL_VAR)
       STRINGIFY_CODE(METADATA, EXPRESSION)
       STRINGIFY_CODE(METADATA, OBJC_PROPERTY)
       STRINGIFY_CODE(METADATA, IMPORTED_ENTITY)
       STRINGIFY_CODE(METADATA, MODULE)
     }
+  case bitc::METADATA_KIND_BLOCK_ID:
+    switch (CodeID) {
+    default:
+      return nullptr;
+      STRINGIFY_CODE(METADATA, KIND)
+    }
   case bitc::USELIST_BLOCK_ID:
     switch(CodeID) {
     default:return nullptr;
     case bitc::USELIST_CODE_DEFAULT: return "USELIST_CODE_DEFAULT";
     case bitc::USELIST_CODE_BB:      return "USELIST_CODE_BB";
     }
   }
 #undef STRINGIFY_CODE