Index: include/llvm/Linker/Linker.h =================================================================== --- include/llvm/Linker/Linker.h +++ include/llvm/Linker/Linker.h @@ -10,7 +10,6 @@ #ifndef LLVM_LINKER_LINKER_H #define LLVM_LINKER_LINKER_H -#include "llvm/IR/ModuleSummaryIndex.h" #include "llvm/Linker/IRMover.h" namespace llvm { @@ -40,15 +39,14 @@ /// Passing OverrideSymbols as true will have symbols from Src /// shadow those in the Dest. /// For ThinLTO function importing/exporting the \p ModuleSummaryIndex - /// is passed. If \p FunctionsToImport is provided, only the functions that + /// is passed. If \p GlobalsToImport is provided, only the globals that /// are part of the set will be imported from the source module. /// The \p ValIDToTempMDMap is populated by the linker when function /// importing is performed. /// /// Returns true on error. bool linkInModule(std::unique_ptr Src, unsigned Flags = Flags::None, - const ModuleSummaryIndex *Index = nullptr, - DenseSet *FunctionsToImport = nullptr, + DenseSet *GlobalsToImport = nullptr, DenseMap *ValIDToTempMDMap = nullptr); static bool linkModules(Module &Dest, std::unique_ptr Src, Index: include/llvm/Transforms/Utils/FunctionImportUtils.h =================================================================== --- include/llvm/Transforms/Utils/FunctionImportUtils.h +++ include/llvm/Transforms/Utils/FunctionImportUtils.h @@ -30,9 +30,9 @@ /// Module summary index passed in for function importing/exporting handling. const ModuleSummaryIndex &ImportIndex; - /// Functions to import from this module, all other functions will be + /// Globals to import from this module, all other functions will be /// imported as declarations instead of definitions. - DenseSet *FunctionsToImport; + DenseSet *GlobalsToImport; /// Set to true if the given ModuleSummaryIndex contains any functions /// from this source module, in which case we must conservatively assume @@ -40,17 +40,12 @@ /// as part of a different backend compilation process. bool HasExportedFunctions = false; - /// Populated during ThinLTO global processing with locals promoted - /// to global scope in an exporting module, which now need to be linked - /// in if calling from the ModuleLinker. - SetVector NewExportedValues; - /// Check if we should promote the given local value to global scope. bool doPromoteLocalToGlobal(const GlobalValue *SGV); /// Helper methods to check if we are importing from or potentially /// exporting from the current source module. - bool isPerformingImport() const { return FunctionsToImport != nullptr; } + bool isPerformingImport() const { return GlobalsToImport != nullptr; } bool isModuleExporting() const { return HasExportedFunctions; } /// If we are importing from the source module, checks if we should @@ -77,13 +72,13 @@ public: FunctionImportGlobalProcessing( Module &M, const ModuleSummaryIndex &Index, - DenseSet *FunctionsToImport = nullptr) - : M(M), ImportIndex(Index), FunctionsToImport(FunctionsToImport) { + DenseSet *GlobalsToImport = nullptr) + : M(M), ImportIndex(Index), GlobalsToImport(GlobalsToImport) { // If we have a ModuleSummaryIndex but no function to import, // then this is the primary module being compiled in a ThinLTO // backend compilation, and we need to see if it has functions that // may be exported to another backend compilation. - if (!FunctionsToImport) + if (!GlobalsToImport) HasExportedFunctions = ImportIndex.hasExportedFunctions(M); } @@ -91,15 +86,14 @@ static bool doImportAsDefinition(const GlobalValue *SGV, - DenseSet *FunctionsToImport); - - /// Access the promoted globals that are now exported and need to be linked. - SetVector &getNewExportedValues() { return NewExportedValues; } + DenseSet *GlobalsToImport); }; /// Perform in-place global value handling on the given Module for /// exported local functions renamed and promoted for ThinLTO. -bool renameModuleForThinLTO(Module &M, const ModuleSummaryIndex &Index); +bool renameModuleForThinLTO( + Module &M, const ModuleSummaryIndex &Index, + DenseSet *GlobalsToImport = nullptr); } // End llvm namespace Index: lib/Linker/LinkModules.cpp =================================================================== --- lib/Linker/LinkModules.cpp +++ lib/Linker/LinkModules.cpp @@ -35,19 +35,9 @@ /// For symbol clashes, prefer those from Src. unsigned Flags; - /// Module summary index passed into ModuleLinker for using in function - /// importing/exporting handling. - const ModuleSummaryIndex *ImportIndex; - /// Functions to import from source module, all other functions are /// imported as declarations instead of definitions. - DenseSet *FunctionsToImport; - - /// Set to true if the given ModuleSummaryIndex contains any functions - /// from this source module, in which case we must conservatively assume - /// that any of its functions may be imported into another module - /// as part of a different backend compilation process. - bool HasExportedFunctions = false; + DenseSet *GlobalsToImport; /// Association between metadata value id and temporary metadata that /// remains unmapped after function importing. Saved during function @@ -116,7 +106,7 @@ /// Helper method to check if we are importing from the current source /// module. - bool isPerformingImport() const { return FunctionsToImport != nullptr; } + bool isPerformingImport() const { return GlobalsToImport != nullptr; } /// If we are importing from the source module, checks if we should /// import SGV as a definition, otherwise import as a declaration. @@ -124,21 +114,10 @@ public: ModuleLinker(IRMover &Mover, std::unique_ptr SrcM, unsigned Flags, - const ModuleSummaryIndex *Index = nullptr, - DenseSet *FunctionsToImport = nullptr, + DenseSet *GlobalsToImport = nullptr, DenseMap *ValIDToTempMDMap = nullptr) - : Mover(Mover), SrcM(std::move(SrcM)), Flags(Flags), ImportIndex(Index), - FunctionsToImport(FunctionsToImport), - ValIDToTempMDMap(ValIDToTempMDMap) { - assert((ImportIndex || !FunctionsToImport) && - "Expect a ModuleSummaryIndex when importing"); - // If we have a ModuleSummaryIndex but no function to import, - // then this is the primary module being compiled in a ThinLTO - // backend compilation, and we need to see if it has functions that - // may be exported to another backend compilation. - if (ImportIndex && !FunctionsToImport) - HasExportedFunctions = ImportIndex->hasExportedFunctions(*this->SrcM); - } + : Mover(Mover), SrcM(std::move(SrcM)), Flags(Flags), + GlobalsToImport(GlobalsToImport), ValIDToTempMDMap(ValIDToTempMDMap) {} bool run(); }; @@ -147,8 +126,8 @@ bool ModuleLinker::doImportAsDefinition(const GlobalValue *SGV) { if (!isPerformingImport()) return false; - return FunctionImportGlobalProcessing::doImportAsDefinition( - SGV, FunctionsToImport); + return FunctionImportGlobalProcessing::doImportAsDefinition(SGV, + GlobalsToImport); } static GlobalValue::VisibilityTypes @@ -297,7 +276,7 @@ if (isa(&Src)) { // For functions, LinkFromSrc iff this is a function requested // for importing. For variables, decide below normally. - LinkFromSrc = FunctionsToImport->count(&Src); + LinkFromSrc = GlobalsToImport->count(&Src); return false; } @@ -423,12 +402,12 @@ if (GV.hasAppendingLinkage() && isPerformingImport()) return false; - if (isPerformingImport() && !doImportAsDefinition(&GV)) - return false; - - if (!DGV && !shouldOverrideFromSrc() && - (GV.hasLocalLinkage() || GV.hasLinkOnceLinkage() || - GV.hasAvailableExternallyLinkage())) + if (isPerformingImport()) { + if (!doImportAsDefinition(&GV)) + return false; + } else if (!DGV && !shouldOverrideFromSrc() && + (GV.hasLocalLinkage() || GV.hasLinkOnceLinkage() || + GV.hasAvailableExternallyLinkage())) return false; if (GV.isDeclaration()) @@ -508,15 +487,6 @@ if (linkIfNeeded(GA)) return true; - if (ImportIndex) { - FunctionImportGlobalProcessing ThinLTOProcessing(*SrcM, *ImportIndex, - FunctionsToImport); - if (ThinLTOProcessing.run()) - return true; - for (auto *GV : ThinLTOProcessing.getNewExportedValues()) - ValuesToLink.insert(GV); - } - for (unsigned I = 0; I < ValuesToLink.size(); ++I) { GlobalValue *GV = ValuesToLink[I]; const Comdat *SC = GV->getComdat(); @@ -549,10 +519,9 @@ Linker::Linker(Module &M) : Mover(M) {} bool Linker::linkInModule(std::unique_ptr Src, unsigned Flags, - const ModuleSummaryIndex *Index, - DenseSet *FunctionsToImport, + DenseSet *GlobalsToImport, DenseMap *ValIDToTempMDMap) { - ModuleLinker ModLinker(Mover, std::move(Src), Flags, Index, FunctionsToImport, + ModuleLinker ModLinker(Mover, std::move(Src), Flags, GlobalsToImport, ValIDToTempMDMap); return ModLinker.run(); } Index: lib/Transforms/IPO/FunctionImport.cpp =================================================================== --- lib/Transforms/IPO/FunctionImport.cpp +++ lib/Transforms/IPO/FunctionImport.cpp @@ -266,7 +266,6 @@ if (!F && isa(SGV)) { auto *SGA = dyn_cast(SGV); F = dyn_cast(SGA->getBaseObject()); - CalledFunctionName = F->getName(); } assert(F && "Imported Function is ... not a Function"); @@ -349,8 +348,11 @@ UpgradeDebugInfo(*SrcModule); // Link in the specified functions. + if (renameModuleForThinLTO(*SrcModule, Index, &FunctionsToImport)) + return true; + if (TheLinker.linkInModule(std::move(SrcModule), Linker::Flags::None, - &Index, &FunctionsToImport)) + &FunctionsToImport)) report_fatal_error("Function Import: link error"); ImportedCount += FunctionsToImport.size(); Index: lib/Transforms/Utils/FunctionImportUtils.cpp =================================================================== --- lib/Transforms/Utils/FunctionImportUtils.cpp +++ lib/Transforms/Utils/FunctionImportUtils.cpp @@ -18,30 +18,20 @@ /// Checks if we should import SGV as a definition, otherwise import as a /// declaration. bool FunctionImportGlobalProcessing::doImportAsDefinition( - const GlobalValue *SGV, DenseSet *FunctionsToImport) { - auto *GA = dyn_cast(SGV); - if (GA) { + const GlobalValue *SGV, DenseSet *GlobalsToImport) { + + // For alias, we tie the definition to the base object. Extract it and recurse + if (auto *GA = dyn_cast(SGV)) { if (GA->hasWeakAnyLinkage()) return false; const GlobalObject *GO = GA->getBaseObject(); if (!GO->hasLinkOnceODRLinkage()) return false; return FunctionImportGlobalProcessing::doImportAsDefinition( - GO, FunctionsToImport); + GO, GlobalsToImport); } - // Always import GlobalVariable definitions, except for the special - // case of WeakAny which are imported as ExternalWeak declarations - // (see comments in FunctionImportGlobalProcessing::getLinkage). The linkage - // changes described in FunctionImportGlobalProcessing::getLinkage ensure the - // correct behavior (e.g. global variables with external linkage are - // transformed to available_externally definitions, which are ultimately - // turned into declarations after the EliminateAvailableExternally pass). - if (isa(SGV) && !SGV->isDeclaration() && - !SGV->hasWeakAnyLinkage()) - return true; - // Only import the function requested for importing. - auto *SF = dyn_cast(SGV); - if (SF && FunctionsToImport->count(SF)) + // Only import the globals requested for importing. + if (GlobalsToImport->count(SGV)) return true; // Otherwise no. return false; @@ -51,8 +41,8 @@ const GlobalValue *SGV) { if (!isPerformingImport()) return false; - return FunctionImportGlobalProcessing::doImportAsDefinition( - SGV, FunctionsToImport); + return FunctionImportGlobalProcessing::doImportAsDefinition(SGV, + GlobalsToImport); } bool FunctionImportGlobalProcessing::doPromoteLocalToGlobal( @@ -198,8 +188,6 @@ GV.setLinkage(getLinkage(&GV)); if (!GV.hasLocalLinkage()) GV.setVisibility(GlobalValue::HiddenVisibility); - if (isModuleExporting()) - NewExportedValues.insert(&GV); } else GV.setLinkage(getLinkage(&GV)); @@ -231,7 +219,9 @@ return false; } -bool llvm::renameModuleForThinLTO(Module &M, const ModuleSummaryIndex &Index) { - FunctionImportGlobalProcessing ThinLTOProcessing(M, Index); +bool llvm::renameModuleForThinLTO( + Module &M, const ModuleSummaryIndex &Index, + DenseSet *GlobalsToImport) { + FunctionImportGlobalProcessing ThinLTOProcessing(M, Index, GlobalsToImport); return ThinLTOProcessing.run(); } Index: test/Linker/funcimport.ll =================================================================== --- test/Linker/funcimport.ll +++ test/Linker/funcimport.ll @@ -67,7 +67,7 @@ ; Ensure that imported static variable and function references are correctly ; promoted and renamed (including static constant variable). ; RUN: llvm-link %t2.bc -summary-index=%t3.thinlto.bc -import=referencestatics:%t.bc -S | FileCheck %s --check-prefix=IMPORTSTATIC -; IMPORTSTATIC-DAG: @staticvar.llvm.1 = available_externally hidden global +; IMPORTSTATIC-DAG: @staticvar.llvm.1 = external hidden global ; IMPORTSTATIC-DAG: @staticconstvar.llvm.1 = internal unnamed_addr constant ; IMPORTSTATIC-DAG: define available_externally i32 @referencestatics ; IMPORTSTATIC-DAG: %call = call i32 @staticfunc.llvm.1 @@ -78,18 +78,18 @@ ; are handled correctly (including referenced variable imported as ; available_externally definition) ; RUN: llvm-link %t2.bc -summary-index=%t3.thinlto.bc -import=referenceglobals:%t.bc -S | FileCheck %s --check-prefix=IMPORTGLOBALS -; IMPORTGLOBALS-DAG: @globalvar = available_externally global +; IMPORTGLOBALS-DAG: @globalvar = external global ; IMPORTGLOBALS-DAG: declare void @globalfunc1() ; IMPORTGLOBALS-DAG: define available_externally i32 @referenceglobals ; Ensure that common variable correctly imported as common defition. ; RUN: llvm-link %t2.bc -summary-index=%t3.thinlto.bc -import=referencecommon:%t.bc -S | FileCheck %s --check-prefix=IMPORTCOMMON -; IMPORTCOMMON-DAG: @commonvar = common global +; IMPORTCOMMON-DAG: @commonvar = external global ; IMPORTCOMMON-DAG: define available_externally i32 @referencecommon ; Ensure that imported static function pointer correctly promoted and renamed. ; RUN: llvm-link %t2.bc -summary-index=%t3.thinlto.bc -import=callfuncptr:%t.bc -S | FileCheck %s --check-prefix=IMPORTFUNCPTR -; IMPORTFUNCPTR-DAG: @P.llvm.1 = available_externally hidden global void ()* null +; IMPORTFUNCPTR-DAG: @P.llvm.1 = external hidden global void ()* ; IMPORTFUNCPTR-DAG: define available_externally void @callfuncptr ; IMPORTFUNCPTR-DAG: %0 = load void ()*, void ()** @P.llvm.1 Index: test/Transforms/FunctionImport/funcimport.ll =================================================================== --- test/Transforms/FunctionImport/funcimport.ll +++ test/Transforms/FunctionImport/funcimport.ll @@ -65,7 +65,7 @@ ; Ensure that all uses of local variable @P which has used in setfuncptr ; and callfuncptr are to the same promoted/renamed global. -; CHECK-DAG: @P.llvm.2 = available_externally hidden global void ()* null +; CHECK-DAG: @P.llvm.2 = external hidden global void ()* ; CHECK-DAG: %0 = load void ()*, void ()** @P.llvm.2, ; CHECK-DAG: store void ()* @staticfunc2.llvm.2, void ()** @P.llvm.2, Index: tools/llvm-link/llvm-link.cpp =================================================================== --- tools/llvm-link/llvm-link.cpp +++ tools/llvm-link/llvm-link.cpp @@ -33,6 +33,8 @@ #include "llvm/Support/SourceMgr.h" #include "llvm/Support/SystemUtils.h" #include "llvm/Support/ToolOutputFile.h" +#include "llvm/Transforms/Utils/FunctionImportUtils.h" + #include using namespace llvm; @@ -191,7 +193,10 @@ if (Verbose) errs() << "Importing " << FunctionName << " from " << FileName << "\n"; - std::unique_ptr Index; + // Link in the specified function. + DenseSet GlobalsToImport; + GlobalsToImport.insert(F); + if (!SummaryIndex.empty()) { ErrorOr> IndexOrErr = llvm::getModuleSummaryIndexForFile(SummaryIndex, diagnosticHandler); @@ -200,7 +205,11 @@ errs() << EC.message() << '\n'; return false; } - Index = std::move(IndexOrErr.get()); + auto Index = std::move(IndexOrErr.get()); + + // Linkage Promotion and renaming + if (renameModuleForThinLTO(*M, *Index, &GlobalsToImport)) + return true; } // Save the mapping of value ids to temporary metadata created when @@ -210,11 +219,8 @@ if (!TempMDVals) TempMDVals = llvm::make_unique>(); - // Link in the specified function. - DenseSet FunctionsToImport; - FunctionsToImport.insert(F); - if (L.linkInModule(std::move(M), Linker::Flags::None, Index.get(), - &FunctionsToImport, TempMDVals.get())) + if (L.linkInModule(std::move(M), Linker::Flags::None, &GlobalsToImport, + TempMDVals.get())) return false; } @@ -260,7 +266,6 @@ // If a module summary index is supplied, load it so linkInModule can treat // local functions/variables as exported and promote if necessary. - std::unique_ptr Index; if (!SummaryIndex.empty()) { ErrorOr> IndexOrErr = llvm::getModuleSummaryIndexForFile(SummaryIndex, diagnosticHandler); @@ -269,13 +274,17 @@ errs() << EC.message() << '\n'; return false; } - Index = std::move(IndexOrErr.get()); + auto Index = std::move(IndexOrErr.get()); + + // Promotion + if (renameModuleForThinLTO(*M, *Index)) + return true; } if (Verbose) errs() << "Linking in '" << File << "'\n"; - if (L.linkInModule(std::move(M), ApplicableFlags, Index.get())) + if (L.linkInModule(std::move(M), ApplicableFlags)) return false; // All linker flags apply to linking of subsequent files. ApplicableFlags = Flags;