diff options
| -rw-r--r-- | llvm/include/llvm/IR/ModuleSummaryIndex.h | 4 | ||||
| -rw-r--r-- | llvm/lib/Transforms/IPO/FunctionImport.cpp | 74 | ||||
| -rw-r--r-- | llvm/lib/Transforms/Utils/FunctionImportUtils.cpp | 5 | ||||
| -rw-r--r-- | llvm/test/ThinLTO/X86/Inputs/section.ll | 13 | ||||
| -rw-r--r-- | llvm/test/ThinLTO/X86/section.ll | 25 |
5 files changed, 117 insertions, 4 deletions
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h index 48f2f7e0b30..c66727b3df2 100644 --- a/llvm/include/llvm/IR/ModuleSummaryIndex.h +++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h @@ -168,6 +168,10 @@ public: return static_cast<GlobalValue::LinkageTypes>(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; } diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp index 3f0d2624f5d..ec82248d78b 100644 --- a/llvm/lib/Transforms/IPO/FunctionImport.cpp +++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp @@ -75,6 +75,69 @@ static std::unique_ptr<Module> loadFile(const std::string &FileName, 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 global that needs renaming if has a section. + return false; + + return true; +} + +// Return true if \p GUID 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 ModuleSummaryIndex &Index, + GlobalValue::GUID GUID) { + auto Summaries = Index.findGlobalValueSummaryList(GUID); + if (Summaries == Index.end()) + return true; + if (Summaries->second.size() != 1) + // If there are multiple globals with this GUID, then we know it is + // not a local symbol, and it is necessarily externally referenced. + return true; + + // We don't need to check for the module path, because if it can't be + // externally referenced and we call it, it is necessarilly in the same + // module + return canBeExternallyReferenced(**Summaries->second.begin()); +} + +// Return true if the global described by \p Summary can be imported in another +// module. +static bool eligibleForImport(const ModuleSummaryIndex &Index, + const GlobalValueSummary &Summary) { + if (!canBeExternallyReferenced(Summary)) + // Can't import a global that needs renaming if has a section for instance. + // FIXME: we may be able to import it by copying it without promotion. + 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) { + return canBeExternallyReferenced(Index, VI.getGUID()); + }); + if (!AllRefsCanBeExternallyReferenced) + return false; + + if (auto *FuncSummary = dyn_cast<FunctionSummary>(&Summary)) { + bool AllCallsCanBeExternallyReferenced = llvm::all_of( + FuncSummary->calls(), [&](const FunctionSummary::EdgeTy &Edge) { + return canBeExternallyReferenced(Index, Edge.first.getGUID()); + }); + if (!AllCallsCanBeExternallyReferenced) + return false; + } + return true; +} + /// Given a list of possible callee implementation for a call site, select one /// that fits the \p Threshold. /// @@ -86,7 +149,8 @@ namespace { /// - One that has PGO data attached. /// - [insert you fancy metric here] static const GlobalValueSummary * -selectCallee(const GlobalValueSummaryList &CalleeSummaryList, +selectCallee(const ModuleSummaryIndex &Index, + const GlobalValueSummaryList &CalleeSummaryList, unsigned Threshold) { auto It = llvm::find_if( CalleeSummaryList, @@ -111,6 +175,9 @@ selectCallee(const GlobalValueSummaryList &CalleeSummaryList, if (Summary->instCount() > Threshold) return false; + if (!eligibleForImport(Index, *Summary)) + return false; + return true; }); if (It == CalleeSummaryList.end()) @@ -125,10 +192,9 @@ static const GlobalValueSummary *selectCallee(GlobalValue::GUID GUID, unsigned Threshold, const ModuleSummaryIndex &Index) { auto CalleeSummaryList = Index.findGlobalValueSummaryList(GUID); - if (CalleeSummaryList == Index.end()) { + if (CalleeSummaryList == Index.end()) return nullptr; // This function does not have a summary - } - return selectCallee(CalleeSummaryList->second, Threshold); + return selectCallee(Index, CalleeSummaryList->second, Threshold); } /// Mark the global \p GUID as export by module \p ExportModulePath if found in diff --git a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp index f1d12bce06c..4280e96841e 100644 --- a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp +++ b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp @@ -64,6 +64,11 @@ bool FunctionImportGlobalProcessing::doPromoteLocalToGlobal( 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). diff --git a/llvm/test/ThinLTO/X86/Inputs/section.ll b/llvm/test/ThinLTO/X86/Inputs/section.ll new file mode 100644 index 00000000000..4f4ea91c986 --- /dev/null +++ b/llvm/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 +} diff --git a/llvm/test/ThinLTO/X86/section.ll b/llvm/test/ThinLTO/X86/section.ll new file mode 100644 index 00000000000..3cc1325e472 --- /dev/null +++ b/llvm/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() |
