From e8df234a6ea0dbc20e4da677f2f9f8c426f44246 Mon Sep 17 00:00:00 2001 From: Teresa Johnson Date: Fri, 18 Dec 2015 17:51:37 +0000 Subject: [PATCH] [ThinLTO/LTO] Don't link in unneeded metadata Summary: Third patch split out from http://reviews.llvm.org/D14752. Only map in needed DISubroutine metadata (imported or otherwise linked in functions and other DISubroutine referenced by inlined instructions). This is supported for ThinLTO, LTO and llvm-link --only-needed, with associated tests for each one. Depends on D14838. Reviewers: dexonsmith, joker.eph Subscribers: davidxl, llvm-commits, joker.eph Differential Revision: http://reviews.llvm.org/D14843 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@256003 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Transforms/Utils/ValueMapper.h | 5 + lib/Linker/IRMover.cpp | 107 ++++++++++++++++++ lib/Transforms/Utils/ValueMapper.cpp | 4 + .../Inputs/only-needed-debug-metadata.ll | 27 +++++ test/Linker/only-needed-debug-metadata.ll | 49 ++++++++ test/Linker/thinlto_funcimport_debug.ll | 13 ++- test/tools/gold/X86/Inputs/linkonce-weak.ll | 20 +++- test/tools/gold/X86/linkonce-weak.ll | 26 ++++- 8 files changed, 244 insertions(+), 7 deletions(-) create mode 100644 test/Linker/Inputs/only-needed-debug-metadata.ll create mode 100644 test/Linker/only-needed-debug-metadata.ll diff --git a/include/llvm/Transforms/Utils/ValueMapper.h b/include/llvm/Transforms/Utils/ValueMapper.h index 7b1382854ba..469022f34c5 100644 --- a/include/llvm/Transforms/Utils/ValueMapper.h +++ b/include/llvm/Transforms/Utils/ValueMapper.h @@ -61,6 +61,11 @@ namespace llvm { virtual Metadata *mapTemporaryMetadata(Metadata *MD) { return nullptr; } virtual void replaceTemporaryMetadata(const Metadata *OrigMD, Metadata *NewMD) {} + + /// The client should implement this method if some metadata need + /// not be mapped, for example DISubprogram metadata for functions not + /// linked into the destination module. + virtual bool isMetadataNeeded(Metadata *MD) { return true; } }; /// RemapFlags - These are flags that the value mapping APIs allow. diff --git a/lib/Linker/IRMover.cpp b/lib/Linker/IRMover.cpp index 01a0a425d8d..5581eaf097e 100644 --- a/lib/Linker/IRMover.cpp +++ b/lib/Linker/IRMover.cpp @@ -13,6 +13,7 @@ #include "llvm/ADT/SmallString.h" #include "llvm/ADT/Triple.h" #include "llvm/IR/Constants.h" +#include "llvm/IR/DebugInfo.h" #include "llvm/IR/DiagnosticPrinter.h" #include "llvm/IR/GVMaterializer.h" #include "llvm/IR/TypeFinder.h" @@ -353,6 +354,7 @@ public: Metadata *mapTemporaryMetadata(Metadata *MD) override; void replaceTemporaryMetadata(const Metadata *OrigMD, Metadata *NewMD) override; + bool isMetadataNeeded(Metadata *MD) override; }; class LocalValueMaterializer final : public ValueMaterializer { @@ -365,6 +367,7 @@ public: Metadata *mapTemporaryMetadata(Metadata *MD) override; void replaceTemporaryMetadata(const Metadata *OrigMD, Metadata *NewMD) override; + bool isMetadataNeeded(Metadata *MD) override; }; /// This is responsible for keeping track of the state used for moving data @@ -419,6 +422,11 @@ class IRLinker { /// importing and consumed during the metadata linking postpass. DenseMap *ValIDToTempMDMap; + /// Set of subprogram metadata that does not need to be linked into the + /// destination module, because the functions were not imported directly + /// or via an inlined body in an imported function. + SmallPtrSet UnneededSubprograms; + /// Handles cloning of a global values from the source module into /// the destination module, including setting the attributes and visibility. GlobalValue *copyGlobalValueProto(const GlobalValue *SGV, bool ForDefinition); @@ -487,6 +495,16 @@ class IRLinker { void linkNamedMDNodes(); + /// Populate the UnneededSubprograms set with the DISubprogram metadata + /// from the source module that we don't need to link into the dest module, + /// because the functions were not imported directly or via an inlined body + /// in an imported function. + void findNeededSubprograms(ValueToValueMapTy &ValueMap); + + /// The value mapper leaves nulls in the list of subprograms for any + /// in the UnneededSubprograms map. Strip those out after metadata linking. + void stripNullSubprograms(); + public: IRLinker(Module &DstM, IRMover::IdentifiedStructTypeSet &Set, Module &SrcM, ArrayRef ValuesToLink, @@ -519,6 +537,11 @@ public: /// the new non-temporary metadata. Used when metadata linking as a postpass /// for function importing. void replaceTemporaryMetadata(const Metadata *OrigMD, Metadata *NewMD); + + /// Indicates whether we need to map the given metadata into the destination + /// module. Used to prevent linking of metadata only needed by functions not + /// linked into the dest module. + bool isMetadataNeeded(Metadata *MD); }; } @@ -561,6 +584,10 @@ void GlobalValueMaterializer::replaceTemporaryMetadata(const Metadata *OrigMD, ModLinker->replaceTemporaryMetadata(OrigMD, NewMD); } +bool GlobalValueMaterializer::isMetadataNeeded(Metadata *MD) { + return ModLinker->isMetadataNeeded(MD); +} + Value *LocalValueMaterializer::materializeDeclFor(Value *V) { return ModLinker->materializeDeclFor(V, true); } @@ -579,6 +606,10 @@ void LocalValueMaterializer::replaceTemporaryMetadata(const Metadata *OrigMD, ModLinker->replaceTemporaryMetadata(OrigMD, NewMD); } +bool LocalValueMaterializer::isMetadataNeeded(Metadata *MD) { + return ModLinker->isMetadataNeeded(MD); +} + Value *IRLinker::materializeDeclFor(Value *V, bool ForAlias) { auto *SGV = dyn_cast(V); if (!SGV) @@ -651,6 +682,19 @@ void IRLinker::replaceTemporaryMetadata(const Metadata *OrigMD, } } +bool IRLinker::isMetadataNeeded(Metadata *MD) { + // Currently only DISubprogram metadata is marked as being unneeded. + if (UnneededSubprograms.empty()) + return true; + MDNode *Node = dyn_cast(MD); + if (!Node) + return true; + DISubprogram *SP = getDISubprogram(Node); + if (!SP) + return true; + return !UnneededSubprograms.count(SP); +} + /// Loop through the global variables in the src module and merge them into the /// dest module. GlobalVariable *IRLinker::copyGlobalVariableProto(const GlobalVariable *SGVar) { @@ -1141,8 +1185,70 @@ bool IRLinker::linkGlobalValueBody(GlobalValue &Dst, GlobalValue &Src) { return false; } +void IRLinker::findNeededSubprograms(ValueToValueMapTy &ValueMap) { + // Track unneeded nodes to make it simpler to handle the case + // where we are checking if an already-mapped SP is needed. + NamedMDNode *CompileUnits = SrcM.getNamedMetadata("llvm.dbg.cu"); + if (!CompileUnits) + return; + for (unsigned I = 0, E = CompileUnits->getNumOperands(); I != E; ++I) { + auto *CU = cast(CompileUnits->getOperand(I)); + assert(CU && "Expected valid compile unit"); + for (const Metadata *Op : CU->getSubprograms()->operands()) { + // Unless we were doing function importing and deferred metadata linking, + // any needed SPs should have been mapped as they would be reached + // from the function linked in (either on the function itself for linked + // function bodies, or from DILocation on inlined instructions). + assert(!(ValueMap.MD()[Op] && IsMetadataLinkingPostpass) && + "DISubprogram shouldn't be mapped yet"); + if (!ValueMap.MD()[Op]) + UnneededSubprograms.insert(Op); + } + } + if (!IsMetadataLinkingPostpass) + return; + // In the case of metadata linking as a postpass (e.g. for function + // importing), see which DISubprogram MD from the source has an associated + // temporary metadata node, which means the SP was needed by an imported + // function. + for (auto MDI : MDValueToValIDMap) { + const MDNode *Node = dyn_cast(MDI.first); + if (!Node) + continue; + DISubprogram *SP = getDISubprogram(Node); + if (!SP || !ValIDToTempMDMap->count(MDI.second)) + continue; + UnneededSubprograms.erase(SP); + } +} + +// Squash null subprograms from compile unit subprogram lists. +void IRLinker::stripNullSubprograms() { + NamedMDNode *CompileUnits = DstM.getNamedMetadata("llvm.dbg.cu"); + if (!CompileUnits) + return; + for (unsigned I = 0, E = CompileUnits->getNumOperands(); I != E; ++I) { + auto *CU = cast(CompileUnits->getOperand(I)); + assert(CU && "Expected valid compile unit"); + + SmallVector NewSPs; + NewSPs.reserve(CU->getSubprograms().size()); + bool FoundNull = false; + for (DISubprogram *SP : CU->getSubprograms()) { + if (!SP) { + FoundNull = true; + continue; + } + NewSPs.push_back(SP); + } + if (FoundNull) + CU->replaceSubprograms(MDTuple::get(CU->getContext(), NewSPs)); + } +} + /// Insert all of the named MDNodes in Src into the Dest module. void IRLinker::linkNamedMDNodes() { + findNeededSubprograms(ValueMap); const NamedMDNode *SrcModFlags = SrcM.getModuleFlagsMetadata(); for (const NamedMDNode &NMD : SrcM.named_metadata()) { // Don't link module flags here. Do them separately. @@ -1155,6 +1261,7 @@ void IRLinker::linkNamedMDNodes() { op, ValueMap, ValueMapperFlags | RF_NullMapMissingGlobalValues, &TypeMap, &GValMaterializer)); } + stripNullSubprograms(); } /// Merge the linker flags in Src into the Dest module. diff --git a/lib/Transforms/Utils/ValueMapper.cpp b/lib/Transforms/Utils/ValueMapper.cpp index 00ee3385981..1add78e0165 100644 --- a/lib/Transforms/Utils/ValueMapper.cpp +++ b/lib/Transforms/Utils/ValueMapper.cpp @@ -197,6 +197,10 @@ static Metadata *mapMetadataOp(Metadata *Op, ValueMaterializer *Materializer) { if (!Op) return nullptr; + + if (Materializer && !Materializer->isMetadataNeeded(Op)) + return nullptr; + if (Metadata *MappedOp = MapMetadataImpl(Op, DistinctWorklist, VM, Flags, TypeMapper, Materializer)) return MappedOp; diff --git a/test/Linker/Inputs/only-needed-debug-metadata.ll b/test/Linker/Inputs/only-needed-debug-metadata.ll new file mode 100644 index 00000000000..ec7f02f4d19 --- /dev/null +++ b/test/Linker/Inputs/only-needed-debug-metadata.ll @@ -0,0 +1,27 @@ +@X = external global i32 + +declare i32 @foo() + +define void @bar() !dbg !4 { + load i32, i32* @X, !dbg !10 + call i32 @foo(), !dbg !11 + ret void, !dbg !12 +} + +!llvm.dbg.cu = !{!0} +!llvm.module.flags = !{!7, !8} +!llvm.ident = !{!9} + +!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 3.8.0 (trunk 251407) (llvm/trunk 251401)", isOptimized: true, runtimeVersion: 0, emissionKind: 1, enums: !2, subprograms: !3) +!1 = !DIFile(filename: "linkused.b.c", directory: ".") +!2 = !{} +!3 = !{!4} +!4 = distinct !DISubprogram(name: "bar", scope: !1, file: !1, line: 5, type: !5, isLocal: false, isDefinition: true, scopeLine: 5, isOptimized: true, variables: !2) +!5 = !DISubroutineType(types: !6) +!6 = !{null} +!7 = !{i32 2, !"Dwarf Version", i32 4} +!8 = !{i32 2, !"Debug Info Version", i32 3} +!9 = !{!"clang version 3.8.0 (trunk 251407) (llvm/trunk 251401)"} +!10 = !DILocation(line: 6, column: 7, scope: !4) +!11 = !DILocation(line: 6, column: 3, scope: !4) +!12 = !DILocation(line: 7, column: 1, scope: !4) diff --git a/test/Linker/only-needed-debug-metadata.ll b/test/Linker/only-needed-debug-metadata.ll new file mode 100644 index 00000000000..f327fe03bf4 --- /dev/null +++ b/test/Linker/only-needed-debug-metadata.ll @@ -0,0 +1,49 @@ +; RUN: llvm-as %s -o %t.bc +; RUN: llvm-as %p/Inputs/only-needed-debug-metadata.ll -o %t2.bc + +; Without -only-needed, we need to link in both DISubprogram. +; RUN: llvm-link -S %t2.bc %t.bc | FileCheck %s +; CHECK: distinct !DISubprogram(name: "foo" +; CHECK: distinct !DISubprogram(name: "unused" + +; With -only-needed, we only need to link in foo's DISubprogram. +; RUN: llvm-link -S -only-needed %t2.bc %t.bc | FileCheck %s -check-prefix=ONLYNEEDED +; ONLYNEEDED: distinct !DISubprogram(name: "foo" +; ONLYNEEDED-NOT: distinct !DISubprogram(name: "unused" + +@X = global i32 5 +@U = global i32 6 +@U_linkonce = linkonce_odr hidden global i32 6 +define i32 @foo() !dbg !4 { + ret i32 7, !dbg !20 +} +define i32 @unused() !dbg !10 { + ret i32 8, !dbg !21 +} + +!llvm.dbg.cu = !{!0} +!llvm.module.flags = !{!16, !17} +!llvm.ident = !{!18} + +!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 3.8.0 (trunk 251407) (llvm/trunk 251401)", isOptimized: true, runtimeVersion: 0, emissionKind: 1, enums: !2, subprograms: !3, globals: !13) +!1 = !DIFile(filename: "linkused2.c", directory: "/usr/local/google/home/tejohnson/llvm/tmp") +!2 = !{} +!3 = !{!4, !10} +!4 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 4, type: !5, isLocal: false, isDefinition: true, scopeLine: 4, flags: DIFlagPrototyped, isOptimized: true, variables: !8) +!5 = !DISubroutineType(types: !6) +!6 = !{!7, !7} +!7 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed) +!8 = !{!9} +!9 = !DILocalVariable(name: "x", arg: 1, scope: !4, file: !1, line: 4, type: !7) +!10 = distinct !DISubprogram(name: "unused", scope: !1, file: !1, line: 8, type: !11, isLocal: false, isDefinition: true, scopeLine: 8, isOptimized: true, variables: !2) +!11 = !DISubroutineType(types: !12) +!12 = !{!7} +!13 = !{!14, !15} +!14 = !DIGlobalVariable(name: "X", scope: !0, file: !1, line: 1, type: !7, isLocal: false, isDefinition: true, variable: i32* @X) +!15 = !DIGlobalVariable(name: "U", scope: !0, file: !1, line: 2, type: !7, isLocal: false, isDefinition: true, variable: i32* @U) +!16 = !{i32 2, !"Dwarf Version", i32 4} +!17 = !{i32 2, !"Debug Info Version", i32 3} +!18 = !{!"clang version 3.8.0 (trunk 251407) (llvm/trunk 251401)"} +!19 = !DIExpression() +!20 = !DILocation(line: 4, column: 13, scope: !4) +!21 = !DILocation(line: 9, column: 3, scope: !10) diff --git a/test/Linker/thinlto_funcimport_debug.ll b/test/Linker/thinlto_funcimport_debug.ll index 6c33caacf9e..02f43b24c17 100644 --- a/test/Linker/thinlto_funcimport_debug.ll +++ b/test/Linker/thinlto_funcimport_debug.ll @@ -3,13 +3,22 @@ ; RUN: llvm-as -function-summary %p/Inputs/thinlto_funcimport_debug.ll -o %t2.bc ; RUN: llvm-lto -thinlto -o %t3 %t.bc %t2.bc -; Confirm that we link the metadata for the imported module. +; If we import func1 and not func2 we should only link DISubprogram for func1 ; RUN: llvm-link %t2.bc -functionindex=%t3.thinlto.bc -import=func1:%t.bc -S | FileCheck %s ; CHECK: declare i32 @func2 ; CHECK: define available_externally i32 @func1 + +; Extract out the list of subprograms from each compile unit and ensure +; that neither contains null. +; CHECK: !{{[0-9]+}} = distinct !DICompileUnit({{.*}} subprograms: ![[SPs1:[0-9]+]] +; CHECK-NOT: ![[SPs1]] = !{{{.*}}null{{.*}}} +; CHECK: !{{[0-9]+}} = distinct !DICompileUnit({{.*}} subprograms: ![[SPs2:[0-9]+]] +; CHECK-NOT: ![[SPs2]] = !{{{.*}}null{{.*}}} + ; CHECK: distinct !DISubprogram(name: "func1" -; CHECK: distinct !DISubprogram(name: "func2" +; CHECK-NOT: distinct !DISubprogram(name: "func2" + ; ModuleID = 'dbg.o' target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" diff --git a/test/tools/gold/X86/Inputs/linkonce-weak.ll b/test/tools/gold/X86/Inputs/linkonce-weak.ll index f42af8faa84..3b7dad1b1ef 100644 --- a/test/tools/gold/X86/Inputs/linkonce-weak.ll +++ b/test/tools/gold/X86/Inputs/linkonce-weak.ll @@ -1,3 +1,19 @@ -define weak_odr void @f() { - ret void +define weak_odr void @f() !dbg !4 { + ret void, !dbg !10 } + +!llvm.dbg.cu = !{!0} +!llvm.module.flags = !{!7, !8} +!llvm.ident = !{!9} + +!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 3.8.0 (trunk 251407) (llvm/trunk 251401)", isOptimized: true, runtimeVersion: 0, emissionKind: 1, enums: !2, subprograms: !3) +!1 = !DIFile(filename: "linkonce-weak.c", directory: ".") +!2 = !{} +!3 = !{!4} +!4 = distinct !DISubprogram(name: "f", scope: !1, file: !1, line: 1, type: !5, isLocal: false, isDefinition: true, scopeLine: 1, isOptimized: true, variables: !2) +!5 = !DISubroutineType(types: !6) +!6 = !{null} +!7 = !{i32 2, !"Dwarf Version", i32 4} +!8 = !{i32 2, !"Debug Info Version", i32 3} +!9 = !{!"clang version 3.8.0 (trunk 251407) (llvm/trunk 251401)"} +!10 = !DILocation(line: 2, column: 1, scope: !4) diff --git a/test/tools/gold/X86/linkonce-weak.ll b/test/tools/gold/X86/linkonce-weak.ll index a0cccea56cf..3397c3480a7 100644 --- a/test/tools/gold/X86/linkonce-weak.ll +++ b/test/tools/gold/X86/linkonce-weak.ll @@ -11,9 +11,29 @@ ; RUN: -shared %t2.o %t.o -o %t3.o ; RUN: llvm-dis %t3.o -o - | FileCheck %s -define linkonce_odr void @f() { - ret void +define linkonce_odr void @f() !dbg !4 { + ret void, !dbg !10 } ; Test that we get a weak_odr regardless of the order of the files -; CHECK: define weak_odr void @f() { +; CHECK: define weak_odr void @f() + +; Test that we only get a single DISubprogram for @f +; CHECK: !DISubprogram(name: "f" +; CHECK-NOT: !DISubprogram(name: "f" + +!llvm.dbg.cu = !{!0} +!llvm.module.flags = !{!7, !8} +!llvm.ident = !{!9} + +!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 3.8.0 (trunk 251407) (llvm/trunk 251401)", isOptimized: true, runtimeVersion: 0, emissionKind: 1, enums: !2, subprograms: !3) +!1 = !DIFile(filename: "linkonce-weak.c", directory: ".") +!2 = !{} +!3 = !{!4} +!4 = distinct !DISubprogram(name: "f", scope: !1, file: !1, line: 1, type: !5, isLocal: false, isDefinition: true, scopeLine: 1, isOptimized: true, variables: !2) +!5 = !DISubroutineType(types: !6) +!6 = !{null} +!7 = !{i32 2, !"Dwarf Version", i32 4} +!8 = !{i32 2, !"Debug Info Version", i32 3} +!9 = !{!"clang version 3.8.0 (trunk 251407) (llvm/trunk 251401)"} +!10 = !DILocation(line: 2, column: 1, scope: !4)