Index: include/llvm/IR/ModuleSummaryIndex.h =================================================================== --- include/llvm/IR/ModuleSummaryIndex.h +++ include/llvm/IR/ModuleSummaryIndex.h @@ -169,6 +169,10 @@ /// Return linkage type recorded for this global value. GlobalValue::LinkageTypes linkage() const { return Flags.Linkage; } + /// Return true if this summary is for a GlobalValue that needs promotion + /// to be referenced from another module. + bool needsRenaming() const { return GlobalValue::isLocalLinkage(linkage()); } + /// Return true if this global value is located in a specific section. bool hasSection() const { return Flags.HasSection; } @@ -399,6 +403,24 @@ return GlobalValueMap.find(ValueGUID); } + /// Find the summary for global \p GUID in module \p ModuleId, or nullptr if + /// not found. + GlobalValueSummary *findSummaryInModule(GlobalValue::GUID ValueGUID, + StringRef ModuleId) const { + auto CalleeInfoList = findGlobalValueInfoList(ValueGUID); + if (CalleeInfoList == end()) { + return nullptr; // This function does not have a summary + } + auto GVInfo = + llvm::find_if(CalleeInfoList->second, + [&](const std::unique_ptr &GlobInfo) { + return GlobInfo->summary()->modulePath() == ModuleId; + }); + if (GVInfo == CalleeInfoList->second.end()) + return nullptr; + return (*GVInfo)->summary(); + } + /// Add a global value info for a value of the given name. void addGlobalValueInfo(StringRef ValueName, std::unique_ptr Info) { Index: lib/Transforms/IPO/FunctionImport.cpp =================================================================== --- lib/Transforms/IPO/FunctionImport.cpp +++ lib/Transforms/IPO/FunctionImport.cpp @@ -75,6 +75,69 @@ namespace { +// Return true if the Summary describes a GlobalValue that can be externally +// referenced, i.e. it does not need renaming (linkage is not local) or renaming +// is possible (does not have a section for instance). +static bool canBeExternallyReferenced(const GlobalValueSummary &Summary) { + if (!Summary.needsRenaming()) + return true; + + if (Summary.hasSection()) + // Can't rename a globals that needs renaming if has a section + return false; + + return true; +} + +// Return true if the module contains a definition for \p GUID and this value +// can be referenced from another module. +static bool valueCanBeExternallyReferencedFromModule( + GlobalValue::GUID GUID, + const std::map & + DefinedGVSummaries) { + auto Summary = DefinedGVSummaries.find(GUID); + if (Summary == DefinedGVSummaries.end()) + return true; + if (!canBeExternallyReferenced(*Summary->second)) + return false; + return true; +} + +/// +static bool +eligibleForImport(const ModuleSummaryIndex &Index, + const GlobalValueSummary &Summary, + const std::map & + DefinedGVSummaries) { + if (!canBeExternallyReferenced(Summary)) + // Can't import a globals that needs renaming if has a section for instance. + return false; + + // Check references (and potential calls) in the same module. If the current + // value references a global that can't be externally referenced it is not + // eligible for import. + bool AllRefsCanBeExternallyReferenced = + llvm::all_of(Summary.refs(), [&](const ValueInfo &VI) { + auto GUID = VI.getGUID(); + return !valueCanBeExternallyReferencedFromModule(GUID, + DefinedGVSummaries); + }); + if (!AllRefsCanBeExternallyReferenced) + return false; + + if (auto *FuncSummary = dyn_cast(&Summary)) { + bool AllCallsCanBeExternallyReferenced = llvm::all_of( + FuncSummary->calls(), [&](const FunctionSummary::EdgeTy &Edge) { + auto CalleeGUID = Edge.first.getGUID(); + return !valueCanBeExternallyReferencedFromModule(CalleeGUID, + DefinedGVSummaries); + }); + if (!AllCallsCanBeExternallyReferenced) + return false; + } + return true; +} + /// Given a list of possible callee implementation for a call site, select one /// that fits the \p Threshold. /// @@ -85,8 +148,10 @@ /// number of source modules parsed/linked. /// - One that has PGO data attached. /// - [insert you fancy metric here] -static const GlobalValueSummary * -selectCallee(const GlobalValueInfoList &CalleeInfoList, unsigned Threshold) { +static const GlobalValueSummary *selectCallee( + const std::map &DefinedGVSummaries, + const ModuleSummaryIndex &Index, const GlobalValueInfoList &CalleeInfoList, + unsigned Threshold) { auto It = llvm::find_if( CalleeInfoList, [&](const std::unique_ptr &GlobInfo) { assert(GlobInfo->summary() && @@ -111,6 +176,9 @@ if (Summary->instCount() > Threshold) return false; + if (!eligibleForImport(Index, *Summary, DefinedGVSummaries)) + return false; + return true; }); if (It == CalleeInfoList.end()) @@ -121,14 +189,16 @@ /// Return the summary for the function \p GUID that fits the \p Threshold, or /// null if there's no match. -static const GlobalValueSummary *selectCallee(GlobalValue::GUID GUID, - unsigned Threshold, - const ModuleSummaryIndex &Index) { +static const GlobalValueSummary *selectCallee( + const std::map &DefinedGVSummaries, + GlobalValue::GUID GUID, unsigned Threshold, + const ModuleSummaryIndex &Index) { auto CalleeInfoList = Index.findGlobalValueInfoList(GUID); if (CalleeInfoList == Index.end()) { return nullptr; // This function does not have a summary } - return selectCallee(CalleeInfoList->second, Threshold); + return selectCallee(DefinedGVSummaries, Index, CalleeInfoList->second, + Threshold); } /// Return true if the global \p GUID is exported by module \p ExportModulePath. @@ -171,7 +241,8 @@ continue; } - auto *CalleeSummary = selectCallee(GUID, Threshold, Index); + auto *CalleeSummary = + selectCallee(DefinedGVSummaries, GUID, Threshold, Index); if (!CalleeSummary) { DEBUG(dbgs() << "ignored! No qualifying callee with summary found.\n"); continue; Index: lib/Transforms/Utils/FunctionImportUtils.cpp =================================================================== --- lib/Transforms/Utils/FunctionImportUtils.cpp +++ lib/Transforms/Utils/FunctionImportUtils.cpp @@ -64,6 +64,11 @@ if (GVar && GVar->isConstant() && GVar->hasUnnamedAddr()) return false; + if (GVar && GVar->hasSection()) + // Some sections like "__DATA,__cfstring" are "magic" and promotion is not + // allowed. Just disable promotion on any GVar with sections right now. + return false; + // Eventually we only need to promote functions in the exporting module that // are referenced by a potentially exported function (i.e. one that is in the // summary index). Index: test/ThinLTO/X86/Inputs/section.ll =================================================================== --- /dev/null +++ test/ThinLTO/X86/Inputs/section.ll @@ -0,0 +1,13 @@ +; An internal global variable that can't be renamed because it has a section +@var_with_section = internal global i32 0, section "some_section" + +; @reference_gv_with_section() can't be imported +define i32 @reference_gv_with_section() { + %res = load i32, i32* @var_with_section + ret i32 %res +} + +; canary +define void @foo() { + ret void +} Index: test/ThinLTO/X86/section.ll =================================================================== --- /dev/null +++ test/ThinLTO/X86/section.ll @@ -0,0 +1,25 @@ +; Do setup work for all below tests: generate bitcode and combined index +; RUN: opt -module-summary %s -o %t.bc +; RUN: opt -module-summary %p/Inputs/section.ll -o %t2.bc +; RUN: llvm-lto -thinlto-action=thinlink -o %t3.bc %t.bc %t2.bc + +; Check that we don't promote 'var_with_section' +; RUN: llvm-lto -thinlto-action=promote %t2.bc -thinlto-index=%t3.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=PROMOTE +; PROMOTE: @var_with_section = internal global i32 0, section "some_section" + +; RUN: llvm-lto -thinlto-action=import %t.bc -thinlto-index=%t3.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=IMPORT +; Check that section prevent import of @reference_gv_with_section. +; IMPORT: declare void @reference_gv_with_section() +; Canary to check that importing is correctly set up. +; IMPORT: define available_externally void @foo() + + +define i32 @main() { + call void @reference_gv_with_section() + call void @foo() + ret i32 42 +} + + +declare void @reference_gv_with_section() +declare void @foo()