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" - -@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 -} - -@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)); _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
