From: Teresa Johnson Date: Mon, 14 Nov 2016 16:40:19 +0000 (+0000) Subject: [ThinLTO] Make inline assembly handling more efficient in summary X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=101d620a14be2253c442ba3df826d3430ed62585;p=llvm [ThinLTO] Make inline assembly handling more efficient in summary Summary: The change in r285513 to prevent exporting of locals used in inline asm added all locals in the llvm.used set to the reference set of functions containing inline asm. Since these locals were marked NoRename, this automatically prevented importing of the function. Unfortunately, this caused an explosion in the summary reference lists in some cases. In my particular example, it happened for a large protocol buffer generated C++ file, where many of the generated functions contained an inline asm call. It was exacerbated when doing a ThinLTO PGO instrumentation build, where the PGO instrumentation included thousands of private __profd_* values that were added to llvm.used. We really only need to include a single llvm.used local (NoRename) value in the reference list of a function containing inline asm to block it being imported. However, it seems cleaner to add a flag to the summary that explicitly describes this situation, which is what this patch does. Reviewers: mehdi_amini Subscribers: llvm-commits Differential Revision: https://reviews.llvm.org/D26402 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@286840 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/IR/ModuleSummaryIndex.h b/include/llvm/IR/ModuleSummaryIndex.h index b3775904e75..97875b6de58 100644 --- a/include/llvm/IR/ModuleSummaryIndex.h +++ b/include/llvm/IR/ModuleSummaryIndex.h @@ -101,17 +101,29 @@ public: /// possibly referenced from inline assembly, etc). unsigned NoRename : 1; + /// Indicate if a function contains inline assembly (which is opaque), + /// that may reference a local value. This is used to prevent importing + /// of this function, since we can't promote and rename the uses of the + /// local in the inline assembly. Use a flag rather than bloating the + /// summary with references to every possible local value in the + /// llvm.used set. + unsigned HasInlineAsmMaybeReferencingInternal : 1; + /// Indicate if the function is not viable to inline. unsigned IsNotViableToInline : 1; /// Convenience Constructors explicit GVFlags(GlobalValue::LinkageTypes Linkage, bool NoRename, + bool HasInlineAsmMaybeReferencingInternal, bool IsNotViableToInline) : Linkage(Linkage), NoRename(NoRename), + HasInlineAsmMaybeReferencingInternal( + HasInlineAsmMaybeReferencingInternal), IsNotViableToInline(IsNotViableToInline) {} GVFlags(const GlobalValue &GV) - : Linkage(GV.getLinkage()), NoRename(GV.hasSection()) { + : Linkage(GV.getLinkage()), NoRename(GV.hasSection()), + HasInlineAsmMaybeReferencingInternal(false) { IsNotViableToInline = false; if (const auto *F = dyn_cast(&GV)) // Inliner doesn't handle variadic functions. @@ -198,6 +210,18 @@ public: /// possibly referenced from inline assembly, etc). void setNoRename() { Flags.NoRename = true; } + /// Return true if this global value possibly references another value + /// that can't be renamed. + bool hasInlineAsmMaybeReferencingInternal() const { + return Flags.HasInlineAsmMaybeReferencingInternal; + } + + /// Flag that this global value possibly references another value that + /// can't be renamed. + void setHasInlineAsmMaybeReferencingInternal() { + Flags.HasInlineAsmMaybeReferencingInternal = 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 a3f43071b44..0c07e1d878f 100644 --- a/lib/Analysis/ModuleSummaryAnalysis.cpp +++ b/lib/Analysis/ModuleSummaryAnalysis.cpp @@ -78,7 +78,7 @@ static CalleeInfo::HotnessType getHotness(uint64_t ProfileCount, static void computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M, const Function &F, BlockFrequencyInfo *BFI, ProfileSummaryInfo *PSI, - SmallPtrSetImpl &LocalsUsed) { + bool HasLocalsInUsed) { // Summary not currently supported for anonymous functions, they should // have been named. assert(F.hasName()); @@ -90,8 +90,8 @@ static void computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M, DenseMap IndirectCallEdges; DenseSet RefEdges; ICallPromotionAnalysis ICallAnalysis; - bool HasLocalsInUsed = !LocalsUsed.empty(); + bool HasInlineAsmMaybeReferencingInternal = false; SmallPtrSet Visited; for (const BasicBlock &BB : F) for (const Instruction &I : BB) { @@ -105,11 +105,12 @@ static void computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M, 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). + // assembly, conservatively mark the function as possibly referencing + // a local value from inline assembly to ensure we don't export a + // reference (which would require renaming and promotion of the + // referenced value). if (HasLocalsInUsed && CI && CI->isInlineAsm()) - RefEdges.insert(LocalsUsed.begin(), LocalsUsed.end()); + HasInlineAsmMaybeReferencingInternal = true; auto *CalledValue = CS.getCalledValue(); auto *CalledFunction = CS.getCalledFunction(); @@ -162,6 +163,8 @@ static void computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M, FuncSummary->addCallGraphEdges(CallGraphEdges); FuncSummary->addCallGraphEdges(IndirectCallEdges); FuncSummary->addRefEdges(RefEdges); + if (HasInlineAsmMaybeReferencingInternal) + FuncSummary->setHasInlineAsmMaybeReferencingInternal(); Index.addGlobalValueSummary(F.getName(), std::move(FuncSummary)); } @@ -232,7 +235,7 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex( BFI = BFIPtr.get(); } - computeFunctionSummary(Index, M, F, BFI, PSI, LocalsUsed); + computeFunctionSummary(Index, M, F, BFI, PSI, !LocalsUsed.empty()); } // Compute summaries for all variables defined in module, and save in the diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp index bf1f6a56fa1..94a3bcf8554 100644 --- a/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/lib/Bitcode/Reader/BitcodeReader.cpp @@ -946,7 +946,10 @@ static GlobalValueSummary::GVFlags getDecodedGVSummaryFlags(uint64_t RawFlags, RawFlags = RawFlags >> 4; bool NoRename = RawFlags & 0x1; bool IsNotViableToInline = RawFlags & 0x2; - return GlobalValueSummary::GVFlags(Linkage, NoRename, IsNotViableToInline); + bool HasInlineAsmMaybeReferencingInternal = RawFlags & 0x4; + return GlobalValueSummary::GVFlags(Linkage, NoRename, + HasInlineAsmMaybeReferencingInternal, + IsNotViableToInline); } static GlobalValue::VisibilityTypes getDecodedVisibility(unsigned Val) { diff --git a/lib/Bitcode/Writer/BitcodeWriter.cpp b/lib/Bitcode/Writer/BitcodeWriter.cpp index 288c7c6fac2..80db5b2e061 100644 --- a/lib/Bitcode/Writer/BitcodeWriter.cpp +++ b/lib/Bitcode/Writer/BitcodeWriter.cpp @@ -992,6 +992,7 @@ static uint64_t getEncodedGVSummaryFlags(GlobalValueSummary::GVFlags Flags) { RawFlags |= Flags.NoRename; // bool RawFlags |= (Flags.IsNotViableToInline << 1); + RawFlags |= (Flags.HasInlineAsmMaybeReferencingInternal << 2); // Linkage don't need to be remapped at that time for the summary. Any future // change to the getEncodedLinkage() function will need to be taken into // account here as well. diff --git a/lib/Transforms/IPO/FunctionImport.cpp b/lib/Transforms/IPO/FunctionImport.cpp index 3bebc609bea..68626c02038 100644 --- a/lib/Transforms/IPO/FunctionImport.cpp +++ b/lib/Transforms/IPO/FunctionImport.cpp @@ -153,7 +153,11 @@ static bool eligibleForImport(const ModuleSummaryIndex &Index, // Check references (and potential calls) in the same module. If the current // value references a global that can't be externally referenced it is not - // eligible for import. + // eligible for import. First check the flag set when we have possible + // opaque references (e.g. inline asm calls), then check the call and + // reference sets. + if (Summary.hasInlineAsmMaybeReferencingInternal()) + return false; bool AllRefsCanBeExternallyReferenced = llvm::all_of(Summary.refs(), [&](const ValueInfo &VI) { return canBeExternallyReferenced(Index, VI.getGUID());