[llvm] r286840 - [ThinLTO] Make inline assembly handling more efficient in summary
Teresa Johnson via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 14 08:40:20 PST 2016
Author: tejohnson
Date: Mon Nov 14 10:40:19 2016
New Revision: 286840
URL: http://llvm.org/viewvc/llvm-project?rev=286840&view=rev
Log:
[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
Modified:
llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h
llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp
llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp
llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp
Modified: llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h?rev=286840&r1=286839&r2=286840&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h (original)
+++ llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h Mon Nov 14 10:40:19 2016
@@ -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<Function>(&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); }
Modified: llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp?rev=286840&r1=286839&r2=286840&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp (original)
+++ llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp Mon Nov 14 10:40:19 2016
@@ -78,7 +78,7 @@ static CalleeInfo::HotnessType getHotnes
static void computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M,
const Function &F, BlockFrequencyInfo *BFI,
ProfileSummaryInfo *PSI,
- SmallPtrSetImpl<GlobalValue *> &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(Modul
DenseMap<GlobalValue::GUID, CalleeInfo> IndirectCallEdges;
DenseSet<const Value *> RefEdges;
ICallPromotionAnalysis ICallAnalysis;
- bool HasLocalsInUsed = !LocalsUsed.empty();
+ bool HasInlineAsmMaybeReferencingInternal = false;
SmallPtrSet<const User *, 8> Visited;
for (const BasicBlock &BB : F)
for (const Instruction &I : BB) {
@@ -105,11 +105,12 @@ static void computeFunctionSummary(Modul
const auto *CI = dyn_cast<CallInst>(&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(Modul
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::buildModuleSumm
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
Modified: llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp?rev=286840&r1=286839&r2=286840&view=diff
==============================================================================
--- llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp (original)
+++ llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp Mon Nov 14 10:40:19 2016
@@ -946,7 +946,10 @@ static GlobalValueSummary::GVFlags getDe
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) {
Modified: llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp?rev=286840&r1=286839&r2=286840&view=diff
==============================================================================
--- llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp (original)
+++ llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp Mon Nov 14 10:40:19 2016
@@ -992,6 +992,7 @@ static uint64_t getEncodedGVSummaryFlags
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.
Modified: llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp?rev=286840&r1=286839&r2=286840&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp Mon Nov 14 10:40:19 2016
@@ -153,7 +153,11 @@ static bool eligibleForImport(const Modu
// 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());
More information about the llvm-commits
mailing list