[llvm-branch-commits] [llvm] 45257fc - Revert "[ThinLTO] Populate declaration import status except for distributed T…"
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Sun May 19 22:43:54 PDT 2024
Author: Mingming Liu
Date: 2024-05-19T22:41:50-07:00
New Revision: 45257fcc3f5c86c406a723d8b87192d5c9ca8b4c
URL: https://github.com/llvm/llvm-project/commit/45257fcc3f5c86c406a723d8b87192d5c9ca8b4c
DIFF: https://github.com/llvm/llvm-project/commit/45257fcc3f5c86c406a723d8b87192d5c9ca8b4c.diff
LOG: Revert "[ThinLTO] Populate declaration import status except for distributed T…"
This reverts commit 8de7890572296830b27b6e6db39b36810bc98c31.
Added:
Modified:
llvm/include/llvm/IR/ModuleSummaryIndex.h
llvm/include/llvm/Transforms/IPO/FunctionImport.h
llvm/lib/LTO/LTO.cpp
llvm/lib/LTO/LTOBackend.cpp
llvm/lib/Transforms/IPO/FunctionImport.cpp
llvm/test/ThinLTO/X86/funcimport-stats.ll
llvm/test/Transforms/FunctionImport/funcimport.ll
llvm/tools/llvm-link/llvm-link.cpp
Removed:
llvm/test/ThinLTO/X86/import_callee_declaration.ll
################################################################################
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index a6bb261af7522..5d137d4b3553c 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -587,10 +587,6 @@ class GlobalValueSummary {
void setImportKind(ImportKind IK) { Flags.ImportType = IK; }
- GlobalValueSummary::ImportKind importType() const {
- return static_cast<ImportKind>(Flags.ImportType);
- }
-
GlobalValue::VisibilityTypes getVisibility() const {
return (GlobalValue::VisibilityTypes)Flags.Visibility;
}
@@ -1276,9 +1272,6 @@ using ModulePathStringTableTy = StringMap<ModuleHash>;
/// a particular module, and provide efficient access to their summary.
using GVSummaryMapTy = DenseMap<GlobalValue::GUID, GlobalValueSummary *>;
-/// A set of global value summary pointers.
-using GVSummaryPtrSet = SmallPtrSet<GlobalValueSummary *, 4>;
-
/// Map of a type GUID to type id string and summary (multimap used
/// in case of GUID conflicts).
using TypeIdSummaryMapTy =
diff --git a/llvm/include/llvm/Transforms/IPO/FunctionImport.h b/llvm/include/llvm/Transforms/IPO/FunctionImport.h
index 024bba8105b89..c4d19e8641eca 100644
--- a/llvm/include/llvm/Transforms/IPO/FunctionImport.h
+++ b/llvm/include/llvm/Transforms/IPO/FunctionImport.h
@@ -31,9 +31,9 @@ class Module;
/// based on the provided summary informations.
class FunctionImporter {
public:
- /// The functions to import from a source module and their import type.
- using FunctionsToImportTy =
- DenseMap<GlobalValue::GUID, GlobalValueSummary::ImportKind>;
+ /// Set of functions to import from a source module. Each entry is a set
+ /// containing all the GUIDs of all functions to import for a source module.
+ using FunctionsToImportTy = std::unordered_set<GlobalValue::GUID>;
/// The
diff erent reasons selectCallee will chose not to import a
/// candidate.
@@ -99,13 +99,8 @@ class FunctionImporter {
/// index's module path string table).
using ImportMapTy = DenseMap<StringRef, FunctionsToImportTy>;
- /// The map contains an entry for every global value the module exports.
- /// The key is ValueInfo, and the value indicates whether the definition
- /// or declaration is visible to another module. If a function's definition is
- /// visible to other modules, the global values this function referenced are
- /// visible and shouldn't be internalized.
- /// TODO: Rename to `ExportMapTy`.
- using ExportSetTy = DenseMap<ValueInfo, GlobalValueSummary::ImportKind>;
+ /// The set contains an entry for every global value the module exports.
+ using ExportSetTy = DenseSet<ValueInfo>;
/// A function of this type is used to load modules referenced by the index.
using ModuleLoaderTy =
diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index e2754d74979e8..5c603ac6ab472 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -121,9 +121,6 @@ void llvm::computeLTOCacheKey(
support::endian::write64le(Data, I);
Hasher.update(Data);
};
- auto AddUint8 = [&](const uint8_t I) {
- Hasher.update(ArrayRef<uint8_t>((const uint8_t *)&I, 1));
- };
AddString(Conf.CPU);
// FIXME: Hash more of Options. For now all clients initialize Options from
// command-line flags (which is unsupported in production), but may set
@@ -159,18 +156,18 @@ void llvm::computeLTOCacheKey(
auto ModHash = Index.getModuleHash(ModuleID);
Hasher.update(ArrayRef<uint8_t>((uint8_t *)&ModHash[0], sizeof(ModHash)));
- std::vector<std::pair<uint64_t, uint8_t>> ExportsGUID;
+ std::vector<uint64_t> ExportsGUID;
ExportsGUID.reserve(ExportList.size());
- for (const auto &[VI, ExportType] : ExportList)
- ExportsGUID.push_back(
- std::make_pair(VI.getGUID(), static_cast<uint8_t>(ExportType)));
+ for (const auto &VI : ExportList) {
+ auto GUID = VI.getGUID();
+ ExportsGUID.push_back(GUID);
+ }
// Sort the export list elements GUIDs.
llvm::sort(ExportsGUID);
- for (auto [GUID, ExportType] : ExportsGUID) {
+ for (uint64_t GUID : ExportsGUID) {
// The export list can impact the internalization, be conservative here
Hasher.update(ArrayRef<uint8_t>((uint8_t *)&GUID, sizeof(GUID)));
- AddUint8(ExportType);
}
// Include the hash for every module we import functions from. The set of
@@ -202,7 +199,7 @@ void llvm::computeLTOCacheKey(
[](const ImportModule &Lhs, const ImportModule &Rhs) -> bool {
return Lhs.getHash() < Rhs.getHash();
});
- std::vector<std::pair<uint64_t, uint8_t>> ImportedGUIDs;
+ std::vector<uint64_t> ImportedGUIDs;
for (const ImportModule &Entry : ImportModulesVector) {
auto ModHash = Entry.getHash();
Hasher.update(ArrayRef<uint8_t>((uint8_t *)&ModHash[0], sizeof(ModHash)));
@@ -210,13 +207,11 @@ void llvm::computeLTOCacheKey(
AddUint64(Entry.getFunctions().size());
ImportedGUIDs.clear();
- for (auto &[Fn, ImportType] : Entry.getFunctions())
- ImportedGUIDs.push_back(std::make_pair(Fn, ImportType));
+ for (auto &Fn : Entry.getFunctions())
+ ImportedGUIDs.push_back(Fn);
llvm::sort(ImportedGUIDs);
- for (auto &[GUID, Type] : ImportedGUIDs) {
+ for (auto &GUID : ImportedGUIDs)
AddUint64(GUID);
- AddUint8(Type);
- }
}
// Include the hash for the resolved ODR.
@@ -286,9 +281,9 @@ void llvm::computeLTOCacheKey(
// Imported functions may introduce new uses of type identifier resolutions,
// so we need to collect their used resolutions as well.
for (const ImportModule &ImpM : ImportModulesVector)
- for (auto &[GUID, UnusedImportType] : ImpM.getFunctions()) {
+ for (auto &ImpF : ImpM.getFunctions()) {
GlobalValueSummary *S =
- Index.findSummaryInModule(GUID, ImpM.getIdentifier());
+ Index.findSummaryInModule(ImpF, ImpM.getIdentifier());
AddUsedThings(S);
// If this is an alias, we also care about any types/etc. that the aliasee
// may reference.
@@ -1400,7 +1395,6 @@ class lto::ThinBackendProc {
llvm::StringRef ModulePath,
const std::string &NewModulePath) {
std::map<std::string, GVSummaryMapTy> ModuleToSummariesForIndex;
-
std::error_code EC;
gatherImportedSummariesForModule(ModulePath, ModuleToDefinedGVSummaries,
ImportList, ModuleToSummariesForIndex);
@@ -1409,8 +1403,6 @@ class lto::ThinBackendProc {
sys::fs::OpenFlags::OF_None);
if (EC)
return errorCodeToError(EC);
-
- // TODO: Serialize declaration bits to bitcode.
writeIndexToFile(CombinedIndex, OS, &ModuleToSummariesForIndex);
if (ShouldEmitImportsFiles) {
diff --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp
index 58434feec6f96..d4b89ede2d713 100644
--- a/llvm/lib/LTO/LTOBackend.cpp
+++ b/llvm/lib/LTO/LTOBackend.cpp
@@ -721,14 +721,7 @@ bool lto::initImportList(const Module &M,
if (Summary->modulePath() == M.getModuleIdentifier())
continue;
// Add an entry to provoke importing by thinBackend.
- // Try emplace the entry first. If an entry with the same key already
- // exists, set the value to 'std::min(existing-value, new-value)' to make
- // sure a definition takes precedence over a declaration.
- auto [Iter, Inserted] = ImportList[Summary->modulePath()].try_emplace(
- GUID, Summary->importType());
-
- if (!Inserted)
- Iter->second = std::min(Iter->second, Summary->importType());
+ ImportList[Summary->modulePath()].insert(GUID);
}
}
return true;
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index a116fd6535347..68f9799616ae6 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -140,17 +140,6 @@ static cl::opt<bool>
ImportAllIndex("import-all-index",
cl::desc("Import all external functions in index."));
-/// This is a test-only option.
-/// If this option is enabled, the ThinLTO indexing step will import each
-/// function declaration as a fallback. In a real build this may increase ram
-/// usage of the indexing step unnecessarily.
-/// TODO: Implement selective import (based on combined summary analysis) to
-/// ensure the imported function has a use case in the postlink pipeline.
-static cl::opt<bool> ImportDeclaration(
- "import-declaration", cl::init(false), cl::Hidden,
- cl::desc("If true, import function declaration as fallback if the function "
- "definition is not imported."));
-
/// Pass a workload description file - an example of workload would be the
/// functions executed to satisfy a RPC request. A workload is defined by a root
/// function and the list of functions that are (frequently) needed to satisfy
@@ -256,12 +245,8 @@ static auto qualifyCalleeCandidates(
}
/// Given a list of possible callee implementation for a call site, select one
-/// that fits the \p Threshold for function definition import. If none are
-/// found, the Reason will give the last reason for the failure (last, in the
-/// order of CalleeSummaryList entries). While looking for a callee definition,
-/// sets \p TooLargeOrNoInlineSummary to the last seen too-large or noinline
-/// candidate; other modules may want to know the function summary or
-/// declaration even if a definition is not needed.
+/// that fits the \p Threshold. If none are found, the Reason will give the last
+/// reason for the failure (last, in the order of CalleeSummaryList entries).
///
/// FIXME: select "best" instead of first that fits. But what is "best"?
/// - The smallest: more likely to be inlined.
@@ -274,32 +259,24 @@ static const GlobalValueSummary *
selectCallee(const ModuleSummaryIndex &Index,
ArrayRef<std::unique_ptr<GlobalValueSummary>> CalleeSummaryList,
unsigned Threshold, StringRef CallerModulePath,
- const GlobalValueSummary *&TooLargeOrNoInlineSummary,
FunctionImporter::ImportFailureReason &Reason) {
- // Records the last summary with reason noinline or too-large.
- TooLargeOrNoInlineSummary = nullptr;
auto QualifiedCandidates =
qualifyCalleeCandidates(Index, CalleeSummaryList, CallerModulePath);
for (auto QualifiedValue : QualifiedCandidates) {
Reason = QualifiedValue.first;
- // Skip a summary if its import is not (proved to be) legal.
if (Reason != FunctionImporter::ImportFailureReason::None)
continue;
auto *Summary =
cast<FunctionSummary>(QualifiedValue.second->getBaseObject());
- // Don't bother importing the definition if the chance of inlining it is
- // not high enough (except under `--force-import-all`).
if ((Summary->instCount() > Threshold) && !Summary->fflags().AlwaysInline &&
!ForceImportAll) {
- TooLargeOrNoInlineSummary = Summary;
Reason = FunctionImporter::ImportFailureReason::TooLarge;
continue;
}
- // Don't bother importing the definition if we can't inline it anyway.
+ // Don't bother importing if we can't inline it anyway.
if (Summary->fflags().NoInline && !ForceImportAll) {
- TooLargeOrNoInlineSummary = Summary;
Reason = FunctionImporter::ImportFailureReason::NoInline;
continue;
}
@@ -381,27 +358,17 @@ class GlobalsImporter final {
if (!GVS || !Index.canImportGlobalVar(GVS, /* AnalyzeRefs */ true) ||
LocalNotInModule(GVS))
continue;
-
- // If there isn't an entry for GUID, insert <GUID, Definition> pair.
- // Otherwise, definition should take precedence over declaration.
- auto [Iter, Inserted] =
- ImportList[RefSummary->modulePath()].try_emplace(
- VI.getGUID(), GlobalValueSummary::Definition);
+ auto ILI = ImportList[RefSummary->modulePath()].insert(VI.getGUID());
// Only update stat and exports if we haven't already imported this
// variable.
- if (!Inserted) {
- // Set the value to 'std::min(existing-value, new-value)' to make
- // sure a definition takes precedence over a declaration.
- Iter->second = std::min(GlobalValueSummary::Definition, Iter->second);
+ 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.
if (ExportLists)
- (*ExportLists)[RefSummary->modulePath()][VI] =
- GlobalValueSummary::Definition;
+ (*ExportLists)[RefSummary->modulePath()].insert(VI);
// If variable is not writeonly we attempt to recursively analyze
// its references in order to import referenced constants.
@@ -578,11 +545,10 @@ class WorkloadImportsManager : public ModuleImportsManager {
LLVM_DEBUG(dbgs() << "[Workload][Including]" << VI.name() << " from "
<< ExportingModule << " : "
<< Function::getGUID(VI.name()) << "\n");
- ImportList[ExportingModule][VI.getGUID()] =
- GlobalValueSummary::Definition;
+ ImportList[ExportingModule].insert(VI.getGUID());
GVI.onImportingSummary(*GVS);
if (ExportLists)
- (*ExportLists)[ExportingModule][VI] = GlobalValueSummary::Definition;
+ (*ExportLists)[ExportingModule].insert(VI);
}
LLVM_DEBUG(dbgs() << "[Workload] Done\n");
}
@@ -803,28 +769,9 @@ static void computeImportForFunction(
}
FunctionImporter::ImportFailureReason Reason{};
-
- // `SummaryForDeclImport` is an summary eligible for declaration import.
- const GlobalValueSummary *SummaryForDeclImport = nullptr;
- CalleeSummary =
- selectCallee(Index, VI.getSummaryList(), NewThreshold,
- Summary.modulePath(), SummaryForDeclImport, Reason);
+ CalleeSummary = selectCallee(Index, VI.getSummaryList(), NewThreshold,
+ Summary.modulePath(), Reason);
if (!CalleeSummary) {
- // There isn't a callee for definition import but one for declaration
- // import.
- if (ImportDeclaration && SummaryForDeclImport) {
- StringRef DeclSourceModule = SummaryForDeclImport->modulePath();
-
- // Since definition takes precedence over declaration for the same VI,
- // try emplace <VI, declaration> pair without checking insert result.
- // If insert doesn't happen, there must be an existing entry keyed by
- // VI.
- if (ExportLists)
- (*ExportLists)[DeclSourceModule].try_emplace(
- VI, GlobalValueSummary::Declaration);
- ImportList[DeclSourceModule].try_emplace(
- VI.getGUID(), GlobalValueSummary::Declaration);
- }
// Update with new larger threshold if this was a retry (otherwise
// we would have already inserted with NewThreshold above). Also
// update failure info if requested.
@@ -869,15 +816,11 @@ static void computeImportForFunction(
"selectCallee() didn't honor the threshold");
auto ExportModulePath = ResolvedCalleeSummary->modulePath();
-
- // Try emplace the definition entry, and update stats based on insertion
- // status.
- auto [Iter, Inserted] = ImportList[ExportModulePath].try_emplace(
- VI.getGUID(), GlobalValueSummary::Definition);
-
+ auto ILI = ImportList[ExportModulePath].insert(VI.getGUID());
// We previously decided to import this GUID definition if it was already
// inserted in the set of imports from the exporting module.
- if (Inserted || Iter->second == GlobalValueSummary::Declaration) {
+ bool PreviouslyImported = !ILI.second;
+ if (!PreviouslyImported) {
NumImportedFunctionsThinLink++;
if (IsHotCallsite)
NumImportedHotFunctionsThinLink++;
@@ -885,14 +828,11 @@ static void computeImportForFunction(
NumImportedCriticalFunctionsThinLink++;
}
- if (Iter->second == GlobalValueSummary::Declaration)
- Iter->second = GlobalValueSummary::Definition;
-
// Any calls/references made by this function will be marked exported
// later, in ComputeCrossModuleImport, after import decisions are
// complete, which is more efficient than adding them here.
if (ExportLists)
- (*ExportLists)[ExportModulePath][VI] = GlobalValueSummary::Definition;
+ (*ExportLists)[ExportModulePath].insert(VI);
}
auto GetAdjustedThreshold = [](unsigned Threshold, bool IsHotCallsite) {
@@ -999,20 +939,12 @@ static bool isGlobalVarSummary(const ModuleSummaryIndex &Index,
}
template <class T>
-static unsigned numGlobalVarSummaries(const ModuleSummaryIndex &Index, T &Cont,
- unsigned &DefinedGVS,
- unsigned &DefinedFS) {
+static unsigned numGlobalVarSummaries(const ModuleSummaryIndex &Index,
+ T &Cont) {
unsigned NumGVS = 0;
- DefinedGVS = 0;
- DefinedFS = 0;
- for (auto &[GUID, Type] : Cont) {
- if (isGlobalVarSummary(Index, GUID)) {
- if (Type == GlobalValueSummary::Definition)
- ++DefinedGVS;
+ for (auto &V : Cont)
+ if (isGlobalVarSummary(Index, V))
++NumGVS;
- } else if (Type == GlobalValueSummary::Definition)
- ++DefinedFS;
- }
return NumGVS;
}
#endif
@@ -1022,12 +954,13 @@ static bool checkVariableImport(
const ModuleSummaryIndex &Index,
DenseMap<StringRef, FunctionImporter::ImportMapTy> &ImportLists,
DenseMap<StringRef, FunctionImporter::ExportSetTy> &ExportLists) {
+
DenseSet<GlobalValue::GUID> FlattenedImports;
for (auto &ImportPerModule : ImportLists)
for (auto &ExportPerModule : ImportPerModule.second)
- for (auto &[GUID, Type] : ExportPerModule.second)
- FlattenedImports.insert(GUID);
+ FlattenedImports.insert(ExportPerModule.second.begin(),
+ ExportPerModule.second.end());
// Checks that all GUIDs of read/writeonly vars we see in export lists
// are also in the import lists. Otherwise we my face linker undefs,
@@ -1046,7 +979,7 @@ static bool checkVariableImport(
};
for (auto &ExportPerModule : ExportLists)
- for (auto &[VI, Unused] : ExportPerModule.second)
+ for (auto &VI : ExportPerModule.second)
if (!FlattenedImports.count(VI.getGUID()) &&
IsReadOrWriteOnlyVarNeedingImporting(ExportPerModule.first, VI))
return false;
@@ -1082,11 +1015,7 @@ void llvm::ComputeCrossModuleImport(
FunctionImporter::ExportSetTy NewExports;
const auto &DefinedGVSummaries =
ModuleToDefinedGVSummaries.lookup(ELI.first);
- for (auto &[EI, Type] : ELI.second) {
- // If a variable is exported as a declaration, its 'refs' and 'calls' are
- // not further exported.
- if (Type == GlobalValueSummary::Declaration)
- continue;
+ for (auto &EI : ELI.second) {
// Find the copy defined in the exporting module so that we can mark the
// values it references in that specific definition as exported.
// Below we will add all references and called values, without regard to
@@ -1105,31 +1034,22 @@ void llvm::ComputeCrossModuleImport(
// we convert such variables initializers to "zeroinitializer".
// See processGlobalForThinLTO.
if (!Index.isWriteOnly(GVS))
- for (const auto &VI : GVS->refs()) {
- // Try to emplace the declaration entry. If a definition entry
- // already exists for key `VI`, this is a no-op.
- NewExports.try_emplace(VI, GlobalValueSummary::Declaration);
- }
+ for (const auto &VI : GVS->refs())
+ NewExports.insert(VI);
} else {
auto *FS = cast<FunctionSummary>(S);
- for (const auto &Edge : FS->calls()) {
- // Try to emplace the declaration entry. If a definition entry
- // already exists for key `VI`, this is a no-op.
- NewExports.try_emplace(Edge.first, GlobalValueSummary::Declaration);
- }
- for (const auto &Ref : FS->refs()) {
- // Try to emplace the declaration entry. If a definition entry
- // already exists for key `VI`, this is a no-op.
- NewExports.try_emplace(Ref, GlobalValueSummary::Declaration);
- }
+ for (const auto &Edge : FS->calls())
+ NewExports.insert(Edge.first);
+ for (const auto &Ref : FS->refs())
+ NewExports.insert(Ref);
}
}
- // Prune list computed above to only include values defined in the
- // exporting module. We do this after the above insertion since we may hit
- // the same ref/call target multiple times in above loop, and it is more
- // efficient to avoid a set lookup each time.
+ // Prune list computed above to only include values defined in the exporting
+ // module. We do this after the above insertion since we may hit the same
+ // ref/call target multiple times in above loop, and it is more efficient to
+ // avoid a set lookup each time.
for (auto EI = NewExports.begin(); EI != NewExports.end();) {
- if (!DefinedGVSummaries.count(EI->first.getGUID()))
+ if (!DefinedGVSummaries.count(EI->getGUID()))
NewExports.erase(EI++);
else
++EI;
@@ -1144,29 +1064,18 @@ void llvm::ComputeCrossModuleImport(
for (auto &ModuleImports : ImportLists) {
auto ModName = ModuleImports.first;
auto &Exports = ExportLists[ModName];
- unsigned DefinedGVS = 0, DefinedFS = 0;
- unsigned NumGVS =
- numGlobalVarSummaries(Index, Exports, DefinedGVS, DefinedFS);
- LLVM_DEBUG(dbgs() << "* Module " << ModName << " exports " << DefinedFS
- << " function as definitions, "
- << Exports.size() - NumGVS - DefinedFS
- << " functions as declarations, " << DefinedGVS
- << " var definitions and " << NumGVS - DefinedGVS
- << " var declarations. Imports from "
- << ModuleImports.second.size() << " modules.\n");
+ unsigned NumGVS = numGlobalVarSummaries(Index, Exports);
+ LLVM_DEBUG(dbgs() << "* Module " << ModName << " exports "
+ << Exports.size() - NumGVS << " functions and " << NumGVS
+ << " vars. Imports from " << ModuleImports.second.size()
+ << " modules.\n");
for (auto &Src : ModuleImports.second) {
auto SrcModName = Src.first;
- unsigned DefinedGVS = 0, DefinedFS = 0;
- unsigned NumGVSPerMod =
- numGlobalVarSummaries(Index, Src.second, DefinedGVS, DefinedFS);
- LLVM_DEBUG(dbgs() << " - " << DefinedFS << " function definitions and "
- << Src.second.size() - NumGVSPerMod - DefinedFS
- << " function declarations imported from " << SrcModName
- << "\n");
- LLVM_DEBUG(dbgs() << " - " << DefinedGVS << " global vars definition and "
- << NumGVSPerMod - DefinedGVS
- << " global vars declaration imported from "
- << SrcModName << "\n");
+ unsigned NumGVSPerMod = numGlobalVarSummaries(Index, Src.second);
+ LLVM_DEBUG(dbgs() << " - " << Src.second.size() - NumGVSPerMod
+ << " functions imported from " << SrcModName << "\n");
+ LLVM_DEBUG(dbgs() << " - " << NumGVSPerMod
+ << " global vars imported from " << SrcModName << "\n");
}
}
#endif
@@ -1180,17 +1089,11 @@ static void dumpImportListForModule(const ModuleSummaryIndex &Index,
<< ImportList.size() << " modules.\n");
for (auto &Src : ImportList) {
auto SrcModName = Src.first;
- unsigned DefinedGVS = 0, DefinedFS = 0;
- unsigned NumGVSPerMod =
- numGlobalVarSummaries(Index, Src.second, DefinedGVS, DefinedFS);
- LLVM_DEBUG(dbgs() << " - " << DefinedFS << " function definitions and "
- << Src.second.size() - DefinedFS - NumGVSPerMod
- << " function declarations imported from " << SrcModName
- << "\n");
- LLVM_DEBUG(dbgs() << " - " << DefinedGVS << " var definitions and "
- << NumGVSPerMod - DefinedGVS
- << " var declarations imported from " << SrcModName
- << "\n");
+ unsigned NumGVSPerMod = numGlobalVarSummaries(Index, Src.second);
+ LLVM_DEBUG(dbgs() << " - " << Src.second.size() - NumGVSPerMod
+ << " functions imported from " << SrcModName << "\n");
+ LLVM_DEBUG(dbgs() << " - " << NumGVSPerMod << " vars imported from "
+ << SrcModName << "\n");
}
}
#endif
@@ -1246,13 +1149,7 @@ static void ComputeCrossModuleImportForModuleFromIndexForTest(
if (Summary->modulePath() == ModulePath)
continue;
// Add an entry to provoke importing by thinBackend.
- auto [Iter, Inserted] = ImportList[Summary->modulePath()].try_emplace(
- GUID, Summary->importType());
- if (!Inserted) {
- // Use 'std::min' to make sure definition (with enum value 0) takes
- // precedence over declaration (with enum value 1).
- Iter->second = std::min(Iter->second, Summary->importType());
- }
+ ImportList[Summary->modulePath()].insert(GUID);
}
#ifndef NDEBUG
dumpImportListForModule(Index, ModulePath, ImportList);
@@ -1442,17 +1339,13 @@ void llvm::gatherImportedSummariesForModule(
// Include summaries for imports.
for (const auto &ILI : ImportList) {
auto &SummariesForIndex = ModuleToSummariesForIndex[std::string(ILI.first)];
-
const auto &DefinedGVSummaries =
ModuleToDefinedGVSummaries.lookup(ILI.first);
- for (const auto &[GUID, Type] : ILI.second) {
- const auto &DS = DefinedGVSummaries.find(GUID);
+ for (const auto &GI : ILI.second) {
+ const auto &DS = DefinedGVSummaries.find(GI);
assert(DS != DefinedGVSummaries.end() &&
"Expected a defined summary for imported global value");
- if (Type == GlobalValueSummary::Declaration)
- continue;
-
- SummariesForIndex[GUID] = DS->second;
+ SummariesForIndex[GI] = DS->second;
}
}
}
@@ -1724,16 +1617,6 @@ Expected<bool> FunctionImporter::importFunctions(
for (const auto &FunctionsToImportPerModule : ImportList) {
ModuleNameOrderedList.insert(FunctionsToImportPerModule.first);
}
-
- auto getImportType = [&](const FunctionsToImportTy &GUIDToImportType,
- GlobalValue::GUID GUID)
- -> std::optional<GlobalValueSummary::ImportKind> {
- auto Iter = GUIDToImportType.find(GUID);
- if (Iter == GUIDToImportType.end())
- return std::nullopt;
- return Iter->second;
- };
-
for (const auto &Name : ModuleNameOrderedList) {
// Get the module for the import
const auto &FunctionsToImportPerModule = ImportList.find(Name);
@@ -1751,27 +1634,17 @@ Expected<bool> FunctionImporter::importFunctions(
return std::move(Err);
auto &ImportGUIDs = FunctionsToImportPerModule->second;
-
// Find the globals to import
SetVector<GlobalValue *> GlobalsToImport;
for (Function &F : *SrcModule) {
if (!F.hasName())
continue;
auto GUID = F.getGUID();
- auto MaybeImportType = getImportType(ImportGUIDs, GUID);
-
- bool ImportDefinition =
- (MaybeImportType &&
- (*MaybeImportType == GlobalValueSummary::Definition));
-
- LLVM_DEBUG(dbgs() << (MaybeImportType ? "Is" : "Not")
- << " importing function"
- << (ImportDefinition
- ? " definition "
- : (MaybeImportType ? " declaration " : " "))
+ auto Import = ImportGUIDs.count(GUID);
+ LLVM_DEBUG(dbgs() << (Import ? "Is" : "Not") << " importing function "
<< GUID << " " << F.getName() << " from "
<< SrcModule->getSourceFileName() << "\n");
- if (ImportDefinition) {
+ if (Import) {
if (Error Err = F.materialize())
return std::move(Err);
// MemProf should match function's definition and summary,
@@ -1797,20 +1670,11 @@ Expected<bool> FunctionImporter::importFunctions(
if (!GV.hasName())
continue;
auto GUID = GV.getGUID();
- auto MaybeImportType = getImportType(ImportGUIDs, GUID);
-
- bool ImportDefinition =
- (MaybeImportType &&
- (*MaybeImportType == GlobalValueSummary::Definition));
-
- LLVM_DEBUG(dbgs() << (MaybeImportType ? "Is" : "Not")
- << " importing global"
- << (ImportDefinition
- ? " definition "
- : (MaybeImportType ? " declaration " : " "))
+ auto Import = ImportGUIDs.count(GUID);
+ LLVM_DEBUG(dbgs() << (Import ? "Is" : "Not") << " importing global "
<< GUID << " " << GV.getName() << " from "
<< SrcModule->getSourceFileName() << "\n");
- if (ImportDefinition) {
+ if (Import) {
if (Error Err = GV.materialize())
return std::move(Err);
ImportedGVCount += GlobalsToImport.insert(&GV);
@@ -1820,20 +1684,11 @@ Expected<bool> FunctionImporter::importFunctions(
if (!GA.hasName() || isa<GlobalIFunc>(GA.getAliaseeObject()))
continue;
auto GUID = GA.getGUID();
- auto MaybeImportType = getImportType(ImportGUIDs, GUID);
-
- bool ImportDefinition =
- (MaybeImportType &&
- (*MaybeImportType == GlobalValueSummary::Definition));
-
- LLVM_DEBUG(dbgs() << (MaybeImportType ? "Is" : "Not")
- << " importing alias"
- << (ImportDefinition
- ? " definition "
- : (MaybeImportType ? " declaration " : " "))
+ auto Import = ImportGUIDs.count(GUID);
+ LLVM_DEBUG(dbgs() << (Import ? "Is" : "Not") << " importing alias "
<< GUID << " " << GA.getName() << " from "
<< SrcModule->getSourceFileName() << "\n");
- if (ImportDefinition) {
+ if (Import) {
if (Error Err = GA.materialize())
return std::move(Err);
// Import alias as a copy of its aliasee.
@@ -1899,7 +1754,6 @@ Expected<bool> FunctionImporter::importFunctions(
NumImportedFunctions += (ImportedCount - ImportedGVCount);
NumImportedGlobalVars += ImportedGVCount;
- // TODO: Print counters for definitions and declarations in the debugging log.
LLVM_DEBUG(dbgs() << "Imported " << ImportedCount - ImportedGVCount
<< " functions for Module "
<< DestModule.getModuleIdentifier() << "\n");
diff --git a/llvm/test/ThinLTO/X86/funcimport-stats.ll b/llvm/test/ThinLTO/X86/funcimport-stats.ll
index 7fcd33855fe1a..913b13004c1c2 100644
--- a/llvm/test/ThinLTO/X86/funcimport-stats.ll
+++ b/llvm/test/ThinLTO/X86/funcimport-stats.ll
@@ -9,8 +9,8 @@
; RUN: cat %t4 | grep 'Is importing aliasee' | count 1
; RUN: cat %t4 | FileCheck %s
-; CHECK: - [[NUM_FUNCS:[0-9]+]] function definitions and 0 function declarations imported from
-; CHECK-NEXT: - [[NUM_VARS:[0-9]+]] global vars definition and 0 global vars declaration imported from
+; CHECK: - [[NUM_FUNCS:[0-9]+]] functions imported from
+; CHECK-NEXT: - [[NUM_VARS:[0-9]+]] global vars imported from
; CHECK: [[NUM_FUNCS]] function-import - Number of functions imported in backend
; CHECK-NEXT: [[NUM_FUNCS]] function-import - Number of functions thin link decided to import
diff --git a/llvm/test/ThinLTO/X86/import_callee_declaration.ll b/llvm/test/ThinLTO/X86/import_callee_declaration.ll
deleted file mode 100644
index df8a9ce6f7109..0000000000000
--- a/llvm/test/ThinLTO/X86/import_callee_declaration.ll
+++ /dev/null
@@ -1,180 +0,0 @@
-; "-debug-only" requires asserts.
-; REQUIRES: asserts
-; RUN: rm -rf %t && split-file %s %t && cd %t
-
-; Generate per-module summaries.
-; RUN: opt -module-summary main.ll -o main.bc
-; RUN: opt -module-summary lib.ll -o lib.bc
-
-; Generate the combined summary and distributed indices.
-
-; - For function import, set 'import-instr-limit' to 7 and fall back to import
-; function declarations.
-; - In main.ll, function 'main' calls 'small_func' and 'large_func'. Both callees
-; are defined in lib.ll. 'small_func' has two indirect callees, one is smaller
-; and the other one is larger. Both callees of 'small_func' are defined in lib.ll.
-; - Given the import limit, in main's combined summary, the import type of 'small_func'
-; and 'small_indirect_callee' will be 'definition', and the import type of
-; 'large_func' and 'large_indirect_callee' will be 'declaration'.
-;
-; The test will disassemble combined summaries and check the import type is
-; correct. Right now postlink optimizer pipeline doesn't do anything (e.g.,
-; import the declaration or de-serialize summary attributes yet) so there is
-; nothing to test more than the summary content.
-;
-; RUN: llvm-lto2 run \
-; RUN: -debug-only=function-import \
-; RUN: -import-instr-limit=7 \
-; RUN: -import-declaration \
-; RUN: -thinlto-distributed-indexes \
-; RUN: -r=main.bc,main,px \
-; RUN: -r=main.bc,small_func, \
-; RUN: -r=main.bc,large_func, \
-; RUN: -r=lib.bc,callee,pl \
-; RUN: -r=lib.bc,large_indirect_callee,px \
-; RUN: -r=lib.bc,small_func,px \
-; RUN: -r=lib.bc,large_func,px \
-; RUN: -r=lib.bc,large_indirect_callee_alias,px \
-; RUN: -r=lib.bc,calleeAddrs,px -o summary main.bc lib.bc 2>&1 | FileCheck %s --check-prefix=DUMP
-;
-; RUN: llvm-lto -thinlto-action=thinlink -import-declaration -import-instr-limit=7 -o combined.index.bc main.bc lib.bc
-; RUN: llvm-lto -thinlto-action=distributedindexes -debug-only=function-import -import-declaration -import-instr-limit=7 -thinlto-index combined.index.bc main.bc lib.bc 2>&1 | FileCheck %s --check-prefix=DUMP
-
-; DUMP: - 2 function definitions and 3 function declarations imported from lib.bc
-
-; First disassemble per-module summary and find out the GUID for {large_func, large_indirect_callee}.
-;
-; RUN: llvm-dis lib.bc -o - | FileCheck %s --check-prefix=LIB-DIS
-; LIB-DIS: [[LARGEFUNC:\^[0-9]+]] = gv: (name: "large_func", summaries: {{.*}}) ; guid = 2418497564662708935
-; LIB-DIS: [[LARGEINDIRECT:\^[0-9]+]] = gv: (name: "large_indirect_callee", summaries: {{.*}}) ; guid = 14343440786664691134
-; LIB-DIS: [[LARGEINDIRECTALIAS:\^[0-9]+]] = gv: (name: "large_indirect_callee_alias", summaries: {{.*}}, aliasee: [[LARGEINDIRECT]]
-;
-; Secondly disassemble main's combined summary and test that large callees are
-; not imported as declarations yet.
-;
-; RUN: llvm-dis main.bc.thinlto.bc -o - | FileCheck %s --check-prefix=MAIN-DIS
-;
-; MAIN-DIS: [[LIBMOD:\^[0-9]+]] = module: (path: "lib.bc", hash: (0, 0, 0, 0, 0))
-; MAIN-DIS-NOT: [[LARGEFUNC:\^[0-9]+]] = gv: (guid: 2418497564662708935, summaries: (function: (module: [[LIBMOD]], flags: ({{.*}} importType: declaration), insts: 8, {{.*}})))
-; MAIN-DIS-NOT: [[LARGEINDIRECT:\^[0-9]+]] = gv: (guid: 14343440786664691134, summaries: (function: (module: [[LIBMOD]], flags: ({{.*}} importType: declaration), insts: 8, {{.*}})))
-; MAIN-DIS-NOT: [[LARGEINDIRECTALIAS:\^[0-9]+]] = gv: (guid: 16730173943625350469, summaries: (alias: (module: [[LIBMOD]], flags: ({{.*}} importType: declaration)
-
-; Run in-process ThinLTO and tests that
-; 1. `callee` remains internalized even if the symbols of its callers
-; (large_func and large_indirect_callee) are exported as declarations and visible to main module.
-; 2. the debugging logs from `function-import` pass are expected.
-
-; RUN: llvm-lto2 run \
-; RUN: -debug-only=function-import \
-; RUN: -save-temps \
-; RUN: -import-instr-limit=7 \
-; RUN: -import-declaration \
-; RUN: -r=main.bc,main,px \
-; RUN: -r=main.bc,small_func, \
-; RUN: -r=main.bc,large_func, \
-; RUN: -r=lib.bc,callee,pl \
-; RUN: -r=lib.bc,large_indirect_callee,px \
-; RUN: -r=lib.bc,small_func,px \
-; RUN: -r=lib.bc,large_func,px \
-; RUN: -r=lib.bc,large_indirect_callee_alias,px \
-; RUN: -r=lib.bc,calleeAddrs,px -o in-process main.bc lib.bc 2>&1 | FileCheck %s --check-prefix=IMPORTDUMP
-
-; Test import status from debugging logs.
-; TODO: Serialize declaration bit and test declaration bits are correctly set,
-; and extend this test case to test IR once postlink optimizer makes use of
-; the import type for declarations.
-; IMPORTDUMP-DAG: Not importing function 11825436545918268459 callee from lib.cc
-; IMPORTDUMP-DAG: Is importing function declaration 14343440786664691134 large_indirect_callee from lib.cc
-; IMPORTDUMP-DAG: Is importing function definition 13568239288960714650 small_indirect_callee from lib.cc
-; IMPORTDUMP-DAG: Is importing function definition 6976996067367342685 small_func from lib.cc
-; IMPORTDUMP-DAG: Is importing function declaration 2418497564662708935 large_func from lib.cc
-; IMPORTDUMP-DAG: Not importing global 7680325410415171624 calleeAddrs from lib.cc
-; IMPORTDUMP-DAG: Is importing alias declaration 16730173943625350469 large_indirect_callee_alias from lib.cc
-
-; RUN: llvm-dis in-process.1.3.import.bc -o - | FileCheck %s --check-prefix=IMPORT
-
-; RUN: llvm-dis in-process.2.2.internalize.bc -o - | FileCheck %s --check-prefix=INTERNALIZE
-
-; IMPORT-DAG: define available_externally void @small_func
-; IMPORT-DAG: define available_externally hidden void @small_indirect_callee
-; IMPORT-DAG: declare void @large_func
-; IMPORT-NOT: large_indirect_callee
-; IMPORT-NOT: large_indirect_callee_alias
-
-; INTERNALIZE: define internal void @callee()
-
-;--- main.ll
-target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-unknown-linux-gnu"
-
-define i32 @main() {
- call void @small_func()
- call void @large_func()
- ret i32 0
-}
-
-declare void @small_func()
-
-; large_func without attributes
-declare void @large_func()
-
-;--- lib.ll
-source_filename = "lib.cc"
-target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-unknown-linux-gnu"
-
- at calleeAddrs = global [3 x ptr] [ptr @large_indirect_callee, ptr @small_indirect_callee, ptr @large_indirect_callee_alias]
-
-define void @callee() #1 {
- ret void
-}
-
-define void @large_indirect_callee()#2 {
- call void @callee()
- call void @callee()
- call void @callee()
- call void @callee()
- call void @callee()
- call void @callee()
- call void @callee()
- ret void
-}
-
-define internal void @small_indirect_callee() #0 {
- ret void
-}
-
- at large_indirect_callee_alias = alias void(), ptr @large_indirect_callee
-
-define void @small_func() {
-entry:
- %0 = load ptr, ptr @calleeAddrs
- call void %0(), !prof !0
- %1 = load ptr, ptr getelementptr inbounds ([3 x ptr], ptr @calleeAddrs, i64 0, i64 1)
- call void %1(), !prof !1
- %2 = load ptr, ptr getelementptr inbounds ([3 x ptr], ptr @calleeAddrs, i64 0, i64 2)
- call void %2(), !prof !2
- ret void
-}
-
-define void @large_func() #0 {
-entry:
- call void @callee()
- call void @callee()
- call void @callee()
- call void @callee()
- call void @callee()
- call void @callee()
- call void @callee()
- ret void
-}
-
-attributes #0 = { nounwind norecurse }
-
-attributes #1 = { noinline }
-
-attributes #2 = { norecurse }
-
-!0 = !{!"VP", i32 0, i64 1, i64 14343440786664691134, i64 1}
-!1 = !{!"VP", i32 0, i64 1, i64 13568239288960714650, i64 1}
-!2 = !{!"VP", i32 0, i64 1, i64 16730173943625350469, i64 1}
diff --git a/llvm/test/Transforms/FunctionImport/funcimport.ll b/llvm/test/Transforms/FunctionImport/funcimport.ll
index 635750b33fff0..a0968a67f5ce8 100644
--- a/llvm/test/Transforms/FunctionImport/funcimport.ll
+++ b/llvm/test/Transforms/FunctionImport/funcimport.ll
@@ -166,8 +166,7 @@ declare void @variadic_va_start(...)
; GUID-DAG: GUID {{.*}} is linkoncefunc
; DUMP: Module [[M1:.*]] imports from 1 module
-; DUMP-NEXT: 15 function definitions and 0 function declarations imported from [[M2:.*]]
-; DUMP-NEXT: 4 var definitions and 0 var declarations imported from [[M2]]
-
+; DUMP-NEXT: 15 functions imported from [[M2:.*]]
+; DUMP-NEXT: 4 vars imported from [[M2]]
; DUMP: Imported 15 functions for Module [[M1]]
; DUMP-NEXT: Imported 4 global variables for Module [[M1]]
diff --git a/llvm/tools/llvm-link/llvm-link.cpp b/llvm/tools/llvm-link/llvm-link.cpp
index 1b90fce76fbd1..7794f2d81ed06 100644
--- a/llvm/tools/llvm-link/llvm-link.cpp
+++ b/llvm/tools/llvm-link/llvm-link.cpp
@@ -377,13 +377,9 @@ static bool importFunctions(const char *argv0, Module &DestModule) {
if (Verbose)
errs() << "Importing " << FunctionName << " from " << FileName << "\n";
- // `-import` specifies the `<filename,function-name>` pairs to import as
- // definition, so make the import type definition directly.
- // FIXME: A follow-up patch should add test coverage for import declaration
- // in `llvm-link` CLI (e.g., by introducing a new command line option).
auto &Entry =
ImportList[FileNameStringCache.insert(FileName).first->getKey()];
- Entry[F->getGUID()] = GlobalValueSummary::Definition;
+ Entry.insert(F->getGUID());
}
auto CachedModuleLoader = [&](StringRef Identifier) {
return ModuleLoaderCache.takeModule(std::string(Identifier));
More information about the llvm-branch-commits
mailing list