Index: docs/BitCodeFormat.rst =================================================================== --- docs/BitCodeFormat.rst +++ docs/BitCodeFormat.rst @@ -862,16 +862,6 @@ ``gc`` attributes within the module. These records can be referenced by 1-based index in the *gc* fields of ``FUNCTION`` records. -MODULE_CODE_GLOBALVAR_ATTACHMENT Record -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -``[GLOBALVAR_ATTACHMENT, valueid, n x [id, mdnode]]`` - -The ``GLOBALVAR_ATTACHMENT`` record (code 19) describes the metadata -attachments for a global variable. The ``valueid`` is the value index for -the global variable, and the remaining fields are pairs of metadata name -indices and metadata node indices. - .. _PARAMATTR_BLOCK: PARAMATTR_BLOCK Contents Index: include/llvm/Bitcode/LLVMBitCodes.h =================================================================== --- include/llvm/Bitcode/LLVMBitCodes.h +++ include/llvm/Bitcode/LLVMBitCodes.h @@ -113,9 +113,6 @@ // IFUNC: [ifunc value type, addrspace, resolver val#, linkage, visibility] MODULE_CODE_IFUNC = 18, - - // GLOBALVAR_ATTACHMENT: [valueid, n x [id, mdnode]] - MODULE_CODE_GLOBALVAR_ATTACHMENT = 19, }; /// PARAMATTR blocks have code for defining a parameter attribute set. @@ -260,6 +257,7 @@ METADATA_MACRO = 33, // [distinct, macinfo, line, name, value] METADATA_MACRO_FILE = 34, // [distinct, macinfo, line, file, ...] METADATA_STRINGS = 35, // [count, offset] blob([lengths][chars]) + METADATA_GLOBAL_DECL_ATTACHMENT = 36, // [valueid, n x [id, mdnode]] }; // The constants block (CONSTANTS_BLOCK_ID) describes emission for each Index: lib/AsmParser/LLParser.cpp =================================================================== --- lib/AsmParser/LLParser.cpp +++ lib/AsmParser/LLParser.cpp @@ -397,8 +397,21 @@ assert(Lex.getKind() == lltok::kw_declare); Lex.Lex(); + std::vector> MDs; + while (Lex.getKind() == lltok::MetadataVar) { + unsigned MDK; + MDNode *N; + if (ParseMetadataAttachment(MDK, N)) + return true; + MDs.push_back({MDK, N}); + } + Function *F; - return ParseFunctionHeader(F, false); + if (ParseFunctionHeader(F, false)) + return true; + for (auto &MD : MDs) + F->addMetadata(MD.first, *MD.second); + return false; } /// toplevelentity Index: lib/Bitcode/Reader/BitcodeReader.cpp =================================================================== --- lib/Bitcode/Reader/BitcodeReader.cpp +++ lib/Bitcode/Reader/BitcodeReader.cpp @@ -2692,6 +2692,16 @@ parseMetadataStrings(Record, Blob, NextMetadataNo)) return EC; break; + case bitc::METADATA_GLOBAL_DECL_ATTACHMENT: { + if (Record.size() % 2 == 0) + return error("Invalid record"); + unsigned ValueID = Record[0]; + if (ValueID >= ValueList.size()) + return error("Invalid record"); + if (auto *GO = dyn_cast(ValueList[ValueID])) + parseGlobalObjectAttachment(*GO, ArrayRef(Record).slice(1)); + break; + } case bitc::METADATA_KIND: { // Support older bitcode files that had METADATA_KIND records in a // block with METADATA_BLOCK_ID. @@ -3840,16 +3850,6 @@ break; } - case bitc::MODULE_CODE_GLOBALVAR_ATTACHMENT: { - if (Record.size() % 2 == 0) - return error("Invalid record"); - unsigned ValueID = Record[0]; - if (ValueID >= ValueList.size()) - return error("Invalid record"); - if (auto *GV = dyn_cast(ValueList[ValueID])) - parseGlobalObjectAttachment(*GV, ArrayRef(Record).slice(1)); - break; - } // FUNCTION: [type, callingconv, isproto, linkage, paramattr, // alignment, section, visibility, gc, unnamed_addr, // prologuedata, dllstorageclass, comdat, prefixdata] Index: lib/Bitcode/Writer/BitcodeWriter.cpp =================================================================== --- lib/Bitcode/Writer/BitcodeWriter.cpp +++ lib/Bitcode/Writer/BitcodeWriter.cpp @@ -227,7 +227,7 @@ void writeGlobalVariableMetadataAttachment(const GlobalVariable &GV); void pushGlobalMetadataAttachment(SmallVectorImpl &Record, const GlobalObject &GO); - void writeModuleMetadataStore(); + void writeModuleMetadataKinds(); void writeOperandBundleTags(); void writeConstants(unsigned FirstVal, unsigned LastVal, bool isGlobal); void writeModuleConstants(); @@ -1832,6 +1832,22 @@ writeMetadataStrings(VE.getMDStrings(), Record); writeMetadataRecords(VE.getNonMDStrings(), Record); writeNamedMetadata(Record); + + auto AddDeclAttachedMetadata = [&](const GlobalObject &GO) { + SmallVector Record; + Record.push_back(VE.getValueID(&GO)); + pushGlobalMetadataAttachment(Record, GO); + Stream.EmitRecord(bitc::METADATA_GLOBAL_DECL_ATTACHMENT, Record); + }; + for (const Function &F : M) + if (F.isDeclaration() && F.hasMetadata()) + AddDeclAttachedMetadata(F); + // FIXME: Only store metadata for declarations here, and move data for global + // variable definitions to a separate block (PR28134). + for (const GlobalVariable &GV : M.globals()) + if (GV.hasMetadata()) + AddDeclAttachedMetadata(GV); + Stream.ExitBlock(); } @@ -1892,7 +1908,7 @@ Stream.ExitBlock(); } -void ModuleBitcodeWriter::writeModuleMetadataStore() { +void ModuleBitcodeWriter::writeModuleMetadataKinds() { SmallVector Record; // Write metadata kinds @@ -3593,11 +3609,11 @@ // Emit constants. writeModuleConstants(); - // Emit metadata. - writeModuleMetadata(); + // Emit metadata kind names. + writeModuleMetadataKinds(); // Emit metadata. - writeModuleMetadataStore(); + writeModuleMetadata(); // Emit module-level use-lists. if (VE.shouldPreserveUseListOrder()) @@ -3619,14 +3635,6 @@ writeValueSymbolTable(M.getValueSymbolTable(), /* IsModuleLevel */ true, &FunctionToBitcodeIndex); - for (const GlobalVariable &GV : M.globals()) - if (GV.hasMetadata()) { - SmallVector Record; - Record.push_back(VE.getValueID(&GV)); - pushGlobalMetadataAttachment(Record, GV); - Stream.EmitRecord(bitc::MODULE_CODE_GLOBALVAR_ATTACHMENT, Record); - } - if (GenerateHash) { writeModuleHash(BlockStartPos); } Index: lib/Bitcode/Writer/ValueEnumerator.h =================================================================== --- lib/Bitcode/Writer/ValueEnumerator.h +++ lib/Bitcode/Writer/ValueEnumerator.h @@ -255,7 +255,7 @@ /// it's an \a MDNode. const MDNode *enumerateMetadataImpl(unsigned F, const Metadata *MD); - unsigned getMetadataGlobalID(const GlobalObject *GO) const; + unsigned getMetadataFunctionID(const Function *F) const; /// Enumerate reachable metadata in (almost) post-order. /// @@ -272,7 +272,7 @@ /// \a organizeMetadata() will later partition distinct nodes ahead of /// uniqued ones. ///{ - void EnumerateMetadata(const GlobalObject *GO, const Metadata *MD); + void EnumerateMetadata(const Function *F, const Metadata *MD); void EnumerateMetadata(unsigned F, const Metadata *MD); ///} Index: lib/Bitcode/Writer/ValueEnumerator.cpp =================================================================== --- lib/Bitcode/Writer/ValueEnumerator.cpp +++ lib/Bitcode/Writer/ValueEnumerator.cpp @@ -348,7 +348,10 @@ MDs.clear(); GV.getAllMetadata(MDs); for (const auto &I : MDs) - EnumerateMetadata(&GV, I.second); + // FIXME: Pass GV to EnumerateMetadata and arrange for the bitcode writer + // to write metadata to the global variable's own metadata block + // (PR28134). + EnumerateMetadata(nullptr, I.second); } // Enumerate types used by function bodies and argument lists. @@ -360,7 +363,7 @@ MDs.clear(); F.getAllMetadata(MDs); for (const auto &I : MDs) - EnumerateMetadata(&F, I.second); + EnumerateMetadata(F.isDeclaration() ? nullptr : &F, I.second); for (const BasicBlock &BB : F) for (const Instruction &I : BB) { @@ -530,18 +533,17 @@ EnumerateMetadata(nullptr, MD->getOperand(i)); } -unsigned ValueEnumerator::getMetadataGlobalID(const GlobalObject *GO) const { - return GO ? getValueID(GO) + 1 : 0; +unsigned ValueEnumerator::getMetadataFunctionID(const Function *F) const { + return F ? getValueID(F) + 1 : 0; } -void ValueEnumerator::EnumerateMetadata(const GlobalObject *GO, - const Metadata *MD) { - EnumerateMetadata(getMetadataGlobalID(GO), MD); +void ValueEnumerator::EnumerateMetadata(const Function *F, const Metadata *MD) { + EnumerateMetadata(getMetadataFunctionID(F), MD); } void ValueEnumerator::EnumerateFunctionLocalMetadata( const Function &F, const LocalAsMetadata *Local) { - EnumerateFunctionLocalMetadata(getMetadataGlobalID(&F), Local); + EnumerateFunctionLocalMetadata(getMetadataFunctionID(&F), Local); } void ValueEnumerator::dropFunctionFromMetadata( Index: lib/IR/AsmWriter.cpp =================================================================== --- lib/IR/AsmWriter.cpp +++ lib/IR/AsmWriter.cpp @@ -2615,9 +2615,15 @@ Out << "; Function Attrs: " << AttrStr << '\n'; } - if (F->isDeclaration()) - Out << "declare "; - else + Machine.incorporateFunction(F); + + if (F->isDeclaration()) { + Out << "declare"; + SmallVector, 4> MDs; + F->getAllMetadata(MDs); + printMetadataAttachments(MDs, " "); + Out << ' '; + } else Out << "define "; Out << getLinkagePrintName(F->getLinkage()); @@ -2637,7 +2643,6 @@ Out << ' '; WriteAsOperandInternal(Out, F, &TypePrinter, &Machine, F->getParent()); Out << '('; - Machine.incorporateFunction(F); // Loop over the arguments, printing them... if (F->isDeclaration() && !IsForDebug) { @@ -2697,13 +2702,13 @@ writeOperand(F->getPersonalityFn(), /*PrintType=*/true); } - SmallVector, 4> MDs; - F->getAllMetadata(MDs); - printMetadataAttachments(MDs, " "); - if (F->isDeclaration()) { Out << '\n'; } else { + SmallVector, 4> MDs; + F->getAllMetadata(MDs); + printMetadataAttachments(MDs, " "); + Out << " {"; // Output all of the function's basic blocks. for (Function::const_iterator I = F->begin(), E = F->end(); I != E; ++I) Index: lib/IR/Verifier.cpp =================================================================== --- lib/IR/Verifier.cpp +++ lib/IR/Verifier.cpp @@ -1956,8 +1956,15 @@ Assert(MDs.empty(), "unmaterialized function cannot have metadata", &F, MDs.empty() ? nullptr : MDs.front().second); } else if (F.isDeclaration()) { - Assert(MDs.empty(), "function without a body cannot have metadata", &F, - MDs.empty() ? nullptr : MDs.front().second); + for (const auto &I : MDs) { + AssertDI(I.first != LLVMContext::MD_dbg, + "function declaration may not have a !dbg attachment", &F); + Assert(I.first != LLVMContext::MD_prof, + "function declaration may not have a !prof attachment", &F); + + // Verify the metadata itself. + visitMDNode(*I.second); + } Assert(!F.hasPersonalityFn(), "Function declaration shouldn't have a personality routine", &F); } else { Index: test/Assembler/metadata-decl.ll =================================================================== --- /dev/null +++ test/Assembler/metadata-decl.ll @@ -0,0 +1,11 @@ +; RUN: llvm-as < %s | llvm-dis | llvm-as | llvm-dis | FileCheck %s +; RUN: llvm-as < %s | llvm-dis -materialize-metadata | FileCheck %s + +; CHECK: @foo = external global i32, !foo !0 +@foo = external global i32, !foo !0 + +; CHECK: declare !bar !1 void @bar() +declare !bar !1 void @bar() + +!0 = distinct !{} +!1 = distinct !{} Index: test/Assembler/metadata.ll =================================================================== --- test/Assembler/metadata.ll +++ test/Assembler/metadata.ll @@ -1,7 +1,8 @@ -; RUN: llvm-as < %s | llvm-dis | llvm-as | llvm-dis | FileCheck %s +; RUN: llvm-as < %s | llvm-dis | llvm-as | llvm-dis | FileCheck --check-prefix=CHECK --check-prefix=CHECK-UNMAT %s +; RUN: llvm-as < %s | llvm-dis -materialize-metadata | FileCheck --check-prefix=CHECK-UNMAT %s ; RUN: verify-uselistorder %s -; CHECK: @global = global i32 0, !foo [[M2:![0-9]+]], !foo [[M3:![0-9]+]], !baz [[M3]] +; CHECK-UNMAT: @global = global i32 0, !foo [[M2:![0-9]+]], !foo [[M3:![0-9]+]], !baz [[M3]] @global = global i32 0, !foo !2, !foo !3, !baz !3 ; CHECK-LABEL: @test @@ -32,8 +33,8 @@ unreachable, !\34\32abc !4 } -; CHECK: [[M2]] = distinct !{} -; CHECK: [[M3]] = distinct !{} +; CHECK-UNMAT: [[M2]] = distinct !{} +; CHECK-UNMAT: [[M3]] = distinct !{} ; CHECK: [[M0]] = !DILocation ; CHECK: [[M1]] = distinct !DISubprogram ; CHECK: [[M4]] = distinct !{} Index: test/Verifier/metadata-function-dbg.ll =================================================================== --- test/Verifier/metadata-function-dbg.ll +++ test/Verifier/metadata-function-dbg.ll @@ -1,11 +1,14 @@ ; RUN: not llvm-as %s -disable-output 2>&1 | FileCheck %s -define void @foo() !dbg !4 { +; CHECK: function declaration may not have a !dbg attachment +declare !dbg !4 void @f1() + +define void @f2() !dbg !4 { unreachable } ; CHECK: function must have a single !dbg attachment -define void @foo2() !dbg !4 !dbg !4 { +define void @f3() !dbg !4 !dbg !4 { unreachable } Index: test/Verifier/metadata-function-prof.ll =================================================================== --- test/Verifier/metadata-function-prof.ll +++ test/Verifier/metadata-function-prof.ll @@ -1,11 +1,14 @@ ; RUN: not llvm-as %s -disable-output 2>&1 | FileCheck %s -define void @foo() !prof !0 { +; CHECK: function declaration may not have a !prof attachment +declare !prof !0 void @f1() + +define void @f2() !prof !0 { unreachable } ; CHECK: function must have a single !prof attachment -define void @foo2() !prof !0 !prof !0 { +define void @f3() !prof !0 !prof !0 { unreachable } Index: tools/llvm-dis/llvm-dis.cpp =================================================================== --- tools/llvm-dis/llvm-dis.cpp +++ tools/llvm-dis/llvm-dis.cpp @@ -27,6 +27,7 @@ #include "llvm/IR/Type.h" #include "llvm/Support/CommandLine.h" #include "llvm/Support/DataStream.h" +#include "llvm/Support/Error.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/FormattedStream.h" #include "llvm/Support/ManagedStatic.h" @@ -59,6 +60,11 @@ cl::desc("Preserve use-list order when writing LLVM assembly."), cl::init(false), cl::Hidden); +static cl::opt + MaterializeMetadata("materialize-metadata", + cl::desc("Load module without materializing metadata, " + "then materialize only the metadata")); + namespace { static void printDebugLoc(const DebugLoc &DL, formatted_raw_ostream &OS) { @@ -132,6 +138,37 @@ exit(1); } +static Expected> openInputFile(LLVMContext &Context) { + if (MaterializeMetadata) { + ErrorOr> MBOrErr = + MemoryBuffer::getFileOrSTDIN(InputFilename); + if (!MBOrErr) + return errorCodeToError(MBOrErr.getError()); + ErrorOr> MOrErr = + getLazyBitcodeModule(std::move(*MBOrErr), Context, + /*ShouldLazyLoadMetadata=*/true); + if (!MOrErr) + return errorCodeToError(MOrErr.getError()); + (*MOrErr)->materializeMetadata(); + return std::move(*MOrErr); + } else { + std::string ErrorMessage; + std::unique_ptr Streamer = + getDataFileStreamer(InputFilename, &ErrorMessage); + if (!Streamer) + return make_error(ErrorMessage, inconvertibleErrorCode()); + std::string DisplayFilename; + if (InputFilename == "-") + DisplayFilename = ""; + else + DisplayFilename = InputFilename; + ErrorOr> MOrErr = + getStreamedBitcodeModule(DisplayFilename, std::move(Streamer), Context); + (*MOrErr)->materializeAll(); + return std::move(*MOrErr); + } +} + int main(int argc, char **argv) { // Print a stack trace if we signal out. sys::PrintStackTraceOnErrorSignal(argv[0]); @@ -144,26 +181,16 @@ cl::ParseCommandLineOptions(argc, argv, "llvm .bc -> .ll disassembler\n"); - std::string ErrorMessage; - std::unique_ptr M; - - // Use the bitcode streaming interface - std::unique_ptr Streamer = - getDataFileStreamer(InputFilename, &ErrorMessage); - if (Streamer) { - std::string DisplayFilename; - if (InputFilename == "-") - DisplayFilename = ""; - else - DisplayFilename = InputFilename; - ErrorOr> MOrErr = - getStreamedBitcodeModule(DisplayFilename, std::move(Streamer), Context); - M = std::move(*MOrErr); - M->materializeAll(); - } else { - errs() << argv[0] << ": " << ErrorMessage << '\n'; + Expected> MOrErr = openInputFile(Context); + if (!MOrErr) { + handleAllErrors(MOrErr.takeError(), [&](ErrorInfoBase &EIB) { + errs() << argv[0] << ": "; + EIB.log(errs()); + errs() << '\n'; + }); return 1; } + std::unique_ptr M = std::move(*MOrErr); // Just use stdout. We won't actually print anything on it. if (DontPrint) Index: unittests/IR/MetadataTest.cpp =================================================================== --- unittests/IR/MetadataTest.cpp +++ unittests/IR/MetadataTest.cpp @@ -2260,20 +2260,20 @@ TEST_F(FunctionAttachmentTest, Verifier) { Function *F = getFunction("foo"); F->setMetadata("attach", getTuple()); + F->setIsMaterializable(true); - // Confirm this has no body. - ASSERT_TRUE(F->empty()); - - // Functions without a body cannot have metadata attachments (they also can't - // be verified directly, so check that the module fails to verify). - EXPECT_TRUE(verifyModule(*F->getParent())); + // Confirm this is materializable. + ASSERT_TRUE(F->isMaterializable()); - // Nor can materializable functions. - F->setIsMaterializable(true); - EXPECT_TRUE(verifyModule(*F->getParent())); + // Materializable functions cannot have metadata attachments. + EXPECT_TRUE(verifyFunction(*F)); - // Functions with a body can. + // Function declarations can. F->setIsMaterializable(false); + EXPECT_FALSE(verifyModule(*F->getParent())); + EXPECT_FALSE(verifyFunction(*F)); + + // So can definitions. (void)new UnreachableInst(Context, BasicBlock::Create(Context, "bb", F)); EXPECT_FALSE(verifyModule(*F->getParent())); EXPECT_FALSE(verifyFunction(*F));