diff --git a/include/llvm/LTO/LTO.h b/include/llvm/LTO/LTO.h index 0a1e3e1d0e4..42c9d5b78c0 100644 --- a/include/llvm/LTO/LTO.h +++ b/include/llvm/LTO/LTO.h @@ -59,7 +59,7 @@ void thinLTOResolvePrevailingInIndex( /// must apply the changes to the Module via thinLTOInternalizeModule. void thinLTOInternalizeAndPromoteInIndex( ModuleSummaryIndex &Index, - function_ref isExported, + function_ref isExported, function_ref isPrevailing); diff --git a/include/llvm/Transforms/IPO/FunctionImport.h b/include/llvm/Transforms/IPO/FunctionImport.h index bbf270c400a..b4dde7b199f 100644 --- a/include/llvm/Transforms/IPO/FunctionImport.h +++ b/include/llvm/Transforms/IPO/FunctionImport.h @@ -98,7 +98,7 @@ public: using ImportMapTy = StringMap; /// The set contains an entry for every global value the module exports. - using ExportSetTy = std::unordered_set; + using ExportSetTy = DenseSet; /// A function of this type is used to load modules referenced by the index. using ModuleLoaderTy = diff --git a/include/llvm/Transforms/IPO/WholeProgramDevirt.h b/include/llvm/Transforms/IPO/WholeProgramDevirt.h index 22435e4ed1e..8af2af7f352 100644 --- a/include/llvm/Transforms/IPO/WholeProgramDevirt.h +++ b/include/llvm/Transforms/IPO/WholeProgramDevirt.h @@ -251,7 +251,7 @@ void runWholeProgramDevirtOnIndex( /// devirt target names for any locals that were exported. void updateIndexWPDForExports( ModuleSummaryIndex &Summary, - function_ref isExported, + function_ref isExported, std::map> &LocalWPDTargetsMap); } // end namespace llvm diff --git a/lib/LTO/LTO.cpp b/lib/LTO/LTO.cpp index 1e345e7dd89..9c59ec40274 100644 --- a/lib/LTO/LTO.cpp +++ b/lib/LTO/LTO.cpp @@ -146,9 +146,11 @@ void llvm::computeLTOCacheKey( // Include the hash for the current module auto ModHash = Index.getModuleHash(ModuleID); Hasher.update(ArrayRef((uint8_t *)&ModHash[0], sizeof(ModHash))); - for (auto F : ExportList) + for (const auto &VI : ExportList) { + auto GUID = VI.getGUID(); // The export list can impact the internalization, be conservative here - Hasher.update(ArrayRef((uint8_t *)&F, sizeof(F))); + Hasher.update(ArrayRef((uint8_t *)&GUID, sizeof(GUID))); + } // Include the hash for every module we import functions from. The set of // imported symbols for each module may affect code generation and is @@ -383,12 +385,12 @@ static bool isWeakObjectWithRWAccess(GlobalValueSummary *GVS) { } static void thinLTOInternalizeAndPromoteGUID( - GlobalValueSummaryList &GVSummaryList, GlobalValue::GUID GUID, - function_ref isExported, + GlobalValueSummaryList &GVSummaryList, ValueInfo VI, + function_ref isExported, function_ref isPrevailing) { for (auto &S : GVSummaryList) { - if (isExported(S->modulePath(), GUID)) { + if (isExported(S->modulePath(), VI)) { if (GlobalValue::isLocalLinkage(S->linkage())) S->setLinkage(GlobalValue::ExternalLinkage); } else if (EnableLTOInternalization && @@ -396,7 +398,7 @@ static void thinLTOInternalizeAndPromoteGUID( // doesn't resolve them. !GlobalValue::isLocalLinkage(S->linkage()) && (!GlobalValue::isInterposableLinkage(S->linkage()) || - isPrevailing(GUID, S.get())) && + isPrevailing(VI.getGUID(), S.get())) && S->linkage() != GlobalValue::AppendingLinkage && // We can't internalize available_externally globals because this // can break function pointer equality. @@ -415,12 +417,12 @@ static void thinLTOInternalizeAndPromoteGUID( // as external and non-exported values as internal. void llvm::thinLTOInternalizeAndPromoteInIndex( ModuleSummaryIndex &Index, - function_ref isExported, + function_ref isExported, function_ref isPrevailing) { for (auto &I : Index) - thinLTOInternalizeAndPromoteGUID(I.second.SummaryList, I.first, isExported, - isPrevailing); + thinLTOInternalizeAndPromoteGUID( + I.second.SummaryList, Index.getValueInfo(I), isExported, isPrevailing); } // Requires a destructor for std::vector. @@ -1330,11 +1332,10 @@ Error LTO::runThinLTO(AddStreamFn AddStream, NativeObjectCache Cache, ExportedGUIDs.insert( GlobalValue::getGUID(GlobalValue::dropLLVMManglingEscape(Def))); - auto isExported = [&](StringRef ModuleIdentifier, GlobalValue::GUID GUID) { + auto isExported = [&](StringRef ModuleIdentifier, ValueInfo VI) { const auto &ExportList = ExportLists.find(ModuleIdentifier); - return (ExportList != ExportLists.end() && - ExportList->second.count(GUID)) || - ExportedGUIDs.count(GUID); + return (ExportList != ExportLists.end() && ExportList->second.count(VI)) || + ExportedGUIDs.count(VI.getGUID()); }; // Update local devirtualized targets that were exported by cross-module diff --git a/lib/LTO/ThinLTOCodeGenerator.cpp b/lib/LTO/ThinLTOCodeGenerator.cpp index 300203bcc1c..02dda2ab7ac 100644 --- a/lib/LTO/ThinLTOCodeGenerator.cpp +++ b/lib/LTO/ThinLTOCodeGenerator.cpp @@ -589,11 +589,10 @@ struct IsExported { const DenseSet &GUIDPreservedSymbols) : ExportLists(ExportLists), GUIDPreservedSymbols(GUIDPreservedSymbols) {} - bool operator()(StringRef ModuleIdentifier, GlobalValue::GUID GUID) const { + bool operator()(StringRef ModuleIdentifier, ValueInfo VI) const { const auto &ExportList = ExportLists.find(ModuleIdentifier); - return (ExportList != ExportLists.end() && - ExportList->second.count(GUID)) || - GUIDPreservedSymbols.count(GUID); + return (ExportList != ExportLists.end() && ExportList->second.count(VI)) || + GUIDPreservedSymbols.count(VI.getGUID()); } }; diff --git a/lib/Transforms/IPO/FunctionImport.cpp b/lib/Transforms/IPO/FunctionImport.cpp index b22cf47a797..d5419e1a1b9 100644 --- a/lib/Transforms/IPO/FunctionImport.cpp +++ b/lib/Transforms/IPO/FunctionImport.cpp @@ -308,7 +308,7 @@ static void computeImportForReferencedGlobals( auto MarkExported = [&](const ValueInfo &VI, const GlobalValueSummary *S) { if (ExportLists) - (*ExportLists)[S->modulePath()].insert(VI.getGUID()); + (*ExportLists)[S->modulePath()].insert(VI); }; for (auto &RefSummary : VI.getSummaryList()) @@ -497,7 +497,7 @@ static void computeImportForFunction( // Make exports in the source module. if (ExportLists) { auto &ExportList = (*ExportLists)[ExportModulePath]; - ExportList.insert(VI.getGUID()); + ExportList.insert(VI); if (!PreviouslyImported) { // This is the first time this function was exported from its source // module, so mark all functions and globals it references as exported @@ -505,14 +505,11 @@ static void computeImportForFunction( // For efficiency, we unconditionally add all the referenced GUIDs // to the ExportList for this module, and will prune out any not // defined in the module later in a single pass. - for (auto &Edge : ResolvedCalleeSummary->calls()) { - auto CalleeGUID = Edge.first.getGUID(); - ExportList.insert(CalleeGUID); - } - for (auto &Ref : ResolvedCalleeSummary->refs()) { - auto GUID = Ref.getGUID(); - ExportList.insert(GUID); - } + for (auto &Edge : ResolvedCalleeSummary->calls()) + ExportList.insert(Edge.first); + + for (auto &Ref : ResolvedCalleeSummary->refs()) + ExportList.insert(Ref); } } } @@ -630,6 +627,37 @@ static unsigned numGlobalVarSummaries(const ModuleSummaryIndex &Index, } #endif +static bool +checkVariableImport(const ModuleSummaryIndex &Index, + StringMap &ImportLists, + StringMap &ExportLists) { + + DenseSet FlattenedImports; + + for (auto &ImportPerModule : ImportLists) + for (auto &ExportPerModule : ImportPerModule.second) + 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, + // because readonly and writeonly vars are internalized in their + // source modules. + auto IsReadOrWriteOnlyVar = [&](StringRef ModulePath, const ValueInfo &VI) { + auto *GVS = dyn_cast_or_null( + Index.findSummaryInModule(VI, ModulePath)); + return GVS && (Index.isReadOnly(GVS) || Index.isWriteOnly(GVS)); + }; + + for (auto &ExportPerModule : ExportLists) + for (auto &VI : ExportPerModule.second) + if (!FlattenedImports.count(VI.getGUID()) && + IsReadOrWriteOnlyVar(ExportPerModule.first(), VI)) + return false; + + return true; +} + /// Compute all the import and export for every module using the Index. void llvm::ComputeCrossModuleImport( const ModuleSummaryIndex &Index, @@ -654,14 +682,13 @@ void llvm::ComputeCrossModuleImport( for (auto &ELI : ExportLists) { const auto &DefinedGVSummaries = ModuleToDefinedGVSummaries.lookup(ELI.first()); - for (auto EI = ELI.second.begin(); EI != ELI.second.end();) { - if (!DefinedGVSummaries.count(*EI)) - EI = ELI.second.erase(EI); - else - ++EI; - } + std::remove_if(ELI.second.begin(), ELI.second.end(), + [&](const ValueInfo &VI) { + return !DefinedGVSummaries.count(VI.getGUID()); + }); } + assert(checkVariableImport(Index, ImportLists, ExportLists)); #ifndef NDEBUG LLVM_DEBUG(dbgs() << "Import/Export lists for " << ImportLists.size() << " modules:\n"); diff --git a/lib/Transforms/IPO/WholeProgramDevirt.cpp b/lib/Transforms/IPO/WholeProgramDevirt.cpp index c667927d17d..a00003762ef 100644 --- a/lib/Transforms/IPO/WholeProgramDevirt.cpp +++ b/lib/Transforms/IPO/WholeProgramDevirt.cpp @@ -710,7 +710,7 @@ void runWholeProgramDevirtOnIndex( void updateIndexWPDForExports( ModuleSummaryIndex &Summary, - function_ref isExported, + function_ref isExported, std::map> &LocalWPDTargetsMap) { for (auto &T : LocalWPDTargetsMap) { auto &VI = T.first; @@ -718,7 +718,7 @@ void updateIndexWPDForExports( assert(VI.getSummaryList().size() == 1 && "Devirt of local target has more than one copy"); auto &S = VI.getSummaryList()[0]; - if (!isExported(S->modulePath(), VI.getGUID())) + if (!isExported(S->modulePath(), VI)) continue; // It's been exported by a cross module import. diff --git a/lib/Transforms/Utils/FunctionImportUtils.cpp b/lib/Transforms/Utils/FunctionImportUtils.cpp index 401be682043..d795a0a2070 100644 --- a/lib/Transforms/Utils/FunctionImportUtils.cpp +++ b/lib/Transforms/Utils/FunctionImportUtils.cpp @@ -250,12 +250,12 @@ void FunctionImportGlobalProcessing::processGlobalForThinLTO(GlobalValue &GV) { // contains summaries from the source modules if they are being imported. // We might have a non-null VI and get here even in that case if the name // matches one in this module (e.g. weak or appending linkage). - auto* GVS = dyn_cast_or_null( - ImportIndex.findSummaryInModule(VI, M.getModuleIdentifier())); + auto *GVS = dyn_cast_or_null( + ImportIndex.findSummaryInModule(VI, M.getModuleIdentifier())); if (GVS && (ImportIndex.isReadOnly(GVS) || ImportIndex.isWriteOnly(GVS))) { V->addAttribute("thinlto-internalize"); - // Objects referenced by writeonly GV initializer should not be + // Objects referenced by writeonly GV initializer should not be // promoted, because there is no any kind of read access to them // on behalf of this writeonly GV. To avoid promotion we convert // GV initializer to 'zeroinitializer'. This effectively drops