Skip to content

Commit

Permalink
[ThinLTO] Import only necessary DICompileUnit fields
Browse files Browse the repository at this point in the history
Summary:
As discussed on mailing list, for ThinLTO importing we don't need
to import all the fields of the DICompileUnit. Don't import enums,
macros, retained types lists. Also only import local scoped imported
entities. Since we don't currently import any global variables,
we also don't need to import the list of global variables (added an
assert to verify none are being imported).

This is being done by pre-populating the value map entries to map
the unneeded metadata to nullptr. For the imported entities, we can
simply replace the source module's list with a new list containing
only those needed imported entities. This is done in the IRLinker
constructor so that value mapping automatically does the desired
mapping.

Reviewers: mehdi_amini, dexonsmith, dblaikie, aprantl

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D27635

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@289441 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
teresajohnson committed Dec 12, 2016
1 parent d616cee commit 23137e5
Show file tree
Hide file tree
Showing 6 changed files with 173 additions and 6 deletions.
4 changes: 3 additions & 1 deletion include/llvm/Linker/IRMover.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,11 @@ class IRMover {
/// should be linked with (concatenated into) the ModuleInlineAsm string
/// for the destination module. It should be true for full LTO, but not
/// when importing for ThinLTO, otherwise we can have duplicate symbols.
/// - \p IsPerformingImport is true when this IR link is to perform ThinLTO
/// function importing from Src.
Error move(std::unique_ptr<Module> Src, ArrayRef<GlobalValue *> ValuesToLink,
std::function<void(GlobalValue &GV, ValueAdder Add)> AddLazyFor,
bool LinkModuleInlineAsm);
bool LinkModuleInlineAsm, bool IsPerformingImport);
Module &getModule() { return Composite; }

private:
Expand Down
3 changes: 2 additions & 1 deletion lib/LTO/LTO.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,8 @@ Error LTO::addRegularLTO(std::unique_ptr<InputFile> Input,

return RegularLTO.Mover->move(Obj->takeModule(), Keep,
[](GlobalValue &, IRMover::ValueAdder) {},
/* LinkModuleInlineAsm */ true);
/* LinkModuleInlineAsm */ true,
/* IsPerformingImport */ false);
}

// Add a ThinLTO object to the link.
Expand Down
76 changes: 73 additions & 3 deletions lib/Linker/IRMover.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -480,14 +480,18 @@ class IRLinker {
Function *copyFunctionProto(const Function *SF);
GlobalValue *copyGlobalAliasProto(const GlobalAlias *SGA);

/// When importing for ThinLTO, prevent importing of types listed on
/// the DICompileUnit that we don't need a copy of in the importing
/// module.
void prepareCompileUnitsForImport();
void linkNamedMDNodes();

public:
IRLinker(Module &DstM, MDMapT &SharedMDs,
IRMover::IdentifiedStructTypeSet &Set, std::unique_ptr<Module> SrcM,
ArrayRef<GlobalValue *> ValuesToLink,
std::function<void(GlobalValue &, IRMover::ValueAdder)> AddLazyFor,
bool LinkModuleInlineAsm)
bool LinkModuleInlineAsm, bool IsPerformingImport)
: DstM(DstM), SrcM(std::move(SrcM)), AddLazyFor(std::move(AddLazyFor)),
TypeMap(Set), GValMaterializer(*this), LValMaterializer(*this),
SharedMDs(SharedMDs), LinkModuleInlineAsm(LinkModuleInlineAsm),
Expand All @@ -498,6 +502,8 @@ class IRLinker {
ValueMap.getMDMap() = std::move(SharedMDs);
for (GlobalValue *GV : ValuesToLink)
maybeAdd(GV);
if (IsPerformingImport)
prepareCompileUnitsForImport();
}
~IRLinker() { SharedMDs = std::move(*ValueMap.getMDMap()); }

Expand Down Expand Up @@ -1005,6 +1011,70 @@ Error IRLinker::linkGlobalValueBody(GlobalValue &Dst, GlobalValue &Src) {
return Error::success();
}

void IRLinker::prepareCompileUnitsForImport() {
NamedMDNode *SrcCompileUnits = SrcM->getNamedMetadata("llvm.dbg.cu");
if (!SrcCompileUnits)
return;
// When importing for ThinLTO, prevent importing of types listed on
// the DICompileUnit that we don't need a copy of in the importing
// module. They will be emitted by the originating module.
for (unsigned I = 0, E = SrcCompileUnits->getNumOperands(); I != E; ++I) {
auto *CU = cast<DICompileUnit>(SrcCompileUnits->getOperand(I));
assert(CU && "Expected valid compile unit");
// Enums, macros, and retained types don't need to be listed on the
// imported DICompileUnit. This means they will only be imported
// if reached from the mapped IR. Do this by setting their value map
// entries to nullptr, which will automatically prevent their importing
// when reached from the DICompileUnit during metadata mapping.
ValueMap.MD()[CU->getRawEnumTypes()].reset(nullptr);
ValueMap.MD()[CU->getRawMacros()].reset(nullptr);
ValueMap.MD()[CU->getRawRetainedTypes()].reset(nullptr);
// If we ever start importing global variable defs, we'll need to
// add their DIGlobalVariable to the globals list on the imported
// DICompileUnit. Confirm none are imported, and then we can
// map the list of global variables to nullptr.
assert(none_of(
ValuesToLink,
[](const GlobalValue *GV) { return isa<GlobalVariable>(GV); }) &&
"Unexpected importing of a GlobalVariable definition");
ValueMap.MD()[CU->getRawGlobalVariables()].reset(nullptr);

// Imported entities only need to be mapped in if they have local
// scope, as those might correspond to an imported entity inside a
// function being imported (any locally scoped imported entities that
// don't end up referenced by an imported function will not be emitted
// into the object). Imported entities not in a local scope
// (e.g. on the namespace) only need to be emitted by the originating
// module. Create a list of the locally scoped imported entities, and
// replace the source CUs imported entity list with the new list, so
// only those are mapped in.
// FIXME: Locally-scoped imported entities could be moved to the
// functions they are local to instead of listing them on the CU, and
// we would naturally only link in those needed by function importing.
SmallVector<TrackingMDNodeRef, 4> AllImportedModules;
bool ReplaceImportedEntities = false;
for (auto *IE : CU->getImportedEntities()) {
DIScope *Scope = IE->getScope();
assert(Scope && "Invalid Scope encoding!");
if (isa<DILocalScope>(Scope))
AllImportedModules.emplace_back(IE);
else
ReplaceImportedEntities = true;
}
if (ReplaceImportedEntities) {
if (!AllImportedModules.empty())
CU->replaceImportedEntities(MDTuple::get(
CU->getContext(),
SmallVector<Metadata *, 16>(AllImportedModules.begin(),
AllImportedModules.end())));
else
// If there were no local scope imported entities, we can map
// the whole list to nullptr.
ValueMap.MD()[CU->getRawImportedEntities()].reset(nullptr);
}
}
}

/// Insert all of the named MDNodes in Src into the Dest module.
void IRLinker::linkNamedMDNodes() {
const NamedMDNode *SrcModFlags = SrcM->getModuleFlagsMetadata();
Expand Down Expand Up @@ -1366,10 +1436,10 @@ IRMover::IRMover(Module &M) : Composite(M) {
Error IRMover::move(
std::unique_ptr<Module> Src, ArrayRef<GlobalValue *> ValuesToLink,
std::function<void(GlobalValue &, ValueAdder Add)> AddLazyFor,
bool LinkModuleInlineAsm) {
bool LinkModuleInlineAsm, bool IsPerformingImport) {
IRLinker TheIRLinker(Composite, SharedMDs, IdentifiedStructTypes,
std::move(Src), ValuesToLink, std::move(AddLazyFor),
LinkModuleInlineAsm);
LinkModuleInlineAsm, IsPerformingImport);
Error E = TheIRLinker.run();
Composite.dropTriviallyDeadConstantArrays();
return E;
Expand Down
3 changes: 2 additions & 1 deletion lib/Linker/LinkModules.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,8 @@ bool ModuleLinker::run() {
[this](GlobalValue &GV, IRMover::ValueAdder Add) {
addLazyFor(GV, Add);
},
!isPerformingImport())) {
/* LinkModuleInlineAsm */ !isPerformingImport(),
/* IsPerformingImport */ isPerformingImport())) {
handleAllErrors(std::move(E), [&](ErrorInfoBase &EIB) {
DstM.getContext().diagnose(LinkDiagnosticInfo(DS_Error, EIB.message()));
HasErrors = true;
Expand Down
13 changes: 13 additions & 0 deletions test/ThinLTO/X86/Inputs/debuginfo-cu-import.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
; ModuleID = 'debuginfo-cu-import2.c'
source_filename = "debuginfo-cu-import2.c"
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

; Function Attrs: nounwind uwtable
define i32 @main() {
entry:
call void (...) @foo()
ret i32 0
}

declare void @foo(...) #1
80 changes: 80 additions & 0 deletions test/ThinLTO/X86/debuginfo-cu-import.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
; Test to ensure only the necessary DICompileUnit fields are imported
; for ThinLTO

; RUN: opt -module-summary %s -o %t1.bc
; RUN: opt -module-summary %p/Inputs/debuginfo-cu-import.ll -o %t2.bc
; RUN: llvm-lto -thinlto-action=thinlink -o %t.index.bc %t1.bc %t2.bc

; Don't import enums, macros, retainedTypes or globals lists.
; Only import local scope imported entities.
; RUN: llvm-lto -thinlto-action=import %t2.bc -thinlto-index=%t.index.bc -o - | llvm-dis -o - | FileCheck %s
; CHECK-NOT: DICompileUnit{{.*}} enums:
; CHECK-NOT: DICompileUnit{{.*}} macros:
; CHECK-NOT: DICompileUnit{{.*}} retainedTypes:
; CHECK-NOT: DICompileUnit{{.*}} globals:
; CHECK: DICompileUnit{{.*}} imports: ![[IMP:[0-9]+]]
; CHECK: ![[IMP]] = !{!{{[0-9]+}}}

; ModuleID = 'debuginfo-cu-import.c'
source_filename = "debuginfo-cu-import.c"
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

; Function Attrs: nounwind uwtable
define void @foo() #0 !dbg !31 {
entry:
ret void, !dbg !34
}

define void @_ZN1A1aEv() #0 !dbg !4 {
entry:
ret void, !dbg !14
}

define internal void @_ZN1A1bEv() #0 !dbg !8 {
entry:
ret void, !dbg !15
}

!llvm.dbg.cu = !{!0}
!llvm.module.flags = !{!11, !12}
!llvm.ident = !{!13}

!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 4.0.0 (trunk 286863) (llvm/trunk 286875)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, macros: !26, retainedTypes: !35, globals: !37, imports: !9)
!1 = !DIFile(filename: "a2.cc", directory: "")
!2 = !{!23}
!4 = distinct !DISubprogram(name: "a", linkageName: "_ZN1A1aEv", scope: !5, file: !1, line: 7, type: !6, isLocal: false, isDefinition: true, scopeLine: 7, flags: DIFlagPrototyped, isOptimized: false, unit: !0, variables: !30)
!5 = !DINamespace(name: "A", scope: null, file: !1, line: 1)
!6 = !DISubroutineType(types: !7)
!7 = !{null}
!8 = distinct !DISubprogram(name: "b", linkageName: "_ZN1A1bEv", scope: !5, file: !1, line: 8, type: !6, isLocal: true, isDefinition: true, scopeLine: 8, flags: DIFlagPrototyped, isOptimized: false, unit: !0, variables: !30)
!9 = !{!10, !16}
!10 = !DIImportedEntity(tag: DW_TAG_imported_declaration, scope: !5, entity: !4, line: 8)
!11 = !{i32 2, !"Dwarf Version", i32 4}
!12 = !{i32 2, !"Debug Info Version", i32 3}
!13 = !{!"clang version 4.0.0 (trunk 286863) (llvm/trunk 286875)"}
!14 = !DILocation(line: 7, column: 12, scope: !4)
!15 = !DILocation(line: 8, column: 24, scope: !8)
!16 = !DIImportedEntity(tag: DW_TAG_imported_declaration, scope: !17, entity: !19, line: 8)
!17 = distinct !DILexicalBlock(scope: !18, file: !1, line: 9, column: 8)
!18 = distinct !DISubprogram(name: "c", linkageName: "_ZN1A1cEv", scope: !5, file: !1, line: 9, type: !6, isLocal: false, isDefinition: true, scopeLine: 8, flags: DIFlagPrototyped, isOptimized: false, unit: !0, variables: !30)
!19 = distinct !DILexicalBlock(scope: !20, file: !1, line: 10, column: 8)
!20 = distinct !DISubprogram(name: "d", linkageName: "_ZN1A1dEv", scope: !5, file: !1, line: 10, type: !6, isLocal: false, isDefinition: true, scopeLine: 8, flags: DIFlagPrototyped, isOptimized: false, unit: !0, variables: !30)
!21 = !DILocation(line: 9, column: 8, scope: !18)
!22 = !DILocation(line: 10, column: 8, scope: !20)
!23 = !DICompositeType(tag: DW_TAG_enumeration_type, name: "enum1", scope: !5, file: !1, line: 50, size: 32, elements: !30, identifier: "_ZTSN9__gnu_cxx12_Lock_policyE")
!24 = !{!25}
!25 = !DICompositeType(tag: DW_TAG_enumeration_type, name: "StateTag", scope: !5, file: !1, line: 1653, size: 32, elements: !30, identifier: "_ZTSN2v88StateTagE")
!26 = !{!27}
!27 = !DIMacroFile(line: 0, file: !1, nodes: !28)
!28 = !{!29}
!29 = !DIMacro(type: DW_MACINFO_define, line: 3, name: "X", value: "5")
!30 = !{}
!31 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 1, type: !32, isLocal: false, isDefinition: true, scopeLine: 2, isOptimized: false, unit: !0, variables: !30)
!32 = !DISubroutineType(types: !33)
!33 = !{null}
!34 = !DILocation(line: 3, column: 1, scope: !31)
!35 = !{!36}
!36 = !DICompositeType(tag: DW_TAG_structure_type, name: "Base", line: 1, size: 32, align: 32, file: !1, elements: !30, identifier: "_ZTS4Base")
!37 = !{!38}
!38 = !DIGlobalVariable(name: "version", scope: !5, file: !1, line: 2, type: !36, isLocal: false, isDefinition: true)

0 comments on commit 23137e5

Please sign in to comment.