Index: lib/Linker/LinkModules.cpp =================================================================== --- lib/Linker/LinkModules.cpp +++ lib/Linker/LinkModules.cpp @@ -367,15 +367,14 @@ /// few get used. class ValueMaterializerTy final : public ValueMaterializer { TypeMapTy &TypeMap; - Module *DstM; std::vector &LazilyLinkGlobalValues; ModuleLinker *ModLinker; public: - ValueMaterializerTy(TypeMapTy &TypeMap, Module *DstM, + ValueMaterializerTy(TypeMapTy &TypeMap, std::vector &LazilyLinkGlobalValues, ModuleLinker *ModLinker) - : ValueMaterializer(), TypeMap(TypeMap), DstM(DstM), + : ValueMaterializer(), TypeMap(TypeMap), LazilyLinkGlobalValues(LazilyLinkGlobalValues), ModLinker(ModLinker) {} Value *materializeValueFor(Value *V) override; @@ -449,7 +448,7 @@ FunctionInfoIndex *Index = nullptr, Function *FuncToImport = nullptr) : DstM(dstM), SrcM(srcM), TypeMap(Set), - ValMaterializer(TypeMap, DstM, LazilyLinkGlobalValues, this), + ValMaterializer(TypeMap, LazilyLinkGlobalValues, this), DiagnosticHandler(DiagnosticHandler), Flags(Flags), ImportIndex(Index), ImportFunction(FuncToImport), HasExportedFunctions(false) { assert((ImportIndex || !ImportFunction) && @@ -544,6 +543,8 @@ void linkAliasBody(GlobalAlias &Dst, GlobalAlias &Src); bool linkGlobalValueBody(GlobalValue &Src); + void copyComdat(GlobalValue &SGV); + /// Functions that take care of cloning a specific global value type /// into the destination module. GlobalVariable *copyGlobalVariableProto(TypeMapTy &TypeMap, @@ -894,13 +895,6 @@ GlobalValue *DGV = ModLinker->copyGlobalValueProto(TypeMap, SGV); - if (Comdat *SC = SGV->getComdat()) { - if (auto *DGO = dyn_cast(DGV)) { - Comdat *DC = DstM->getOrInsertComdat(SC->getName()); - DGO->setComdat(DC); - } - } - LazilyLinkGlobalValues.push_back(SGV); return DGV; } @@ -1363,14 +1357,11 @@ cast(SGV)); bool LinkFromSrc = true; - Comdat *C = nullptr; bool HasUnnamedAddr = SGV->hasUnnamedAddr(); if (const Comdat *SC = SGV->getComdat()) { Comdat::SelectionKind SK; std::tie(SK, LinkFromSrc) = ComdatsChosen[SC]; - C = DstM->getOrInsertComdat(SC->getName()); - C->setSelectionKind(SK); } else if (DGV) { if (shouldLinkFromSource(LinkFromSrc, *DGV, *SGV)) return true; @@ -1425,8 +1416,6 @@ NewGV->setUnnamedAddr(HasUnnamedAddr); if (auto *NewGO = dyn_cast(NewGV)) { - if (C) - NewGO->setComdat(C); if (DGV && DGV->hasCommonLinkage() && SGV->hasCommonLinkage()) NewGO->setAlignment(std::max(DGV->getAlignment(), SGV->getAlignment())); @@ -1588,6 +1577,27 @@ return false; } +/// For any Src comdats with an associated definition in the Dest module, +/// setup the corresponding comdat in Dest. +void ModuleLinker::copyComdat(GlobalValue &SGV) { + const Comdat *SC = SGV.getComdat(); + if (!SC) + return; + // See if there is an associated definition now in the Dest module. + // If so, make sure Dest copy is also in a comdat. + GlobalValue *DGV = DstM->getNamedValue(getName(&SGV)); + auto *GO = dyn_cast_or_null(DGV); + if (!GO || GO->isDeclaration()) + return; + Comdat::SelectionKind SK; + bool LinkFromSrc = true; + std::tie(SK, LinkFromSrc) = ComdatsChosen[SC]; + Comdat *C = DstM->getOrInsertComdat(SC->getName()); + C->setSelectionKind(SK); + assert(!GO->hasComdat() || GO->getComdat() == C); + GO->setComdat(C); +} + /// Insert all of the named MDNodes in Src into the Dest module. void ModuleLinker::linkNamedMDNodes() { const NamedMDNode *SrcModFlags = SrcM->getModuleFlagsMetadata(); @@ -1900,13 +1910,19 @@ for (const AppendingVarInfo &AppendingVar : AppendingVars) linkAppendingVarInit(AppendingVar); - for (const auto &Entry : DstM->getComdatSymbolTable()) { + + // Ensure values from comdats with specific selection critera + // are mapped, and therefore materialized/lazy linked in Dest. + for (const auto &Entry : SrcM->getComdatSymbolTable()) { const Comdat &C = Entry.getValue(); - if (C.getSelectionKind() == Comdat::Any) + bool LinkFromSrc = true; + Comdat::SelectionKind SK; + std::tie(SK, LinkFromSrc) = ComdatsChosen[&C]; + if (SK == Comdat::Any) continue; const GlobalValue *GV = SrcM->getNamedValue(C.getName()); - if (GV) - MapValue(GV, ValueMap, RF_MoveDistinctMDs, &TypeMap, &ValMaterializer); + assert(GV); + MapValue(GV, ValueMap, RF_MoveDistinctMDs, &TypeMap, &ValMaterializer); } // Strip replaced subprograms before mapping any metadata -- so that we're @@ -1977,6 +1993,19 @@ return true; } + // Now that we have linked in all needed global value bodies from Src, + // ensure that comdat information is copied for definitions in the Dest + // module. + for (GlobalVariable &SGV : SrcM->globals()) { + copyComdat(SGV); + } + for (Function &SF : *SrcM) { + copyComdat(SF); + } + for (GlobalAlias &SGA : SrcM->aliases()) { + copyComdat(SGA); + } + return false; } Index: test/Linker/funcimport.ll =================================================================== --- test/Linker/funcimport.ll +++ test/Linker/funcimport.ll @@ -12,18 +12,31 @@ ; EXPORTSTATIC: define hidden i32 @staticfunc.llvm.1 ; EXPORTSTATIC: define hidden void @staticfunc2.llvm.1 +; Ensure linking of linkonce functions and alias in a comdat works. +; RUN: llvm-link %t2.bc -functionindex=%t3.thinlto.bc -import=linkoncecomdatfunc:%t.bc -S | FileCheck %s --check-prefix=IMPORTLINKONCECOMDAT +; IMPORTLINKONCECOMDAT: @linkoncecomdatalias = alias void (...), bitcast (void ()* @linkoncecomdatfunc +; IMPORTLINKONCECOMDAT: define linkonce_odr void @linkoncecomdatfunc() comdat + +; Ensure linking of linkonce functions and alias in a comdat works as a +; second pass import. +; RUN: llvm-link %t2.bc -functionindex=%t3.thinlto.bc -import=globalfunc1:%t.bc -import=linkoncecomdatfunc:%t.bc -S | FileCheck %s --check-prefix=IMPORTLINKONCECOMDAT2 +; IMPORTLINKONCECOMDAT2: @linkoncecomdatalias = alias void (...), bitcast (void ()* @linkoncecomdatfunc +; IMPORTLINKONCECOMDAT2: define linkonce_odr void @linkoncecomdatfunc() comdat + ; Ensure that both weak alias to an imported function and strong alias to a ; non-imported function are correctly turned into declarations. -; Also ensures that alias to a linkonce function is turned into a declaration -; and that the associated linkonce function is not in the output, as it is -; lazily linked and never referenced/materialized. +; Also ensures that aliases to linkonce functions are turned into non-comdat +; declarations and that the associated linkonce functions are not in the output, +; as they are lazily linked and never referenced/materialized. ; RUN: llvm-link %t2.bc -functionindex=%t3.thinlto.bc -import=globalfunc1:%t.bc -S | FileCheck %s --check-prefix=IMPORTGLOB1 ; IMPORTGLOB1: define available_externally void @globalfunc1 ; IMPORTGLOB1: declare void @globalfunc2 ; IMPORTGLOB1: declare extern_weak void @weakalias ; IMPORTGLOB1: declare void @analias ; IMPORTGLOB1: declare void @linkoncealias +; IMPORTGLOB1: declare void @linkoncecomdatalias ; IMPORTGLOB1-NOT: @linkoncefunc +; IMPORTGLOB1-NOT: @linkoncecomdatfunc ; Ensure that weak alias to a non-imported function is correctly ; turned into a declaration, but that strong alias to an imported function @@ -97,6 +110,7 @@ @weakalias = weak alias void (...), bitcast (void ()* @globalfunc1 to void (...)*) @analias = alias void (...), bitcast (void ()* @globalfunc2 to void (...)*) @linkoncealias = alias void (...), bitcast (void ()* @linkoncefunc to void (...)*) +@linkoncecomdatalias = alias void (...), bitcast (void ()* @linkoncecomdatfunc to void (...)*) define void @globalfunc1() #0 { entry: @@ -113,6 +127,12 @@ ret void } +$linkoncecomdatfunc = comdat any +define linkonce_odr void @linkoncecomdatfunc() #0 comdat { +entry: + ret void +} + define i32 @referencestatics(i32 %i) #0 { entry: %i.addr = alloca i32, align 4