[llvm] r284030 - [ThinLTO] Don't link module level assembly when importing
Teresa Johnson via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 12 11:39:29 PDT 2016
Author: tejohnson
Date: Wed Oct 12 13:39:29 2016
New Revision: 284030
URL: http://llvm.org/viewvc/llvm-project?rev=284030&view=rev
Log:
[ThinLTO] Don't link module level assembly when importing
Module inline asm was always being linked/concatenated
when running the IRLinker. This is correct for full LTO but not when
we are importing for ThinLTO, as it can result in multiply defined
symbols when the module asm defines a global symbol.
In order to test with llvm-lto2, I had to work around PR30396,
where a symbol that is defined in module assembly but defined in the
LLVM IR appears twice. Added workaround to llvm-lto2 with a FIXME.
Fixes PR30610.
Reviewers: mehdi_amini
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D25359
Added:
llvm/trunk/test/ThinLTO/X86/Inputs/module_asm.ll
llvm/trunk/test/ThinLTO/X86/module_asm_glob.ll
Modified:
llvm/trunk/include/llvm/Linker/IRMover.h
llvm/trunk/lib/LTO/LTO.cpp
llvm/trunk/lib/Linker/IRMover.cpp
llvm/trunk/lib/Linker/LinkModules.cpp
llvm/trunk/tools/llvm-lto2/llvm-lto2.cpp
Modified: llvm/trunk/include/llvm/Linker/IRMover.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Linker/IRMover.h?rev=284030&r1=284029&r2=284030&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Linker/IRMover.h (original)
+++ llvm/trunk/include/llvm/Linker/IRMover.h Wed Oct 12 13:39:29 2016
@@ -71,8 +71,13 @@ public:
/// not present in ValuesToLink. The GlobalValue and a ValueAdder callback
/// are passed as an argument, and the callback is expected to be called
/// if the GlobalValue needs to be added to the \p ValuesToLink and linked.
+ /// - \p LinkModuleInlineAsm is true if the ModuleInlineAsm string in Src
+ /// 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.
Error move(std::unique_ptr<Module> Src, ArrayRef<GlobalValue *> ValuesToLink,
- std::function<void(GlobalValue &GV, ValueAdder Add)> AddLazyFor);
+ std::function<void(GlobalValue &GV, ValueAdder Add)> AddLazyFor,
+ bool LinkModuleInlineAsm);
Module &getModule() { return Composite; }
private:
Modified: llvm/trunk/lib/LTO/LTO.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/LTO/LTO.cpp?rev=284030&r1=284029&r2=284030&view=diff
==============================================================================
--- llvm/trunk/lib/LTO/LTO.cpp (original)
+++ llvm/trunk/lib/LTO/LTO.cpp Wed Oct 12 13:39:29 2016
@@ -367,7 +367,8 @@ Error LTO::addRegularLTO(std::unique_ptr
assert(ResI == Res.end());
return RegularLTO.Mover->move(Obj->takeModule(), Keep,
- [](GlobalValue &, IRMover::ValueAdder) {});
+ [](GlobalValue &, IRMover::ValueAdder) {},
+ /* LinkModuleInlineAsm */ true);
}
// Add a ThinLTO object to the link.
Modified: llvm/trunk/lib/Linker/IRMover.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/IRMover.cpp?rev=284030&r1=284029&r2=284030&view=diff
==============================================================================
--- llvm/trunk/lib/Linker/IRMover.cpp (original)
+++ llvm/trunk/lib/Linker/IRMover.cpp Wed Oct 12 13:39:29 2016
@@ -397,6 +397,12 @@ class IRLinker {
Worklist.push_back(GV);
}
+ /// Flag whether the ModuleInlineAsm string in Src 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.
+ bool LinkModuleInlineAsm;
+
/// Set to true when all global value body linking is complete (including
/// lazy linking). Used to prevent metadata linking from creating new
/// references.
@@ -482,10 +488,11 @@ public:
IRLinker(Module &DstM, MDMapT &SharedMDs,
IRMover::IdentifiedStructTypeSet &Set, std::unique_ptr<Module> SrcM,
ArrayRef<GlobalValue *> ValuesToLink,
- std::function<void(GlobalValue &, IRMover::ValueAdder)> AddLazyFor)
+ std::function<void(GlobalValue &, IRMover::ValueAdder)> AddLazyFor,
+ bool LinkModuleInlineAsm)
: DstM(DstM), SrcM(std::move(SrcM)), AddLazyFor(std::move(AddLazyFor)),
TypeMap(Set), GValMaterializer(*this), LValMaterializer(*this),
- SharedMDs(SharedMDs),
+ SharedMDs(SharedMDs), LinkModuleInlineAsm(LinkModuleInlineAsm),
Mapper(ValueMap, RF_MoveDistinctMDs | RF_IgnoreMissingLocals, &TypeMap,
&GValMaterializer),
AliasMCID(Mapper.registerAlternateMappingContext(AliasValueMap,
@@ -1216,7 +1223,7 @@ Error IRLinker::run() {
DstM.setTargetTriple(mergeTriples(SrcTriple, DstTriple));
// Append the module inline asm string.
- if (!SrcM->getModuleInlineAsm().empty()) {
+ if (LinkModuleInlineAsm && !SrcM->getModuleInlineAsm().empty()) {
if (DstM.getModuleInlineAsm().empty())
DstM.setModuleInlineAsm(SrcM->getModuleInlineAsm());
else
@@ -1354,9 +1361,11 @@ IRMover::IRMover(Module &M) : Composite(
Error IRMover::move(
std::unique_ptr<Module> Src, ArrayRef<GlobalValue *> ValuesToLink,
- std::function<void(GlobalValue &, ValueAdder Add)> AddLazyFor) {
+ std::function<void(GlobalValue &, ValueAdder Add)> AddLazyFor,
+ bool LinkModuleInlineAsm) {
IRLinker TheIRLinker(Composite, SharedMDs, IdentifiedStructTypes,
- std::move(Src), ValuesToLink, std::move(AddLazyFor));
+ std::move(Src), ValuesToLink, std::move(AddLazyFor),
+ LinkModuleInlineAsm);
Error E = TheIRLinker.run();
Composite.dropTriviallyDeadConstantArrays();
return E;
Modified: llvm/trunk/lib/Linker/LinkModules.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/LinkModules.cpp?rev=284030&r1=284029&r2=284030&view=diff
==============================================================================
--- llvm/trunk/lib/Linker/LinkModules.cpp (original)
+++ llvm/trunk/lib/Linker/LinkModules.cpp Wed Oct 12 13:39:29 2016
@@ -582,7 +582,8 @@ bool ModuleLinker::run() {
if (Error E = Mover.move(std::move(SrcM), ValuesToLink.getArrayRef(),
[this](GlobalValue &GV, IRMover::ValueAdder Add) {
addLazyFor(GV, Add);
- })) {
+ },
+ !isPerformingImport())) {
handleAllErrors(std::move(E), [&](ErrorInfoBase &EIB) {
DstM.getContext().diagnose(LinkDiagnosticInfo(DS_Error, EIB.message()));
HasErrors = true;
Added: llvm/trunk/test/ThinLTO/X86/Inputs/module_asm.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/Inputs/module_asm.ll?rev=284030&view=auto
==============================================================================
--- llvm/trunk/test/ThinLTO/X86/Inputs/module_asm.ll (added)
+++ llvm/trunk/test/ThinLTO/X86/Inputs/module_asm.ll Wed Oct 12 13:39:29 2016
@@ -0,0 +1,8 @@
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @main({ i64, { i64, i8* }* } %unnamed) #0 {
+ %1 = call i32 @_simplefunction() #1
+ ret i32 %1
+}
+declare i32 @_simplefunction() #1
Added: llvm/trunk/test/ThinLTO/X86/module_asm_glob.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/module_asm_glob.ll?rev=284030&view=auto
==============================================================================
--- llvm/trunk/test/ThinLTO/X86/module_asm_glob.ll (added)
+++ llvm/trunk/test/ThinLTO/X86/module_asm_glob.ll Wed Oct 12 13:39:29 2016
@@ -0,0 +1,35 @@
+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %p/Inputs/module_asm.ll -o %t2.bc
+
+; RUN: llvm-lto -thinlto-action=run -exported-symbol=main %t1.bc %t2.bc
+; RUN: llvm-nm %t1.bc.thinlto.o | FileCheck %s --check-prefix=NM0
+; RUN: llvm-nm %t2.bc.thinlto.o | FileCheck %s --check-prefix=NM1
+
+; RUN: llvm-lto2 %t1.bc %t2.bc -o %t.o -save-temps \
+; RUN: -r=%t1.bc,foo,plx \
+; RUN: -r=%t1.bc,_simplefunction,pl \
+; RUN: -r=%t2.bc,main,plx \
+; RUN: -r=%t2.bc,_simplefunction,l
+; RUN: llvm-nm %t.o.0 | FileCheck %s --check-prefix=NM0
+; RUN: llvm-nm %t.o.1 | FileCheck %s --check-prefix=NM1
+
+; NM0: T foo
+; NM1-NOT: foo
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+module asm "\09.text"
+module asm "\09.globl\09foo"
+module asm "\09.align\0916, 0x90"
+module asm "\09.type\09foo, at function"
+module asm "foo:"
+module asm "\09ret "
+module asm "\09.size\09foo, .-foo"
+module asm ""
+
+declare zeroext i16 @foo() #0
+
+define i32 @_simplefunction() #1 {
+ ret i32 1
+}
Modified: llvm/trunk/tools/llvm-lto2/llvm-lto2.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-lto2/llvm-lto2.cpp?rev=284030&r1=284029&r2=284030&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-lto2/llvm-lto2.cpp (original)
+++ llvm/trunk/tools/llvm-lto2/llvm-lto2.cpp Wed Oct 12 13:39:29 2016
@@ -158,6 +158,12 @@ int main(int argc, char **argv) {
check(InputFile::create(MB->getMemBufferRef()), F);
std::vector<SymbolResolution> Res;
+ // FIXME: Workaround PR30396 which means that a symbol can appear
+ // more than once if it is defined in module-level assembly and
+ // has a GV declaration. Keep track of the resolutions found in this
+ // file and remove them from the CommandLineResolutions map afterwards,
+ // so that we don't flag the second one as missing.
+ std::map<std::string, SymbolResolution> CurrentFileSymResolutions;
for (const InputFile::Symbol &Sym : Input->symbols()) {
auto I = CommandLineResolutions.find({F, Sym.getName()});
if (I == CommandLineResolutions.end()) {
@@ -166,9 +172,13 @@ int main(int argc, char **argv) {
HasErrors = true;
} else {
Res.push_back(I->second);
- CommandLineResolutions.erase(I);
+ CurrentFileSymResolutions[Sym.getName()] = I->second;
}
}
+ for (auto I : CurrentFileSymResolutions) {
+ auto NumErased = CommandLineResolutions.erase({F, I.first});
+ assert(NumErased > 0);
+ }
if (HasErrors)
continue;
More information about the llvm-commits
mailing list