From 9fd063832c1541aad3907cd60ac344d36997905f Mon Sep 17 00:00:00 2001 From: Teresa Johnson Date: Sun, 30 Oct 2016 05:40:44 +0000 Subject: [PATCH] [ThinLTO] Use per-summary flag to prevent exporting locals used in inline asm Summary: Instead of using the workaround of suppressing the entire index for modules that call inline asm that may reference locals, use the NoRename flag on the summary for any locals in the llvm.used set, and add a reference edge from any functions containing inline asm. This avoids issues from having no summaries despite the module defining global values, which was preventing more aggressive index-based optimization. It will be followed by a subsequent patch to make a similar fix for local references in module level asm (to fix PR30610). Reviewers: mehdi_amini Subscribers: llvm-commits Differential Revision: https://reviews.llvm.org/D26121 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@285513 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Analysis/ModuleSummaryAnalysis.h | 5 -- include/llvm/IR/ModuleSummaryIndex.h | 4 + lib/Analysis/ModuleSummaryAnalysis.cpp | 76 ++++++++----------- lib/Transforms/Utils/FunctionImportUtils.cpp | 8 -- 4 files changed, 34 insertions(+), 59 deletions(-) diff --git a/include/llvm/Analysis/ModuleSummaryAnalysis.h b/include/llvm/Analysis/ModuleSummaryAnalysis.h index c8adc3bf81a..617c3d8f17a 100644 --- a/include/llvm/Analysis/ModuleSummaryAnalysis.h +++ b/include/llvm/Analysis/ModuleSummaryAnalysis.h @@ -70,11 +70,6 @@ public: // object for the module, to be written to bitcode or LLVM assembly. // ModulePass *createModuleSummaryIndexWrapperPass(); - -/// Returns true if \p M is eligible for ThinLTO promotion. -/// -/// Currently we check if it has any any InlineASM that uses an internal symbol. -bool moduleCanBeRenamedForThinLTO(const Module &M); } #endif diff --git a/include/llvm/IR/ModuleSummaryIndex.h b/include/llvm/IR/ModuleSummaryIndex.h index 1ceb9eb32cb..b3775904e75 100644 --- a/include/llvm/IR/ModuleSummaryIndex.h +++ b/include/llvm/IR/ModuleSummaryIndex.h @@ -194,6 +194,10 @@ public: /// possibly referenced from inline assembly, etc). bool noRename() const { return Flags.NoRename; } + /// Flag that this global value cannot be renamed (in a specific section, + /// possibly referenced from inline assembly, etc). + void setNoRename() { Flags.NoRename = true; } + /// Record a reference from this global value to the global value identified /// by \p RefGUID. void addRefEdge(GlobalValue::GUID RefGUID) { RefEdgeList.push_back(RefGUID); } diff --git a/lib/Analysis/ModuleSummaryAnalysis.cpp b/lib/Analysis/ModuleSummaryAnalysis.cpp index f13ae844442..09d18eb3da1 100644 --- a/lib/Analysis/ModuleSummaryAnalysis.cpp +++ b/lib/Analysis/ModuleSummaryAnalysis.cpp @@ -77,7 +77,8 @@ static CalleeInfo::HotnessType getHotness(uint64_t ProfileCount, static void computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M, const Function &F, BlockFrequencyInfo *BFI, - ProfileSummaryInfo *PSI) { + ProfileSummaryInfo *PSI, + SmallPtrSetImpl &LocalsUsed) { // Summary not currently supported for anonymous functions, they should // have been named. assert(F.hasName()); @@ -89,6 +90,7 @@ static void computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M, DenseMap IndirectCallEdges; DenseSet RefEdges; ICallPromotionAnalysis ICallAnalysis; + bool HasLocalsInUsed = !LocalsUsed.empty(); SmallPtrSet Visited; for (const BasicBlock &BB : F) @@ -100,6 +102,15 @@ static void computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M, auto CS = ImmutableCallSite(&I); if (!CS) continue; + + const auto *CI = dyn_cast(&I); + // Since we don't know exactly which local values are referenced in inline + // assembly, conservatively reference all of them from this function, to + // ensure we don't export a reference (which would require renaming and + // promotion). + if (HasLocalsInUsed && CI && CI->isInlineAsm()) + RefEdges.insert(LocalsUsed.begin(), LocalsUsed.end()); + auto *CalledValue = CS.getCalledValue(); auto *CalledFunction = CS.getCalledFunction(); // Check if this is an alias to a function. If so, get the @@ -127,7 +138,6 @@ static void computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M, : CalleeInfo::HotnessType::Unknown; CallGraphEdges[CalleeId].updateHotness(Hotness); } else { - const auto *CI = dyn_cast(&I); // Skip inline assembly calls. if (CI && CI->isInlineAsm()) continue; @@ -183,11 +193,17 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex( std::function GetBFICallback, ProfileSummaryInfo *PSI) { ModuleSummaryIndex Index; - // Check if the module can be promoted, otherwise just disable importing from - // it by not emitting any summary. - // FIXME: we could still import *into* it most of the time. - if (!moduleCanBeRenamedForThinLTO(M)) - return Index; + + // Identify the local values in the llvm.used set, which should not be + // exported as they would then require renaming and promotion, but we + // may have opaque uses e.g. in inline asm. + SmallPtrSet Used; + collectUsedGlobalVariables(M, Used, /*CompilerUsed*/ false); + SmallPtrSet LocalsUsed; + for (auto *V : Used) { + if (V->hasLocalLinkage()) + LocalsUsed.insert(V); + } // Compute summaries for all functions defined in module, and save in the // index. @@ -206,7 +222,7 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex( BFI = BFIPtr.get(); } - computeFunctionSummary(Index, M, F, BFI, PSI); + computeFunctionSummary(Index, M, F, BFI, PSI, LocalsUsed); } // Compute summaries for all variables defined in module, and save in the @@ -222,6 +238,12 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex( for (const GlobalAlias &A : M.aliases()) computeAliasSummary(Index, A); + for (auto *V : LocalsUsed) { + auto *Summary = Index.getGlobalValueSummary(*V); + assert(Summary && "Missing summary for global value"); + Summary->setNoRename(); + } + return Index; } @@ -279,41 +301,3 @@ void ModuleSummaryIndexWrapperPass::getAnalysisUsage(AnalysisUsage &AU) const { AU.addRequired(); AU.addRequired(); } - -bool llvm::moduleCanBeRenamedForThinLTO(const Module &M) { - // We cannot currently promote or rename anything used in inline assembly, - // which are not visible to the compiler. Detect a possible case by looking - // for a llvm.used local value, in conjunction with an inline assembly call - // in the module. Prevent importing of any modules containing these uses by - // suppressing generation of the index. This also prevents importing - // into this module, which is also necessary to avoid needing to rename - // in case of a name clash between a local in this module and an imported - // global. - // FIXME: If we find we need a finer-grained approach of preventing promotion - // and renaming of just the functions using inline assembly we will need to: - // - Add flag in the function summaries to identify those with inline asm. - // - Prevent importing of any functions with flag set. - // - Prevent importing of any global function with the same name as a - // function in current module that has the flag set. - // - For any llvm.used value that is exported and promoted, add a private - // alias to the original name in the current module (even if we don't - // export the function using those values in inline asm, another function - // with a reference could be exported). - SmallPtrSet Used; - collectUsedGlobalVariables(M, Used, /*CompilerUsed*/ false); - bool LocalIsUsed = - any_of(Used, [](GlobalValue *V) { return V->hasLocalLinkage(); }); - if (!LocalIsUsed) - return true; - - // Walk all the instructions in the module and find if one is inline ASM - auto HasInlineAsm = any_of(M, [](const Function &F) { - return any_of(instructions(F), [](const Instruction &I) { - const CallInst *CallI = dyn_cast(&I); - if (!CallI) - return false; - return CallI->isInlineAsm(); - }); - }); - return !HasInlineAsm; -} diff --git a/lib/Transforms/Utils/FunctionImportUtils.cpp b/lib/Transforms/Utils/FunctionImportUtils.cpp index d14920575a4..b9b3606ecfb 100644 --- a/lib/Transforms/Utils/FunctionImportUtils.cpp +++ b/lib/Transforms/Utils/FunctionImportUtils.cpp @@ -220,14 +220,6 @@ void FunctionImportGlobalProcessing::processGlobalForThinLTO(GlobalValue &GV) { } void FunctionImportGlobalProcessing::processGlobalsForThinLTO() { - if (!moduleCanBeRenamedForThinLTO(M)) { - // We would have blocked importing from this module by suppressing index - // generation. We still may be able to import into this module though. - assert(!isPerformingImport() && - "Should have blocked importing from module with local used in ASM"); - return; - } - for (GlobalVariable &GV : M.globals()) processGlobalForThinLTO(GV); for (Function &SF : M) -- 2.49.0