From 086ed798ff99f87ddf63184138d5797382c9a0f6 Mon Sep 17 00:00:00 2001 From: Teresa Johnson Date: Fri, 20 Nov 2015 14:51:27 +0000 Subject: [PATCH] [ThinLTO] Add MODULE_CODE_METADATA_VALUES record Summary: This is split out from the ThinLTO metadata mapping patch http://reviews.llvm.org/D14752. To avoid needing to parse the module level metadata during function importing, a new module-level record is added which holds the number of module-level metadata values. This is required because metadata value ids are assigned implicitly during parsing, and the function-level metadata ids start after the module-level metadata ids. I made a change to this version of the code compared to D14752 in order to add more consistent and thorough assertion checking of the new record value. We now unconditionally use the record value to initialize the MDValueList size, and handle it the same in parseMetadata for all module level metadata cases (lazy loading or not). Reviewers: dexonsmith, joker.eph Subscribers: davidxl, llvm-commits, joker.eph Differential Revision: http://reviews.llvm.org/D14825 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@253668 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Bitcode/LLVMBitCodes.h | 3 ++ lib/Bitcode/Reader/BitcodeReader.cpp | 62 ++++++++++++++++++++--- lib/Bitcode/Writer/BitcodeWriter.cpp | 15 ++++++ lib/Bitcode/Writer/ValueEnumerator.h | 1 + tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp | 1 + 5 files changed, 76 insertions(+), 6 deletions(-) diff --git a/include/llvm/Bitcode/LLVMBitCodes.h b/include/llvm/Bitcode/LLVMBitCodes.h index efce0fbcd80..7cb0d7edff4 100644 --- a/include/llvm/Bitcode/LLVMBitCodes.h +++ b/include/llvm/Bitcode/LLVMBitCodes.h @@ -102,6 +102,9 @@ enum { BITCODE_CURRENT_EPOCH = 0 }; // ALIAS: [alias value type, addrspace, aliasee val#, linkage, visibility] MODULE_CODE_ALIAS = 14, + + // METADATA_VALUES: [numvals] + MODULE_CODE_METADATA_VALUES = 15, }; /// PARAMATTR blocks have code for defining a parameter attribute set. diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp index 36bf1e8ef63..f2b0a0fd678 100644 --- a/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/lib/Bitcode/Reader/BitcodeReader.cpp @@ -154,6 +154,11 @@ class BitcodeReader : public GVMaterializer { uint64_t VSTOffset = 0; // Contains an arbitrary and optional string identifying the bitcode producer std::string ProducerIdentification; + // Number of module level metadata records specified by the + // MODULE_CODE_METADATA_VALUES record. + unsigned NumModuleMDs = 0; + // Support older bitcode without the MODULE_CODE_METADATA_VALUES record. + bool SeenModuleValuesRecord = false; std::vector TypeList; BitcodeReaderValueList ValueList; @@ -404,7 +409,7 @@ private: std::error_code parseFunctionBody(Function *F); std::error_code globalCleanup(); std::error_code resolveGlobalAndAliasInits(); - std::error_code parseMetadata(); + std::error_code parseMetadata(bool ModuleLevel = false); std::error_code parseMetadataKinds(); std::error_code parseMetadataKindRecord(SmallVectorImpl &Record); std::error_code parseMetadataAttachment(Function &F); @@ -1911,9 +1916,25 @@ BitcodeReader::parseMetadataKindRecord(SmallVectorImpl &Record) { static int64_t unrotateSign(uint64_t U) { return U & 1 ? ~(U >> 1) : U >> 1; } -std::error_code BitcodeReader::parseMetadata() { +/// Parse a METADATA_BLOCK. If ModuleLevel is true then we are parsing +/// module level metadata. +std::error_code BitcodeReader::parseMetadata(bool ModuleLevel) { IsMetadataMaterialized = true; unsigned NextMDValueNo = MDValueList.size(); + if (ModuleLevel && SeenModuleValuesRecord) { + // Now that we are parsing the module level metadata, we want to restart + // the numbering of the MD values, and replace temp MD created earlier + // with their real values. If we saw a METADATA_VALUE record then we + // would have set the MDValueList size to the number specified in that + // record, to support parsing function-level metadata first, and we need + // to reset back to 0 to fill the MDValueList in with the parsed module + // The function-level metadata parsing should have reset the MDValueList + // size back to the value reported by the METADATA_VALUE record, saved in + // NumModuleMDs. + assert(NumModuleMDs == MDValueList.size() && + "Expected MDValueList to only contain module level values"); + NextMDValueNo = 0; + } if (Stream.EnterSubBlock(bitc::METADATA_BLOCK_ID)) return error("Invalid record"); @@ -2375,6 +2396,9 @@ std::error_code BitcodeReader::parseMetadata() { } } } + assert((!(ModuleLevel && SeenModuleValuesRecord) || + NumModuleMDs == MDValueList.size()) && + "Inconsistent bitcode: METADATA_VALUES mismatch"); #undef GET_OR_DISTINCT } @@ -3062,7 +3086,7 @@ std::error_code BitcodeReader::materializeMetadata() { for (uint64_t BitPos : DeferredMetadataInfo) { // Move the bit stream to the saved position. Stream.JumpToBit(BitPos); - if (std::error_code EC = parseMetadata()) + if (std::error_code EC = parseMetadata(true)) return EC; } DeferredMetadataInfo.clear(); @@ -3274,7 +3298,7 @@ std::error_code BitcodeReader::parseModule(uint64_t ResumeBit, break; } assert(DeferredMetadataInfo.empty() && "Unexpected deferred metadata"); - if (std::error_code EC = parseMetadata()) + if (std::error_code EC = parseMetadata(true)) return EC; break; case bitc::METADATA_KIND_BLOCK_ID: @@ -3654,6 +3678,28 @@ std::error_code BitcodeReader::parseModule(uint64_t ResumeBit, return error("Invalid record"); VSTOffset = Record[0]; break; + /// MODULE_CODE_METADATA_VALUES: [numvals] + case bitc::MODULE_CODE_METADATA_VALUES: + if (Record.size() < 1) + return error("Invalid record"); + assert(!IsMetadataMaterialized); + // This record contains the number of metadata values in the module-level + // METADATA_BLOCK. It is used to support lazy parsing of metadata as + // a postpass, where we will parse function-level metadata first. + // This is needed because the ids of metadata are assigned implicitly + // based on their ordering in the bitcode, with the function-level + // metadata ids starting after the module-level metadata ids. Otherwise, + // we would have to parse the module-level metadata block to prime the + // MDValueList when we are lazy loading metadata during function + // importing. Initialize the MDValueList size here based on the + // record value, regardless of whether we are doing lazy metadata + // loading, so that we have consistent handling and assertion + // checking in parseMetadata for module-level metadata. + NumModuleMDs = Record[0]; + SeenModuleValuesRecord = true; + assert(MDValueList.size() == 0); + MDValueList.resize(NumModuleMDs); + break; } Record.clear(); } @@ -5212,8 +5258,12 @@ std::error_code BitcodeReader::findFunctionInStream( void BitcodeReader::releaseBuffer() { Buffer.release(); } std::error_code BitcodeReader::materialize(GlobalValue *GV) { - if (std::error_code EC = materializeMetadata()) - return EC; + // In older bitcode we must materialize the metadata before parsing + // any functions, in order to set up the MDValueList properly. + if (!SeenModuleValuesRecord) { + if (std::error_code EC = materializeMetadata()) + return EC; + } Function *F = dyn_cast(GV); // If it's not a function or is already material, ignore the request. diff --git a/lib/Bitcode/Writer/BitcodeWriter.cpp b/lib/Bitcode/Writer/BitcodeWriter.cpp index 7586112325e..b1b699765bd 100644 --- a/lib/Bitcode/Writer/BitcodeWriter.cpp +++ b/lib/Bitcode/Writer/BitcodeWriter.cpp @@ -776,6 +776,21 @@ static uint64_t WriteModuleInfo(const Module *M, const ValueEnumerator &VE, Vals.clear(); } + // Write a record indicating the number of module-level metadata IDs + // This is needed because the ids of metadata are assigned implicitly + // based on their ordering in the bitcode, with the function-level + // metadata ids starting after the module-level metadata ids. For + // function importing where we lazy load the metadata as a postpass, + // we want to avoid parsing the module-level metadata before parsing + // the imported functions. + BitCodeAbbrev *Abbv = new BitCodeAbbrev(); + Abbv->Add(BitCodeAbbrevOp(bitc::MODULE_CODE_METADATA_VALUES)); + Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); + unsigned MDValsAbbrev = Stream.EmitAbbrev(Abbv); + Vals.push_back(VE.numMDs()); + Stream.EmitRecord(bitc::MODULE_CODE_METADATA_VALUES, Vals, MDValsAbbrev); + Vals.clear(); + uint64_t VSTOffsetPlaceholder = WriteValueSymbolTableForwardDecl(M->getValueSymbolTable(), Stream); return VSTOffsetPlaceholder; diff --git a/lib/Bitcode/Writer/ValueEnumerator.h b/lib/Bitcode/Writer/ValueEnumerator.h index 92d166e3ba9..abe383d7ee4 100644 --- a/lib/Bitcode/Writer/ValueEnumerator.h +++ b/lib/Bitcode/Writer/ValueEnumerator.h @@ -119,6 +119,7 @@ public: unsigned getMetadataOrNullID(const Metadata *MD) const { return MDValueMap.lookup(MD); } + unsigned numMDs() const { return MDs.size(); } bool hasMDString() const { return HasMDString; } bool hasDILocation() const { return HasDILocation; } diff --git a/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp b/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp index c460eb534cd..f08b0c4cbc0 100644 --- a/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp +++ b/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp @@ -172,6 +172,7 @@ static const char *GetCodeName(unsigned CodeID, unsigned BlockID, STRINGIFY_CODE(MODULE_CODE, PURGEVALS) STRINGIFY_CODE(MODULE_CODE, GCNAME) STRINGIFY_CODE(MODULE_CODE, VSTOFFSET) + STRINGIFY_CODE(MODULE_CODE, METADATA_VALUES) } case bitc::IDENTIFICATION_BLOCK_ID: switch (CodeID) { -- 2.34.1