[llvm] 460ea85 - [nfc][thinlto] Handle global constant importing separately
Mircea Trofin via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 27 12:22:07 PDT 2023
Author: Mircea Trofin
Date: 2023-04-27T12:21:50-07:00
New Revision: 460ea850148b10dfd30cce32b2ac600b313324c4
URL: https://github.com/llvm/llvm-project/commit/460ea850148b10dfd30cce32b2ac600b313324c4
DIFF: https://github.com/llvm/llvm-project/commit/460ea850148b10dfd30cce32b2ac600b313324c4.diff
LOG: [nfc][thinlto] Handle global constant importing separately
This makes the logic for referenced globals reusable for import criteria
that don't use thresholds - in fact, we currently didn't consider any
thresholds when importing.
Differential Revision: https://reviews.llvm.org/D149298
Added:
Modified:
llvm/include/llvm/IR/ModuleSummaryIndex.h
llvm/lib/IR/ModuleSummaryIndex.cpp
llvm/lib/Transforms/IPO/FunctionImport.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index c540fa5b4ba1..6fdc378ab5c3 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -1815,7 +1815,7 @@ class ModuleSummaryIndex {
void propagateAttributes(const DenseSet<GlobalValue::GUID> &PreservedSymbols);
/// Checks if we can import global variable from another module.
- bool canImportGlobalVar(GlobalValueSummary *S, bool AnalyzeRefs) const;
+ bool canImportGlobalVar(const GlobalValueSummary *S, bool AnalyzeRefs) const;
};
/// GraphTraits definition to build SCC for the index
diff --git a/llvm/lib/IR/ModuleSummaryIndex.cpp b/llvm/lib/IR/ModuleSummaryIndex.cpp
index 2d1440756a95..99e5044bdb9b 100644
--- a/llvm/lib/IR/ModuleSummaryIndex.cpp
+++ b/llvm/lib/IR/ModuleSummaryIndex.cpp
@@ -317,7 +317,7 @@ void ModuleSummaryIndex::propagateAttributes(
}
}
-bool ModuleSummaryIndex::canImportGlobalVar(GlobalValueSummary *S,
+bool ModuleSummaryIndex::canImportGlobalVar(const GlobalValueSummary *S,
bool AnalyzeRefs) const {
auto HasRefsPreventingImport = [this](const GlobalVarSummary *GVS) {
// We don't analyze GV references during attribute propagation, so
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index ee19fcb88964..33da236d947c 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -252,87 +252,115 @@ selectCallee(const ModuleSummaryIndex &Index,
namespace {
-using EdgeInfo =
- std::tuple<const GlobalValueSummary *, unsigned /* Threshold */>;
+using EdgeInfo = std::tuple<const FunctionSummary *, unsigned /* Threshold */>;
} // anonymous namespace
-static bool shouldImportGlobal(
- const ValueInfo &VI, const GVSummaryMapTy &DefinedGVSummaries,
- function_ref<bool(GlobalValue::GUID, const GlobalValueSummary *)>
- isPrevailing) {
- const auto &GVS = DefinedGVSummaries.find(VI.getGUID());
- if (GVS == DefinedGVSummaries.end())
- return true;
- // We should not skip import if the module contains a non-prevailing
- // definition with interposable linkage type. This is required for correctness
- // in the situation where there is a prevailing def available for import and
- // marked read-only. In this case, the non-prevailing def will be converted to
- // a declaration, while the prevailing one becomes internal, thus no
- // definitions will be available for linking. In order to prevent undefined
- // symbol link error, the prevailing definition must be imported.
- // FIXME: Consider adding a check that the suitable prevailing definition
- // exists and marked read-only.
- if (VI.getSummaryList().size() > 1 &&
- GlobalValue::isInterposableLinkage(GVS->second->linkage()) &&
- !isPrevailing(VI.getGUID(), GVS->second))
- return true;
-
- return false;
-}
+/// Import globals referenced by a function or other globals that are being
+/// imported, if importing such global is possible.
+class GlobalsImporter final {
+ const ModuleSummaryIndex &Index;
+ const GVSummaryMapTy &DefinedGVSummaries;
+ function_ref<bool(GlobalValue::GUID, const GlobalValueSummary *)>
+ IsPrevailing;
+ FunctionImporter::ImportMapTy &ImportList;
+ StringMap<FunctionImporter::ExportSetTy> *const ExportLists;
+
+ bool shouldImportGlobal(const ValueInfo &VI) {
+ const auto &GVS = DefinedGVSummaries.find(VI.getGUID());
+ if (GVS == DefinedGVSummaries.end())
+ return true;
+ // We should not skip import if the module contains a non-prevailing
+ // definition with interposable linkage type. This is required for
+ // correctness in the situation where there is a prevailing def available
+ // for import and marked read-only. In this case, the non-prevailing def
+ // will be converted to a declaration, while the prevailing one becomes
+ // internal, thus no definitions will be available for linking. In order to
+ // prevent undefined symbol link error, the prevailing definition must be
+ // imported.
+ // FIXME: Consider adding a check that the suitable prevailing definition
+ // exists and marked read-only.
+ if (VI.getSummaryList().size() > 1 &&
+ GlobalValue::isInterposableLinkage(GVS->second->linkage()) &&
+ !IsPrevailing(VI.getGUID(), GVS->second))
+ return true;
-static void computeImportForReferencedGlobals(
- const GlobalValueSummary &Summary, const ModuleSummaryIndex &Index,
- const GVSummaryMapTy &DefinedGVSummaries,
- function_ref<bool(GlobalValue::GUID, const GlobalValueSummary *)>
- isPrevailing,
- SmallVectorImpl<EdgeInfo> &Worklist,
- FunctionImporter::ImportMapTy &ImportList,
- StringMap<FunctionImporter::ExportSetTy> *ExportLists) {
- for (const auto &VI : Summary.refs()) {
- if (!shouldImportGlobal(VI, DefinedGVSummaries, isPrevailing)) {
- LLVM_DEBUG(
- dbgs() << "Ref ignored! Target already in destination module.\n");
- continue;
- }
+ return false;
+ }
- LLVM_DEBUG(dbgs() << " ref -> " << VI << "\n");
-
- // If this is a local variable, make sure we import the copy
- // in the caller's module. The only time a local variable can
- // share an entry in the index is if there is a local with the same name
- // in another module that had the same source file name (in a
diff erent
- // directory), where each was compiled in their own directory so there
- // was not distinguishing path.
- auto LocalNotInModule = [&](const GlobalValueSummary *RefSummary) -> bool {
- return GlobalValue::isLocalLinkage(RefSummary->linkage()) &&
- RefSummary->modulePath() != Summary.modulePath();
- };
+ void
+ onImportingSummaryImpl(const GlobalValueSummary &Summary,
+ SmallVectorImpl<const GlobalVarSummary *> &Worklist) {
+ for (const auto &VI : Summary.refs()) {
+ if (!shouldImportGlobal(VI)) {
+ LLVM_DEBUG(
+ dbgs() << "Ref ignored! Target already in destination module.\n");
+ continue;
+ }
- for (const auto &RefSummary : VI.getSummaryList())
- if (isa<GlobalVarSummary>(RefSummary.get()) &&
- Index.canImportGlobalVar(RefSummary.get(), /* AnalyzeRefs */ true) &&
- !LocalNotInModule(RefSummary.get())) {
+ LLVM_DEBUG(dbgs() << " ref -> " << VI << "\n");
+
+ // If this is a local variable, make sure we import the copy
+ // in the caller's module. The only time a local variable can
+ // share an entry in the index is if there is a local with the same name
+ // in another module that had the same source file name (in a
diff erent
+ // directory), where each was compiled in their own directory so there
+ // was not distinguishing path.
+ auto LocalNotInModule =
+ [&](const GlobalValueSummary *RefSummary) -> bool {
+ return GlobalValue::isLocalLinkage(RefSummary->linkage()) &&
+ RefSummary->modulePath() != Summary.modulePath();
+ };
+
+ for (const auto &RefSummary : VI.getSummaryList()) {
+ const auto *GVS = dyn_cast<GlobalVarSummary>(RefSummary.get());
+ // Functions could be referenced by global vars - e.g. a vtable; but we
+ // don't currently imagine a reason those would be imported here, rather
+ // than as part of the logic deciding which functions to import (i.e.
+ // based on profile information). Should we decide to handle them here,
+ // we can refactor accordingly at that time.
+ if (!GVS || !Index.canImportGlobalVar(GVS, /* AnalyzeRefs */ true) ||
+ LocalNotInModule(GVS))
+ continue;
auto ILI = ImportList[RefSummary->modulePath()].insert(VI.getGUID());
// Only update stat and exports if we haven't already imported this
// variable.
if (!ILI.second)
break;
NumImportedGlobalVarsThinLink++;
- // Any references made by this variable will be marked exported later,
- // in ComputeCrossModuleImport, after import decisions are complete,
- // which is more efficient than adding them here.
+ // Any references made by this variable will be marked exported
+ // later, in ComputeCrossModuleImport, after import decisions are
+ // complete, which is more efficient than adding them here.
if (ExportLists)
(*ExportLists)[RefSummary->modulePath()].insert(VI);
// If variable is not writeonly we attempt to recursively analyze
// its references in order to import referenced constants.
- if (!Index.isWriteOnly(cast<GlobalVarSummary>(RefSummary.get())))
- Worklist.emplace_back(RefSummary.get(), 0);
+ if (!Index.isWriteOnly(GVS))
+ Worklist.emplace_back(GVS);
break;
}
+ }
}
-}
+
+public:
+ GlobalsImporter(
+ const ModuleSummaryIndex &Index, const GVSummaryMapTy &DefinedGVSummaries,
+ function_ref<bool(GlobalValue::GUID, const GlobalValueSummary *)>
+ IsPrevailing,
+ FunctionImporter::ImportMapTy &ImportList,
+ StringMap<FunctionImporter::ExportSetTy> *ExportLists)
+ : Index(Index), DefinedGVSummaries(DefinedGVSummaries),
+ IsPrevailing(IsPrevailing), ImportList(ImportList),
+ ExportLists(ExportLists) {}
+
+ void onImportingSummary(const GlobalValueSummary &Summary) {
+ SmallVector<const GlobalVarSummary *, 128> Worklist;
+ onImportingSummaryImpl(Summary, Worklist);
+ while (!Worklist.empty())
+ onImportingSummaryImpl(*Worklist.pop_back_val(), Worklist);
+ }
+};
static const char *
getFailureName(FunctionImporter::ImportFailureReason Reason) {
@@ -365,13 +393,11 @@ static void computeImportForFunction(
const unsigned Threshold, const GVSummaryMapTy &DefinedGVSummaries,
function_ref<bool(GlobalValue::GUID, const GlobalValueSummary *)>
isPrevailing,
- SmallVectorImpl<EdgeInfo> &Worklist,
+ SmallVectorImpl<EdgeInfo> &Worklist, GlobalsImporter &GVImporter,
FunctionImporter::ImportMapTy &ImportList,
StringMap<FunctionImporter::ExportSetTy> *ExportLists,
FunctionImporter::ImportThresholdsTy &ImportThresholds) {
- computeImportForReferencedGlobals(Summary, Index, DefinedGVSummaries,
- isPrevailing, Worklist, ImportList,
- ExportLists);
+ GVImporter.onImportingSummary(Summary);
static int ImportCount = 0;
for (const auto &Edge : Summary.calls()) {
ValueInfo VI = Edge.first;
@@ -546,6 +572,8 @@ static void ComputeImportForModule(
// Worklist contains the list of function imported in this module, for which
// we will analyse the callees and may import further down the callgraph.
SmallVector<EdgeInfo, 128> Worklist;
+ GlobalsImporter GVI(Index, DefinedGVSummaries, isPrevailing, ImportList,
+ ExportLists);
FunctionImporter::ImportThresholdsTy ImportThresholds;
// Populate the worklist with the import for the functions in the current
@@ -567,7 +595,7 @@ static void ComputeImportForModule(
continue;
LLVM_DEBUG(dbgs() << "Initialize import for " << VI << "\n");
computeImportForFunction(*FuncSummary, Index, ImportInstrLimit,
- DefinedGVSummaries, isPrevailing, Worklist,
+ DefinedGVSummaries, isPrevailing, Worklist, GVI,
ImportList, ExportLists, ImportThresholds);
}
@@ -579,12 +607,8 @@ static void ComputeImportForModule(
if (auto *FS = dyn_cast<FunctionSummary>(Summary))
computeImportForFunction(*FS, Index, Threshold, DefinedGVSummaries,
- isPrevailing, Worklist, ImportList, ExportLists,
- ImportThresholds);
- else
- computeImportForReferencedGlobals(*Summary, Index, DefinedGVSummaries,
- isPrevailing, Worklist, ImportList,
- ExportLists);
+ isPrevailing, Worklist, GVI, ImportList,
+ ExportLists, ImportThresholds);
}
// Print stats about functions considered but rejected for importing
More information about the llvm-commits
mailing list