[llvm] [ThinLTO] Populate declaration import status except for distributed ThinLTO under a default-off new option (PR #88024)
Mingming Liu via llvm-commits
llvm-commits at lists.llvm.org
Sun May 19 22:02:54 PDT 2024
https://github.com/minglotus-6 updated https://github.com/llvm/llvm-project/pull/88024
>From 78fa2e09ac19594baced9dfd27b4ae196f087056 Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Wed, 3 Apr 2024 21:45:50 -0700
Subject: [PATCH 01/17] [ThinLTO]Record import type (declaration or definition)
in GlobalValueSummary::GVFlags
- This change doesn't set the bit in the summaries, just make the code changes.
---
llvm/include/llvm/AsmParser/LLToken.h | 1 +
llvm/include/llvm/IR/ModuleSummaryIndex.h | 14 +++++++++++---
llvm/include/llvm/IR/ModuleSummaryIndexYAML.h | 7 ++++---
llvm/lib/Analysis/ModuleSummaryAnalysis.cpp | 12 ++++++++----
llvm/lib/AsmParser/LLParser.cpp | 15 ++++++++++++---
llvm/lib/Bitcode/Reader/BitcodeReader.cpp | 3 ++-
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp | 5 ++++-
7 files changed, 42 insertions(+), 15 deletions(-)
diff --git a/llvm/include/llvm/AsmParser/LLToken.h b/llvm/include/llvm/AsmParser/LLToken.h
index 65ccb1b81b3a8..21229683f2496 100644
--- a/llvm/include/llvm/AsmParser/LLToken.h
+++ b/llvm/include/llvm/AsmParser/LLToken.h
@@ -370,6 +370,7 @@ enum Kind {
kw_live,
kw_dsoLocal,
kw_canAutoHide,
+ kw_importAsDec,
kw_function,
kw_insts,
kw_funcFlags,
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index d5ff15063671d..7eba77540fa7e 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -472,14 +472,20 @@ class GlobalValueSummary {
/// means the symbol was externally visible.
unsigned CanAutoHide : 1;
+ /// This field is written by the ThinLTO indexing step to postlink
+ /// per-module summary. It indicates whether a global value (function or
+ /// function alias, etc) should be imported as a definition or declaration.
+ unsigned ImportAsDec : 1;
+
/// Convenience Constructors
explicit GVFlags(GlobalValue::LinkageTypes Linkage,
GlobalValue::VisibilityTypes Visibility,
bool NotEligibleToImport, bool Live, bool IsLocal,
- bool CanAutoHide)
+ bool CanAutoHide, bool ImportAsDec)
: Linkage(Linkage), Visibility(Visibility),
NotEligibleToImport(NotEligibleToImport), Live(Live),
- DSOLocal(IsLocal), CanAutoHide(CanAutoHide) {}
+ DSOLocal(IsLocal), CanAutoHide(CanAutoHide),
+ ImportAsDec(ImportAsDec) {}
};
private:
@@ -564,6 +570,8 @@ class GlobalValueSummary {
bool canAutoHide() const { return Flags.CanAutoHide; }
+ bool shouldImportAsDec() const { return Flags.ImportAsDec; }
+
GlobalValue::VisibilityTypes getVisibility() const {
return (GlobalValue::VisibilityTypes)Flags.Visibility;
}
@@ -813,7 +821,7 @@ class FunctionSummary : public GlobalValueSummary {
GlobalValue::LinkageTypes::AvailableExternallyLinkage,
GlobalValue::DefaultVisibility,
/*NotEligibleToImport=*/true, /*Live=*/true, /*IsLocal=*/false,
- /*CanAutoHide=*/false),
+ /*CanAutoHide=*/false, /*ImportAsDec=*/false),
/*NumInsts=*/0, FunctionSummary::FFlags{}, /*EntryCount=*/0,
std::vector<ValueInfo>(), std::move(Edges),
std::vector<GlobalValue::GUID>(),
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h b/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
index 33e57e5f2102f..21dddcf0c03dc 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
@@ -137,7 +137,7 @@ template <> struct MappingTraits<TypeIdSummary> {
struct FunctionSummaryYaml {
unsigned Linkage, Visibility;
- bool NotEligibleToImport, Live, IsLocal, CanAutoHide;
+ bool NotEligibleToImport, Live, IsLocal, CanAutoHide, ImportAsDec;
std::vector<uint64_t> Refs;
std::vector<uint64_t> TypeTests;
std::vector<FunctionSummary::VFuncId> TypeTestAssumeVCalls,
@@ -227,7 +227,7 @@ template <> struct CustomMappingTraits<GlobalValueSummaryMapTy> {
static_cast<GlobalValue::LinkageTypes>(FSum.Linkage),
static_cast<GlobalValue::VisibilityTypes>(FSum.Visibility),
FSum.NotEligibleToImport, FSum.Live, FSum.IsLocal,
- FSum.CanAutoHide),
+ FSum.CanAutoHide, FSum.ImportAsDec),
/*NumInsts=*/0, FunctionSummary::FFlags{}, /*EntryCount=*/0, Refs,
ArrayRef<FunctionSummary::EdgeTy>{}, std::move(FSum.TypeTests),
std::move(FSum.TypeTestAssumeVCalls),
@@ -251,7 +251,8 @@ template <> struct CustomMappingTraits<GlobalValueSummaryMapTy> {
static_cast<bool>(FSum->flags().NotEligibleToImport),
static_cast<bool>(FSum->flags().Live),
static_cast<bool>(FSum->flags().DSOLocal),
- static_cast<bool>(FSum->flags().CanAutoHide), Refs,
+ static_cast<bool>(FSum->flags().CanAutoHide),
+ static_cast<bool>(FSum->flags().ImportAsDec), Refs,
FSum->type_tests(), FSum->type_test_assume_vcalls(),
FSum->type_checked_load_vcalls(),
FSum->type_test_assume_const_vcalls(),
diff --git a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
index 3ad0bab827a51..eb81de81e476d 100644
--- a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
+++ b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
@@ -635,7 +635,8 @@ static void computeFunctionSummary(
HasIndirBranchToBlockAddress || HasIFuncCall;
GlobalValueSummary::GVFlags Flags(
F.getLinkage(), F.getVisibility(), NotEligibleForImport,
- /* Live = */ false, F.isDSOLocal(), F.canBeOmittedFromSymbolTable());
+ /* Live = */ false, F.isDSOLocal(), F.canBeOmittedFromSymbolTable(),
+ /*ImportAsDec=*/false);
FunctionSummary::FFlags FunFlags{
F.doesNotAccessMemory(), F.onlyReadsMemory() && !F.doesNotAccessMemory(),
F.hasFnAttribute(Attribute::NoRecurse), F.returnDoesNotAlias(),
@@ -761,7 +762,8 @@ static void computeVariableSummary(ModuleSummaryIndex &Index,
bool NonRenamableLocal = isNonRenamableLocal(V);
GlobalValueSummary::GVFlags Flags(
V.getLinkage(), V.getVisibility(), NonRenamableLocal,
- /* Live = */ false, V.isDSOLocal(), V.canBeOmittedFromSymbolTable());
+ /* Live = */ false, V.isDSOLocal(), V.canBeOmittedFromSymbolTable(),
+ /* ImportAsDec= */ false);
VTableFuncList VTableFuncs;
// If splitting is not enabled, then we compute the summary information
@@ -807,7 +809,8 @@ static void computeAliasSummary(ModuleSummaryIndex &Index, const GlobalAlias &A,
bool NonRenamableLocal = isNonRenamableLocal(A);
GlobalValueSummary::GVFlags Flags(
A.getLinkage(), A.getVisibility(), NonRenamableLocal,
- /* Live = */ false, A.isDSOLocal(), A.canBeOmittedFromSymbolTable());
+ /* Live = */ false, A.isDSOLocal(), A.canBeOmittedFromSymbolTable(),
+ /* ImportAsDec= */ false);
auto AS = std::make_unique<AliasSummary>(Flags);
auto AliaseeVI = Index.getValueInfo(Aliasee->getGUID());
assert(AliaseeVI && "Alias expects aliasee summary to be available");
@@ -887,7 +890,8 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex(
GlobalValue::InternalLinkage, GlobalValue::DefaultVisibility,
/* NotEligibleToImport = */ true,
/* Live = */ true,
- /* Local */ GV->isDSOLocal(), GV->canBeOmittedFromSymbolTable());
+ /* Local */ GV->isDSOLocal(), GV->canBeOmittedFromSymbolTable(),
+ /* ImportAsDec= */ false);
CantBePromoted.insert(GV->getGUID());
// Create the appropriate summary type.
if (Function *F = dyn_cast<Function>(GV)) {
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index fe49e52ae4283..d437637f468cf 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -9215,7 +9215,8 @@ bool LLParser::parseFunctionSummary(std::string Name, GlobalValue::GUID GUID,
GlobalValueSummary::GVFlags GVFlags = GlobalValueSummary::GVFlags(
GlobalValue::ExternalLinkage, GlobalValue::DefaultVisibility,
/*NotEligibleToImport=*/false,
- /*Live=*/false, /*IsLocal=*/false, /*CanAutoHide=*/false);
+ /*Live=*/false, /*IsLocal=*/false, /*CanAutoHide=*/false,
+ /*ImportAsDec=*/false);
unsigned InstCount;
std::vector<FunctionSummary::EdgeTy> Calls;
FunctionSummary::TypeIdInfo TypeIdInfo;
@@ -9302,7 +9303,8 @@ bool LLParser::parseVariableSummary(std::string Name, GlobalValue::GUID GUID,
GlobalValueSummary::GVFlags GVFlags = GlobalValueSummary::GVFlags(
GlobalValue::ExternalLinkage, GlobalValue::DefaultVisibility,
/*NotEligibleToImport=*/false,
- /*Live=*/false, /*IsLocal=*/false, /*CanAutoHide=*/false);
+ /*Live=*/false, /*IsLocal=*/false, /*CanAutoHide=*/false,
+ /*ImportAsDec*/ false);
GlobalVarSummary::GVarFlags GVarFlags(/*ReadOnly*/ false,
/* WriteOnly */ false,
/* Constant */ false,
@@ -9360,7 +9362,8 @@ bool LLParser::parseAliasSummary(std::string Name, GlobalValue::GUID GUID,
GlobalValueSummary::GVFlags GVFlags = GlobalValueSummary::GVFlags(
GlobalValue::ExternalLinkage, GlobalValue::DefaultVisibility,
/*NotEligibleToImport=*/false,
- /*Live=*/false, /*IsLocal=*/false, /*CanAutoHide=*/false);
+ /*Live=*/false, /*IsLocal=*/false, /*CanAutoHide=*/false,
+ /*ImportAsDec=*/false);
if (parseToken(lltok::colon, "expected ':' here") ||
parseToken(lltok::lparen, "expected '(' here") ||
parseModuleReference(ModulePath) ||
@@ -10146,6 +10149,12 @@ bool LLParser::parseGVFlags(GlobalValueSummary::GVFlags &GVFlags) {
return true;
GVFlags.CanAutoHide = Flag;
break;
+ case lltok::kw_importAsDec:
+ Lex.Lex();
+ if (parseToken(lltok::colon, "expected ':'") || parseFlag(Flag))
+ return true;
+ GVFlags.ImportAsDec = Flag;
+ break;
default:
return error(Lex.getLoc(), "expected gv flag type");
}
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index aa6c9c95ca240..408e6f55cdecd 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -1133,6 +1133,7 @@ static GlobalValueSummary::GVFlags getDecodedGVSummaryFlags(uint64_t RawFlags,
// to getDecodedLinkage() will need to be taken into account here as above.
auto Linkage = GlobalValue::LinkageTypes(RawFlags & 0xF); // 4 bits
auto Visibility = GlobalValue::VisibilityTypes((RawFlags >> 8) & 3); // 2 bits
+ bool ImportAsDec = ((RawFlags >> 10) & 1);
RawFlags = RawFlags >> 4;
bool NotEligibleToImport = (RawFlags & 0x1) || Version < 3;
// The Live flag wasn't introduced until version 3. For dead stripping
@@ -1143,7 +1144,7 @@ static GlobalValueSummary::GVFlags getDecodedGVSummaryFlags(uint64_t RawFlags,
bool AutoHide = (RawFlags & 0x8);
return GlobalValueSummary::GVFlags(Linkage, Visibility, NotEligibleToImport,
- Live, Local, AutoHide);
+ Live, Local, AutoHide, ImportAsDec);
}
// Decode the flags for GlobalVariable in the summary
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index dd554e422516f..5007d498e5a22 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -1202,7 +1202,8 @@ static uint64_t getEncodedFFlags(FunctionSummary::FFlags Flags) {
// Decode the flags for GlobalValue in the summary. See getDecodedGVSummaryFlags
// in BitcodeReader.cpp.
-static uint64_t getEncodedGVSummaryFlags(GlobalValueSummary::GVFlags Flags) {
+static uint64_t getEncodedGVSummaryFlags(GlobalValueSummary::GVFlags Flags,
+ bool ImportAsDec = false) {
uint64_t RawFlags = 0;
RawFlags |= Flags.NotEligibleToImport; // bool
@@ -1217,6 +1218,8 @@ static uint64_t getEncodedGVSummaryFlags(GlobalValueSummary::GVFlags Flags) {
RawFlags |= (Flags.Visibility << 8); // 2 bits
+ RawFlags |= ((ImportAsDec) & (1 << 10)); // 1 bit
+
return RawFlags;
}
>From 4b4c33fc9b42ab6969d38ca300c4cbadb329c33f Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Wed, 3 Apr 2024 22:48:55 -0700
Subject: [PATCH 02/17] [ThinLTO]Generate import type in bitcode writer
---
llvm/include/llvm/Bitcode/BitcodeWriter.h | 15 +++++---
llvm/include/llvm/IR/ModuleSummaryIndex.h | 6 ++++
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp | 44 ++++++++++++++++++-----
3 files changed, 51 insertions(+), 14 deletions(-)
diff --git a/llvm/include/llvm/Bitcode/BitcodeWriter.h b/llvm/include/llvm/Bitcode/BitcodeWriter.h
index 248d33f4502ef..e94b074d9788e 100644
--- a/llvm/include/llvm/Bitcode/BitcodeWriter.h
+++ b/llvm/include/llvm/Bitcode/BitcodeWriter.h
@@ -102,7 +102,8 @@ class raw_ostream;
void writeIndex(
const ModuleSummaryIndex *Index,
- const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex);
+ const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex,
+ const ModuleToGVSummaryPtrSet *ModuleToDecSummaries);
};
/// Write the specified module to the specified raw output stream.
@@ -147,10 +148,14 @@ class raw_ostream;
/// where it will be written in a new bitcode block. This is used when
/// writing the combined index file for ThinLTO. When writing a subset of the
/// index for a distributed backend, provide the \p ModuleToSummariesForIndex
- /// map.
- void writeIndexToFile(const ModuleSummaryIndex &Index, raw_ostream &Out,
- const std::map<std::string, GVSummaryMapTy>
- *ModuleToSummariesForIndex = nullptr);
+ /// map. For each module, \p ModuleToDecSummaries specifies the set of
+ /// summaries for which the corresponding value should be imported as a
+ /// declaration (prototype).
+ void writeIndexToFile(
+ const ModuleSummaryIndex &Index, raw_ostream &Out,
+ const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex =
+ nullptr,
+ const ModuleToGVSummaryPtrSet *ModuleToDecSummaries = nullptr);
/// If EmbedBitcode is set, save a copy of the llvm IR as data in the
/// __LLVM,__bitcode section (.llvmbc on non-MacOS).
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index 7eba77540fa7e..286b51bda0e2c 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -1257,6 +1257,12 @@ 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>;
+
+/// The key is module path, and value is a set of global value summary pointers.
+using ModuleToGVSummaryPtrSet = std::map<std::string, GVSummaryPtrSet>;
+
/// Map of a type GUID to type id string and summary (multimap used
/// in case of GUID conflicts).
using TypeIdSummaryMapTy =
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index 5007d498e5a22..51266f31451b1 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -428,6 +428,10 @@ class IndexBitcodeWriter : public BitcodeWriterBase {
/// The combined index to write to bitcode.
const ModuleSummaryIndex &Index;
+ /// For each module, provides the set of global value summaries for which the
+ /// value (function, function alias, etc) should be imported as a declaration.
+ const ModuleToGVSummaryPtrSet *ModuleToDecSummaries = nullptr;
+
/// When writing a subset of the index for distributed backends, client
/// provides a map of modules to the corresponding GUIDs/summaries to write.
const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex;
@@ -452,11 +456,17 @@ class IndexBitcodeWriter : public BitcodeWriterBase {
/// Constructs a IndexBitcodeWriter object for the given combined index,
/// writing to the provided \p Buffer. When writing a subset of the index
/// for a distributed backend, provide a \p ModuleToSummariesForIndex map.
- IndexBitcodeWriter(BitstreamWriter &Stream, StringTableBuilder &StrtabBuilder,
- const ModuleSummaryIndex &Index,
- const std::map<std::string, GVSummaryMapTy>
- *ModuleToSummariesForIndex = nullptr)
+ /// If provided, \p ModuleToDecSummaries specifies the set of summaries for
+ /// which the corresponding functions or aliased functions should be imported
+ /// as a declaration (but not definition) for each module.
+ IndexBitcodeWriter(
+ BitstreamWriter &Stream, StringTableBuilder &StrtabBuilder,
+ const ModuleSummaryIndex &Index,
+ const ModuleToGVSummaryPtrSet *ModuleToDecSummaries = nullptr,
+ const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex =
+ nullptr)
: BitcodeWriterBase(Stream, StrtabBuilder), Index(Index),
+ ModuleToDecSummaries(ModuleToDecSummaries),
ModuleToSummariesForIndex(ModuleToSummariesForIndex) {
// Assign unique value ids to all summaries to be written, for use
// in writing out the call graph edges. Save the mapping from GUID
@@ -4544,6 +4554,17 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));
unsigned AllocAbbrev = Stream.EmitAbbrev(std::move(Abbv));
+ auto shouldImportValueAsDec = [&](GlobalValueSummary *GVS) -> bool {
+ if (ModuleToDecSummaries == nullptr)
+ return false;
+ auto Iter = ModuleToDecSummaries->find(GVS->modulePath().str());
+ if (Iter == ModuleToDecSummaries->end())
+ return false;
+ // For the current module, the value for GVS should be imported as a
+ // declaration.
+ return Iter->second.contains(GVS);
+ };
+
// The aliases are emitted as a post-pass, and will point to the value
// id of the aliasee. Save them in a vector for post-processing.
SmallVector<AliasSummary *, 64> Aliases;
@@ -4654,7 +4675,8 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
NameVals.push_back(*ValueId);
assert(ModuleIdMap.count(FS->modulePath()));
NameVals.push_back(ModuleIdMap[FS->modulePath()]);
- NameVals.push_back(getEncodedGVSummaryFlags(FS->flags()));
+ NameVals.push_back(
+ getEncodedGVSummaryFlags(FS->flags(), shouldImportValueAsDec(FS)));
NameVals.push_back(FS->instCount());
NameVals.push_back(getEncodedFFlags(FS->fflags()));
NameVals.push_back(FS->entryCount());
@@ -4703,7 +4725,8 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
NameVals.push_back(AliasValueId);
assert(ModuleIdMap.count(AS->modulePath()));
NameVals.push_back(ModuleIdMap[AS->modulePath()]);
- NameVals.push_back(getEncodedGVSummaryFlags(AS->flags()));
+ NameVals.push_back(
+ getEncodedGVSummaryFlags(AS->flags(), shouldImportValueAsDec(AS)));
auto AliaseeValueId = SummaryToValueIdMap[&AS->getAliasee()];
assert(AliaseeValueId);
NameVals.push_back(AliaseeValueId);
@@ -5037,8 +5060,10 @@ void BitcodeWriter::writeModule(const Module &M,
void BitcodeWriter::writeIndex(
const ModuleSummaryIndex *Index,
- const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex) {
+ const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex,
+ const ModuleToGVSummaryPtrSet *ModuleToDecSummaries) {
IndexBitcodeWriter IndexWriter(*Stream, StrtabBuilder, *Index,
+ ModuleToDecSummaries,
ModuleToSummariesForIndex);
IndexWriter.write();
}
@@ -5091,12 +5116,13 @@ void IndexBitcodeWriter::write() {
// index for a distributed backend, provide a \p ModuleToSummariesForIndex map.
void llvm::writeIndexToFile(
const ModuleSummaryIndex &Index, raw_ostream &Out,
- const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex) {
+ const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex,
+ const ModuleToGVSummaryPtrSet *ModuleToDecSummaries) {
SmallVector<char, 0> Buffer;
Buffer.reserve(256 * 1024);
BitcodeWriter Writer(Buffer);
- Writer.writeIndex(&Index, ModuleToSummariesForIndex);
+ Writer.writeIndex(&Index, ModuleToSummariesForIndex, ModuleToDecSummaries);
Writer.writeStrtab();
Out.write((char *)&Buffer.front(), Buffer.size());
>From cfb63d775d43a28b560d938346f1dd4b2dddc765 Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Thu, 4 Apr 2024 11:54:17 -0700
Subject: [PATCH 03/17] function import changes
---
llvm/include/llvm/IR/ModuleSummaryIndex.h | 24 ++++
.../llvm/Transforms/IPO/FunctionImport.h | 18 ++-
llvm/lib/LTO/LTO.cpp | 13 +-
llvm/lib/LTO/LTOBackend.cpp | 5 +-
llvm/lib/LTO/ThinLTOCodeGenerator.cpp | 9 +-
llvm/lib/Transforms/IPO/FunctionImport.cpp | 130 ++++++++++++------
llvm/tools/llvm-link/llvm-link.cpp | 2 +-
7 files changed, 146 insertions(+), 55 deletions(-)
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index 286b51bda0e2c..259fe56ce5f63 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -296,6 +296,30 @@ template <> struct DenseMapInfo<ValueInfo> {
static unsigned getHashValue(ValueInfo I) { return (uintptr_t)I.getRef(); }
};
+struct SummaryImportInfo {
+ enum class ImportType : uint8_t {
+ NotImported = 0,
+ Declaration = 1,
+ Definition = 2,
+ };
+ unsigned Type : 3;
+ SummaryImportInfo() : Type(static_cast<unsigned>(ImportType::NotImported)) {}
+ SummaryImportInfo(ImportType Type) : Type(static_cast<unsigned>(Type)) {}
+
+ // FIXME: delete the first two set* helper function.
+ void updateType(ImportType InputType) {
+ Type = std::max(Type, static_cast<unsigned>(InputType));
+ }
+
+ bool isDefinition() const {
+ return static_cast<ImportType>(Type) == ImportType::Definition;
+ }
+
+ bool isDeclaration() const {
+ return static_cast<ImportType>(Type) == ImportType::Declaration;
+ }
+};
+
/// Summary of memprof callsite metadata.
struct CallsiteInfo {
// Actual callee function.
diff --git a/llvm/include/llvm/Transforms/IPO/FunctionImport.h b/llvm/include/llvm/Transforms/IPO/FunctionImport.h
index c4d19e8641eca..9adc0c31eed43 100644
--- a/llvm/include/llvm/Transforms/IPO/FunctionImport.h
+++ b/llvm/include/llvm/Transforms/IPO/FunctionImport.h
@@ -33,7 +33,14 @@ class FunctionImporter {
public:
/// 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>;
+ using FunctionsToImportTy = DenseMap<GlobalValue::GUID, SummaryImportInfo>;
+
+ // FIXME: Remove this.
+ enum ImportStatus {
+ NotImported,
+ ImportDeclaration,
+ ImportDefinition,
+ };
/// The different reasons selectCallee will chose not to import a
/// candidate.
@@ -99,8 +106,10 @@ class FunctionImporter {
/// index's module path string table).
using ImportMapTy = DenseMap<StringRef, FunctionsToImportTy>;
- /// The set contains an entry for every global value the module exports.
- using ExportSetTy = DenseSet<ValueInfo>;
+ /// The map contains an entry for every global value the module exports, the
+ /// key being the value info, and the value is the summary-based import info.
+ /// FIXME: Does this set need to be a map?
+ using ExportSetTy = DenseMap<ValueInfo, SummaryImportInfo>;
/// A function of this type is used to load modules referenced by the index.
using ModuleLoaderTy =
@@ -211,7 +220,8 @@ void gatherImportedSummariesForModule(
StringRef ModulePath,
const DenseMap<StringRef, GVSummaryMapTy> &ModuleToDefinedGVSummaries,
const FunctionImporter::ImportMapTy &ImportList,
- std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex);
+ std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex,
+ ModuleToGVSummaryPtrSet &ModuleToDecSummaries);
/// Emit into \p OutputFilename the files module \p ModulePath will import from.
std::error_code EmitImportsFiles(
diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index 53060df7f503e..ace533fe28c92 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -159,7 +159,7 @@ void llvm::computeLTOCacheKey(
std::vector<uint64_t> ExportsGUID;
ExportsGUID.reserve(ExportList.size());
for (const auto &VI : ExportList) {
- auto GUID = VI.getGUID();
+ auto GUID = VI.first.getGUID();
ExportsGUID.push_back(GUID);
}
@@ -205,7 +205,7 @@ void llvm::computeLTOCacheKey(
AddUint64(Entry.getFunctions().size());
for (auto &Fn : Entry.getFunctions())
- AddUint64(Fn);
+ AddUint64(Fn.first);
}
// Include the hash for the resolved ODR.
@@ -277,7 +277,7 @@ void llvm::computeLTOCacheKey(
for (const ImportModule &ImpM : ImportModulesVector)
for (auto &ImpF : ImpM.getFunctions()) {
GlobalValueSummary *S =
- Index.findSummaryInModule(ImpF, ImpM.getIdentifier());
+ Index.findSummaryInModule(ImpF.first, ImpM.getIdentifier());
AddUsedThings(S);
// If this is an alias, we also care about any types/etc. that the aliasee
// may reference.
@@ -1389,15 +1389,18 @@ class lto::ThinBackendProc {
llvm::StringRef ModulePath,
const std::string &NewModulePath) {
std::map<std::string, GVSummaryMapTy> ModuleToSummariesForIndex;
+ ModuleToGVSummaryPtrSet ModuleToDeclarationSummaries;
std::error_code EC;
gatherImportedSummariesForModule(ModulePath, ModuleToDefinedGVSummaries,
- ImportList, ModuleToSummariesForIndex);
+ ImportList, ModuleToSummariesForIndex,
+ ModuleToDeclarationSummaries);
raw_fd_ostream OS(NewModulePath + ".thinlto.bc", EC,
sys::fs::OpenFlags::OF_None);
if (EC)
return errorCodeToError(EC);
- writeIndexToFile(CombinedIndex, OS, &ModuleToSummariesForIndex);
+ writeIndexToFile(CombinedIndex, OS, &ModuleToSummariesForIndex,
+ &ModuleToDeclarationSummaries);
if (ShouldEmitImportsFiles) {
EC = EmitImportsFiles(ModulePath, NewModulePath + ".imports",
diff --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp
index 71e8849dc3cc9..6bb514d3d24e7 100644
--- a/llvm/lib/LTO/LTOBackend.cpp
+++ b/llvm/lib/LTO/LTOBackend.cpp
@@ -716,7 +716,10 @@ bool lto::initImportList(const Module &M,
if (Summary->modulePath() == M.getModuleIdentifier())
continue;
// Add an entry to provoke importing by thinBackend.
- ImportList[Summary->modulePath()].insert(GUID);
+ ImportList[Summary->modulePath()][GUID].updateType(
+ Summary->flags().ImportAsDec
+ ? SummaryImportInfo::ImportType::Declaration
+ : SummaryImportInfo::ImportType::Definition);
}
}
return true;
diff --git a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
index 8f517eb50dc76..75aecc95e789f 100644
--- a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
+++ b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
@@ -794,9 +794,11 @@ void ThinLTOCodeGenerator::gatherImportedSummariesForModule(
IsPrevailing(PrevailingCopy), ImportLists,
ExportLists);
+ ModuleToGVSummaryPtrSet ModuleToDecSummaries;
llvm::gatherImportedSummariesForModule(
ModuleIdentifier, ModuleToDefinedGVSummaries,
- ImportLists[ModuleIdentifier], ModuleToSummariesForIndex);
+ ImportLists[ModuleIdentifier], ModuleToSummariesForIndex,
+ ModuleToDecSummaries);
}
/**
@@ -833,9 +835,12 @@ void ThinLTOCodeGenerator::emitImports(Module &TheModule, StringRef OutputName,
ExportLists);
std::map<std::string, GVSummaryMapTy> ModuleToSummariesForIndex;
+ // FIXME: Pass on `ModuleToDecSummaries` to `EmitImportFiles` below.
+ ModuleToGVSummaryPtrSet ModuleToDecSummaries;
llvm::gatherImportedSummariesForModule(
ModuleIdentifier, ModuleToDefinedGVSummaries,
- ImportLists[ModuleIdentifier], ModuleToSummariesForIndex);
+ ImportLists[ModuleIdentifier], ModuleToSummariesForIndex,
+ ModuleToDecSummaries);
std::error_code EC;
if ((EC = EmitImportsFiles(ModuleIdentifier, OutputName,
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index 68f9799616ae6..042fc285128b1 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -358,17 +358,23 @@ class GlobalsImporter final {
if (!GVS || !Index.canImportGlobalVar(GVS, /* AnalyzeRefs */ true) ||
LocalNotInModule(GVS))
continue;
- auto ILI = ImportList[RefSummary->modulePath()].insert(VI.getGUID());
+ auto [Iter, Inserted] =
+ ImportList[RefSummary->modulePath()].try_emplace(
+ VI.getGUID(),
+ SummaryImportInfo(SummaryImportInfo::ImportType::Definition));
// Only update stat and exports if we haven't already imported this
// variable.
- if (!ILI.second)
+ if (!Inserted) {
+ Iter->second.updateType(SummaryImportInfo::ImportType::Definition);
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()].insert(VI);
+ (*ExportLists)[RefSummary->modulePath()][VI].updateType(
+ SummaryImportInfo::ImportType::Definition);
// If variable is not writeonly we attempt to recursively analyze
// its references in order to import referenced constants.
@@ -545,10 +551,12 @@ class WorkloadImportsManager : public ModuleImportsManager {
LLVM_DEBUG(dbgs() << "[Workload][Including]" << VI.name() << " from "
<< ExportingModule << " : "
<< Function::getGUID(VI.name()) << "\n");
- ImportList[ExportingModule].insert(VI.getGUID());
+ ImportList[ExportingModule][VI.getGUID()].updateType(
+ SummaryImportInfo::ImportType::Definition);
GVI.onImportingSummary(*GVS);
if (ExportLists)
- (*ExportLists)[ExportingModule].insert(VI);
+ (*ExportLists)[ExportingModule][VI].updateType(
+ SummaryImportInfo::ImportType::Definition);
}
LLVM_DEBUG(dbgs() << "[Workload] Done\n");
}
@@ -816,23 +824,27 @@ static void computeImportForFunction(
"selectCallee() didn't honor the threshold");
auto ExportModulePath = ResolvedCalleeSummary->modulePath();
- auto ILI = ImportList[ExportModulePath].insert(VI.getGUID());
+ auto [Iter, Inserted] =
+ ImportList[ExportModulePath].insert(std::make_pair(
+ VI.getGUID(),
+ SummaryImportInfo(SummaryImportInfo::ImportType::Definition)));
// We previously decided to import this GUID definition if it was already
// inserted in the set of imports from the exporting module.
- bool PreviouslyImported = !ILI.second;
- if (!PreviouslyImported) {
+ if (Inserted) {
NumImportedFunctionsThinLink++;
if (IsHotCallsite)
NumImportedHotFunctionsThinLink++;
if (IsCriticalCallsite)
NumImportedCriticalFunctionsThinLink++;
- }
+ } else
+ Iter->second.updateType(SummaryImportInfo::ImportType::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].insert(VI);
+ (*ExportLists)[ExportModulePath][VI].updateType(
+ SummaryImportInfo::ImportType::Definition);
}
auto GetAdjustedThreshold = [](unsigned Threshold, bool IsHotCallsite) {
@@ -942,9 +954,11 @@ template <class T>
static unsigned numGlobalVarSummaries(const ModuleSummaryIndex &Index,
T &Cont) {
unsigned NumGVS = 0;
- for (auto &V : Cont)
- if (isGlobalVarSummary(Index, V))
+ for (auto &[GUID, Type] : Cont) {
+ assert(Type.isDefinition() && "Declaration type doesn't exist yet");
+ if (isGlobalVarSummary(Index, GUID))
++NumGVS;
+ }
return NumGVS;
}
#endif
@@ -959,8 +973,8 @@ static bool checkVariableImport(
for (auto &ImportPerModule : ImportLists)
for (auto &ExportPerModule : ImportPerModule.second)
- FlattenedImports.insert(ExportPerModule.second.begin(),
- ExportPerModule.second.end());
+ for (auto &[GUID, Type] : ExportPerModule.second)
+ FlattenedImports.insert(GUID);
// 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,
@@ -979,7 +993,7 @@ static bool checkVariableImport(
};
for (auto &ExportPerModule : ExportLists)
- for (auto &VI : ExportPerModule.second)
+ for (auto &[VI, Unused] : ExportPerModule.second)
if (!FlattenedImports.count(VI.getGUID()) &&
IsReadOrWriteOnlyVarNeedingImporting(ExportPerModule.first, VI))
return false;
@@ -1015,7 +1029,8 @@ void llvm::ComputeCrossModuleImport(
FunctionImporter::ExportSetTy NewExports;
const auto &DefinedGVSummaries =
ModuleToDefinedGVSummaries.lookup(ELI.first);
- for (auto &EI : ELI.second) {
+ for (auto &[EI, Type] : ELI.second) {
+ assert(Type.isDefinition() && "Declaration type doesn't exist yet");
// 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
@@ -1035,13 +1050,16 @@ void llvm::ComputeCrossModuleImport(
// See processGlobalForThinLTO.
if (!Index.isWriteOnly(GVS))
for (const auto &VI : GVS->refs())
- NewExports.insert(VI);
+ NewExports[VI].updateType(
+ SummaryImportInfo::ImportType::Declaration);
} else {
auto *FS = cast<FunctionSummary>(S);
for (const auto &Edge : FS->calls())
- NewExports.insert(Edge.first);
+ NewExports[Edge.first].updateType(
+ SummaryImportInfo::ImportType::Declaration);
for (const auto &Ref : FS->refs())
- NewExports.insert(Ref);
+ NewExports[Ref].updateType(
+ SummaryImportInfo::ImportType::Declaration);
}
}
// Prune list computed above to only include values defined in the exporting
@@ -1049,7 +1067,7 @@ void llvm::ComputeCrossModuleImport(
// 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->getGUID()))
+ if (!DefinedGVSummaries.count(EI->first.getGUID()))
NewExports.erase(EI++);
else
++EI;
@@ -1149,7 +1167,10 @@ static void ComputeCrossModuleImportForModuleFromIndexForTest(
if (Summary->modulePath() == ModulePath)
continue;
// Add an entry to provoke importing by thinBackend.
- ImportList[Summary->modulePath()].insert(GUID);
+ ImportList[Summary->modulePath()][GUID].updateType(
+ Summary->flags().ImportAsDec
+ ? SummaryImportInfo::ImportType::Declaration
+ : SummaryImportInfo::ImportType::Definition);
}
#ifndef NDEBUG
dumpImportListForModule(Index, ModulePath, ImportList);
@@ -1332,20 +1353,25 @@ void llvm::gatherImportedSummariesForModule(
StringRef ModulePath,
const DenseMap<StringRef, GVSummaryMapTy> &ModuleToDefinedGVSummaries,
const FunctionImporter::ImportMapTy &ImportList,
- std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex) {
+ std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex,
+ ModuleToGVSummaryPtrSet &ModuleToDecSummaries) {
// Include all summaries from the importing module.
ModuleToSummariesForIndex[std::string(ModulePath)] =
ModuleToDefinedGVSummaries.lookup(ModulePath);
// Include summaries for imports.
for (const auto &ILI : ImportList) {
- auto &SummariesForIndex = ModuleToSummariesForIndex[std::string(ILI.first)];
+ std::string ModulePath(ILI.first);
+ auto &SummariesForIndex = ModuleToSummariesForIndex[ModulePath];
+ auto &DecSummaries = ModuleToDecSummaries[ModulePath];
const auto &DefinedGVSummaries =
ModuleToDefinedGVSummaries.lookup(ILI.first);
- for (const auto &GI : ILI.second) {
- const auto &DS = DefinedGVSummaries.find(GI);
+ for (const auto &[GUID, Type] : ILI.second) {
+ const auto &DS = DefinedGVSummaries.find(GUID);
assert(DS != DefinedGVSummaries.end() &&
"Expected a defined summary for imported global value");
- SummariesForIndex[GI] = DS->second;
+ SummariesForIndex[GUID] = DS->second;
+ if (Type.isDeclaration())
+ DecSummaries.insert(DS->second);
}
}
}
@@ -1617,6 +1643,16 @@ Expected<bool> FunctionImporter::importFunctions(
for (const auto &FunctionsToImportPerModule : ImportList) {
ModuleNameOrderedList.insert(FunctionsToImportPerModule.first);
}
+
+ auto getImportStatus = [&](const FunctionsToImportTy &GUIDToImportType,
+ GlobalValue::GUID GUID) -> ImportStatus {
+ auto Iter = GUIDToImportType.find(GUID);
+ if (Iter == GUIDToImportType.end())
+ return ImportStatus::NotImported;
+ return Iter->second.isDefinition() ? ImportStatus::ImportDefinition
+ : ImportStatus::ImportDeclaration;
+ };
+
for (const auto &Name : ModuleNameOrderedList) {
// Get the module for the import
const auto &FunctionsToImportPerModule = ImportList.find(Name);
@@ -1634,17 +1670,21 @@ Expected<bool> FunctionImporter::importFunctions(
return std::move(Err);
auto &ImportGUIDs = FunctionsToImportPerModule->second;
+
// Find the globals to import
SetVector<GlobalValue *> GlobalsToImport;
+ SetVector<GlobalValue *> GlobalDeclsToImport;
for (Function &F : *SrcModule) {
if (!F.hasName())
continue;
auto GUID = F.getGUID();
- auto Import = ImportGUIDs.count(GUID);
- LLVM_DEBUG(dbgs() << (Import ? "Is" : "Not") << " importing function "
- << GUID << " " << F.getName() << " from "
- << SrcModule->getSourceFileName() << "\n");
- if (Import) {
+ auto ImportStatus = getImportStatus(ImportGUIDs, GUID);
+ const bool ImportDefinition =
+ ImportStatus == ImportStatus::ImportDefinition;
+ LLVM_DEBUG(dbgs() << (ImportDefinition ? "Is" : "Not")
+ << " importing function " << GUID << " " << F.getName()
+ << " from " << SrcModule->getSourceFileName() << "\n");
+ if (ImportDefinition) {
if (Error Err = F.materialize())
return std::move(Err);
// MemProf should match function's definition and summary,
@@ -1664,17 +1704,20 @@ Expected<bool> FunctionImporter::importFunctions(
SrcModule->getSourceFileName())}));
}
GlobalsToImport.insert(&F);
- }
+ } else if (ImportStatus == ImportDeclaration)
+ GlobalDeclsToImport.insert(&F);
}
for (GlobalVariable &GV : SrcModule->globals()) {
if (!GV.hasName())
continue;
auto GUID = GV.getGUID();
- auto Import = ImportGUIDs.count(GUID);
- LLVM_DEBUG(dbgs() << (Import ? "Is" : "Not") << " importing global "
- << GUID << " " << GV.getName() << " from "
- << SrcModule->getSourceFileName() << "\n");
- if (Import) {
+ auto ImportStatus = getImportStatus(ImportGUIDs, GUID);
+ const bool ImportDefinition =
+ (ImportStatus == ImportStatus::ImportDefinition);
+ LLVM_DEBUG(dbgs() << (ImportDefinition ? "Is" : "Not")
+ << " importing global " << GUID << " " << GV.getName()
+ << " from " << SrcModule->getSourceFileName() << "\n");
+ if (ImportDefinition) {
if (Error Err = GV.materialize())
return std::move(Err);
ImportedGVCount += GlobalsToImport.insert(&GV);
@@ -1684,11 +1727,13 @@ Expected<bool> FunctionImporter::importFunctions(
if (!GA.hasName() || isa<GlobalIFunc>(GA.getAliaseeObject()))
continue;
auto GUID = GA.getGUID();
- auto Import = ImportGUIDs.count(GUID);
- LLVM_DEBUG(dbgs() << (Import ? "Is" : "Not") << " importing alias "
- << GUID << " " << GA.getName() << " from "
- << SrcModule->getSourceFileName() << "\n");
- if (Import) {
+ auto ImportStatus = getImportStatus(ImportGUIDs, GUID);
+ const bool ImportDefinition =
+ (ImportStatus == ImportStatus::ImportDefinition);
+ LLVM_DEBUG(dbgs() << (ImportDefinition ? "Is" : "Not")
+ << " importing alias " << GUID << " " << GA.getName()
+ << " from " << SrcModule->getSourceFileName() << "\n");
+ if (ImportDefinition) {
if (Error Err = GA.materialize())
return std::move(Err);
// Import alias as a copy of its aliasee.
@@ -1738,6 +1783,7 @@ Expected<bool> FunctionImporter::importFunctions(
<< " from " << SrcModule->getSourceFileName() << "\n";
}
+ // FIXME: A later change will pass on GlobalDeclsToImport to IRMover.
if (Error Err = Mover.move(std::move(SrcModule),
GlobalsToImport.getArrayRef(), nullptr,
/*IsPerformingImport=*/true))
diff --git a/llvm/tools/llvm-link/llvm-link.cpp b/llvm/tools/llvm-link/llvm-link.cpp
index 9049cb5e85800..0a60b1e3d8f5d 100644
--- a/llvm/tools/llvm-link/llvm-link.cpp
+++ b/llvm/tools/llvm-link/llvm-link.cpp
@@ -376,7 +376,7 @@ static bool importFunctions(const char *argv0, Module &DestModule) {
auto &Entry =
ImportList[FileNameStringCache.insert(FileName).first->getKey()];
- Entry.insert(F->getGUID());
+ Entry[F->getGUID()].updateType(SummaryImportInfo::ImportType::Definition);
}
auto CachedModuleLoader = [&](StringRef Identifier) {
return ModuleLoaderCache.takeModule(std::string(Identifier));
>From 175febc5e740ba7da2ca76a6222b187ad1e28f60 Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Mon, 15 Apr 2024 22:55:57 -0700
Subject: [PATCH 04/17] rename Dec to Decl
---
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index 7e845e9ed165c..a060265a9bc8d 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -1213,7 +1213,7 @@ static uint64_t getEncodedFFlags(FunctionSummary::FFlags Flags) {
// Decode the flags for GlobalValue in the summary. See getDecodedGVSummaryFlags
// in BitcodeReader.cpp.
static uint64_t getEncodedGVSummaryFlags(GlobalValueSummary::GVFlags Flags,
- bool ImportAsDec = false) {
+ bool ImportAsDecl = false) {
uint64_t RawFlags = 0;
RawFlags |= Flags.NotEligibleToImport; // bool
@@ -1228,7 +1228,8 @@ static uint64_t getEncodedGVSummaryFlags(GlobalValueSummary::GVFlags Flags,
RawFlags |= (Flags.Visibility << 8); // 2 bits
- RawFlags |= (Flags.ImportType << 10); // 1 bit
+ unsigned ImportType = Flags.ImportType | ImportAsDecl;
+ RawFlags |= (ImportType << 10); // 1 bit
return RawFlags;
}
@@ -4554,7 +4555,7 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));
unsigned AllocAbbrev = Stream.EmitAbbrev(std::move(Abbv));
- auto shouldImportValueAsDec = [&](GlobalValueSummary *GVS) -> bool {
+ auto shouldImportValueAsDecl = [&](GlobalValueSummary *GVS) -> bool {
if (ModuleToDecSummaries == nullptr)
return false;
auto Iter = ModuleToDecSummaries->find(GVS->modulePath().str());
@@ -4676,7 +4677,7 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
assert(ModuleIdMap.count(FS->modulePath()));
NameVals.push_back(ModuleIdMap[FS->modulePath()]);
NameVals.push_back(
- getEncodedGVSummaryFlags(FS->flags(), shouldImportValueAsDec(FS)));
+ getEncodedGVSummaryFlags(FS->flags(), shouldImportValueAsDecl(FS)));
NameVals.push_back(FS->instCount());
NameVals.push_back(getEncodedFFlags(FS->fflags()));
NameVals.push_back(FS->entryCount());
@@ -4726,7 +4727,7 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
assert(ModuleIdMap.count(AS->modulePath()));
NameVals.push_back(ModuleIdMap[AS->modulePath()]);
NameVals.push_back(
- getEncodedGVSummaryFlags(AS->flags(), shouldImportValueAsDec(AS)));
+ getEncodedGVSummaryFlags(AS->flags(), shouldImportValueAsDecl(AS)));
auto AliaseeValueId = SummaryToValueIdMap[&AS->getAliasee()];
assert(AliaseeValueId);
NameVals.push_back(AliaseeValueId);
>From 23a3fa4d3d124a2525f20e3a58da6d5e7740e7df Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Tue, 16 Apr 2024 21:12:34 -0700
Subject: [PATCH 05/17] simplify code
---
llvm/include/llvm/IR/ModuleSummaryIndex.h | 30 ++-----
.../llvm/Transforms/IPO/FunctionImport.h | 16 ++--
llvm/lib/LTO/LTOBackend.cpp | 10 ++-
llvm/lib/Transforms/IPO/FunctionImport.cpp | 90 +++++++++----------
llvm/tools/llvm-link/llvm-link.cpp | 2 +-
5 files changed, 62 insertions(+), 86 deletions(-)
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index 7f6f7446e0962..29d13cf4f4fae 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -296,30 +296,6 @@ template <> struct DenseMapInfo<ValueInfo> {
static unsigned getHashValue(ValueInfo I) { return (uintptr_t)I.getRef(); }
};
-struct SummaryImportInfo {
- enum class ImportType : uint8_t {
- NotImported = 0,
- Declaration = 1,
- Definition = 2,
- };
- unsigned Type : 3;
- SummaryImportInfo() : Type(static_cast<unsigned>(ImportType::NotImported)) {}
- SummaryImportInfo(ImportType Type) : Type(static_cast<unsigned>(Type)) {}
-
- // FIXME: delete the first two set* helper function.
- void updateType(ImportType InputType) {
- Type = std::max(Type, static_cast<unsigned>(InputType));
- }
-
- bool isDefinition() const {
- return static_cast<ImportType>(Type) == ImportType::Definition;
- }
-
- bool isDeclaration() const {
- return static_cast<ImportType>(Type) == ImportType::Declaration;
- }
-};
-
/// Summary of memprof callsite metadata.
struct CallsiteInfo {
// Actual callee function.
@@ -609,7 +585,11 @@ class GlobalValueSummary {
return Flags.ImportType == GlobalValueSummary::ImportKind::Declaration;
}
- void setImportKind(ImportKind IK) { Flags.ImportType = IK; }
+ GlobalValueSummary::ImportKind importType() const {
+ return static_cast<ImportKind>(Flags.ImportType);
+ }
+
+ void setImportType(ImportKind IK) { Flags.ImportType = IK; }
GlobalValue::VisibilityTypes getVisibility() const {
return (GlobalValue::VisibilityTypes)Flags.Visibility;
diff --git a/llvm/include/llvm/Transforms/IPO/FunctionImport.h b/llvm/include/llvm/Transforms/IPO/FunctionImport.h
index 9adc0c31eed43..6729fcc420a6e 100644
--- a/llvm/include/llvm/Transforms/IPO/FunctionImport.h
+++ b/llvm/include/llvm/Transforms/IPO/FunctionImport.h
@@ -33,14 +33,8 @@ class FunctionImporter {
public:
/// 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 = DenseMap<GlobalValue::GUID, SummaryImportInfo>;
-
- // FIXME: Remove this.
- enum ImportStatus {
- NotImported,
- ImportDeclaration,
- ImportDefinition,
- };
+ using FunctionsToImportTy =
+ DenseMap<GlobalValue::GUID, GlobalValueSummary::ImportKind>;
/// The different reasons selectCallee will chose not to import a
/// candidate.
@@ -107,9 +101,9 @@ class FunctionImporter {
using ImportMapTy = DenseMap<StringRef, FunctionsToImportTy>;
/// The map contains an entry for every global value the module exports, the
- /// key being the value info, and the value is the summary-based import info.
- /// FIXME: Does this set need to be a map?
- using ExportSetTy = DenseMap<ValueInfo, SummaryImportInfo>;
+ /// key being the value info, and the value indicates whether the definition
+ /// or declaration is visible to another module.
+ using ExportSetTy = DenseMap<ValueInfo, GlobalValueSummary::ImportKind>;
/// A function of this type is used to load modules referenced by the index.
using ModuleLoaderTy =
diff --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp
index 6bb514d3d24e7..23d75c238fc29 100644
--- a/llvm/lib/LTO/LTOBackend.cpp
+++ b/llvm/lib/LTO/LTOBackend.cpp
@@ -715,11 +715,13 @@ bool lto::initImportList(const Module &M,
// e.g. record required linkage changes.
if (Summary->modulePath() == M.getModuleIdentifier())
continue;
+
// Add an entry to provoke importing by thinBackend.
- ImportList[Summary->modulePath()][GUID].updateType(
- Summary->flags().ImportAsDec
- ? SummaryImportInfo::ImportType::Declaration
- : SummaryImportInfo::ImportType::Definition);
+ auto [Iter, Inserted] = ImportList[Summary->modulePath()].try_emplace(
+ GUID, Summary->importType());
+
+ if (!Inserted)
+ Iter->second = std::min(Iter->second, Summary->importType());
}
}
return true;
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index 042fc285128b1..1485259e642a0 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -358,14 +358,14 @@ class GlobalsImporter final {
if (!GVS || !Index.canImportGlobalVar(GVS, /* AnalyzeRefs */ true) ||
LocalNotInModule(GVS))
continue;
+
auto [Iter, Inserted] =
ImportList[RefSummary->modulePath()].try_emplace(
- VI.getGUID(),
- SummaryImportInfo(SummaryImportInfo::ImportType::Definition));
+ VI.getGUID(), GlobalValueSummary::Definition);
// Only update stat and exports if we haven't already imported this
// variable.
if (!Inserted) {
- Iter->second.updateType(SummaryImportInfo::ImportType::Definition);
+ Iter->second = std::min(GlobalValueSummary::Definition, Iter->second);
break;
}
NumImportedGlobalVarsThinLink++;
@@ -373,8 +373,8 @@ class GlobalsImporter final {
// later, in ComputeCrossModuleImport, after import decisions are
// complete, which is more efficient than adding them here.
if (ExportLists)
- (*ExportLists)[RefSummary->modulePath()][VI].updateType(
- SummaryImportInfo::ImportType::Definition);
+ (*ExportLists)[RefSummary->modulePath()][VI] =
+ GlobalValueSummary::Definition;
// If variable is not writeonly we attempt to recursively analyze
// its references in order to import referenced constants.
@@ -551,12 +551,11 @@ class WorkloadImportsManager : public ModuleImportsManager {
LLVM_DEBUG(dbgs() << "[Workload][Including]" << VI.name() << " from "
<< ExportingModule << " : "
<< Function::getGUID(VI.name()) << "\n");
- ImportList[ExportingModule][VI.getGUID()].updateType(
- SummaryImportInfo::ImportType::Definition);
+ ImportList[ExportingModule][VI.getGUID()] =
+ GlobalValueSummary::Definition;
GVI.onImportingSummary(*GVS);
if (ExportLists)
- (*ExportLists)[ExportingModule][VI].updateType(
- SummaryImportInfo::ImportType::Definition);
+ (*ExportLists)[ExportingModule][VI] = GlobalValueSummary::Definition;
}
LLVM_DEBUG(dbgs() << "[Workload] Done\n");
}
@@ -824,10 +823,8 @@ static void computeImportForFunction(
"selectCallee() didn't honor the threshold");
auto ExportModulePath = ResolvedCalleeSummary->modulePath();
- auto [Iter, Inserted] =
- ImportList[ExportModulePath].insert(std::make_pair(
- VI.getGUID(),
- SummaryImportInfo(SummaryImportInfo::ImportType::Definition)));
+ auto [Iter, Inserted] = ImportList[ExportModulePath].insert(
+ std::make_pair(VI.getGUID(), GlobalValueSummary::Definition));
// We previously decided to import this GUID definition if it was already
// inserted in the set of imports from the exporting module.
if (Inserted) {
@@ -837,14 +834,13 @@ static void computeImportForFunction(
if (IsCriticalCallsite)
NumImportedCriticalFunctionsThinLink++;
} else
- Iter->second.updateType(SummaryImportInfo::ImportType::Definition);
+ 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].updateType(
- SummaryImportInfo::ImportType::Definition);
+ (*ExportLists)[ExportModulePath][VI] = GlobalValueSummary::Definition;
}
auto GetAdjustedThreshold = [](unsigned Threshold, bool IsHotCallsite) {
@@ -955,7 +951,8 @@ static unsigned numGlobalVarSummaries(const ModuleSummaryIndex &Index,
T &Cont) {
unsigned NumGVS = 0;
for (auto &[GUID, Type] : Cont) {
- assert(Type.isDefinition() && "Declaration type doesn't exist yet");
+ assert(Type == GlobalValueSummary::Definition &&
+ "Declaration type doesn't exist yet");
if (isGlobalVarSummary(Index, GUID))
++NumGVS;
}
@@ -1030,7 +1027,8 @@ void llvm::ComputeCrossModuleImport(
const auto &DefinedGVSummaries =
ModuleToDefinedGVSummaries.lookup(ELI.first);
for (auto &[EI, Type] : ELI.second) {
- assert(Type.isDefinition() && "Declaration type doesn't exist yet");
+ assert(Type == GlobalValueSummary::Definition &&
+ "Declaration type doesn't exist yet");
// 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
@@ -1049,17 +1047,15 @@ void llvm::ComputeCrossModuleImport(
// we convert such variables initializers to "zeroinitializer".
// See processGlobalForThinLTO.
if (!Index.isWriteOnly(GVS))
- for (const auto &VI : GVS->refs())
- NewExports[VI].updateType(
- SummaryImportInfo::ImportType::Declaration);
+ for (const auto &VI : GVS->refs()) {
+ NewExports.try_emplace(VI, GlobalValueSummary::Declaration);
+ }
} else {
auto *FS = cast<FunctionSummary>(S);
for (const auto &Edge : FS->calls())
- NewExports[Edge.first].updateType(
- SummaryImportInfo::ImportType::Declaration);
+ NewExports.try_emplace(Edge.first, GlobalValueSummary::Declaration);
for (const auto &Ref : FS->refs())
- NewExports[Ref].updateType(
- SummaryImportInfo::ImportType::Declaration);
+ NewExports.try_emplace(Ref, GlobalValueSummary::Declaration);
}
}
// Prune list computed above to only include values defined in the exporting
@@ -1167,10 +1163,10 @@ static void ComputeCrossModuleImportForModuleFromIndexForTest(
if (Summary->modulePath() == ModulePath)
continue;
// Add an entry to provoke importing by thinBackend.
- ImportList[Summary->modulePath()][GUID].updateType(
- Summary->flags().ImportAsDec
- ? SummaryImportInfo::ImportType::Declaration
- : SummaryImportInfo::ImportType::Definition);
+ auto [Iter, Inserted] = ImportList[Summary->modulePath()].try_emplace(
+ GUID, Summary->importType());
+ if (!Inserted)
+ Iter->second = std::min(Iter->second, Summary->importType());
}
#ifndef NDEBUG
dumpImportListForModule(Index, ModulePath, ImportList);
@@ -1370,7 +1366,7 @@ void llvm::gatherImportedSummariesForModule(
assert(DS != DefinedGVSummaries.end() &&
"Expected a defined summary for imported global value");
SummariesForIndex[GUID] = DS->second;
- if (Type.isDeclaration())
+ if (Type == GlobalValueSummary::Declaration)
DecSummaries.insert(DS->second);
}
}
@@ -1644,13 +1640,13 @@ Expected<bool> FunctionImporter::importFunctions(
ModuleNameOrderedList.insert(FunctionsToImportPerModule.first);
}
- auto getImportStatus = [&](const FunctionsToImportTy &GUIDToImportType,
- GlobalValue::GUID GUID) -> ImportStatus {
+ auto maybeGetImportType = [&](const FunctionsToImportTy &GUIDToImportType,
+ GlobalValue::GUID GUID)
+ -> std::optional<GlobalValueSummary::ImportKind> {
auto Iter = GUIDToImportType.find(GUID);
if (Iter == GUIDToImportType.end())
- return ImportStatus::NotImported;
- return Iter->second.isDefinition() ? ImportStatus::ImportDefinition
- : ImportStatus::ImportDeclaration;
+ return std::nullopt;
+ return Iter->second;
};
for (const auto &Name : ModuleNameOrderedList) {
@@ -1673,14 +1669,15 @@ Expected<bool> FunctionImporter::importFunctions(
// Find the globals to import
SetVector<GlobalValue *> GlobalsToImport;
- SetVector<GlobalValue *> GlobalDeclsToImport;
for (Function &F : *SrcModule) {
if (!F.hasName())
continue;
auto GUID = F.getGUID();
- auto ImportStatus = getImportStatus(ImportGUIDs, GUID);
+ auto ImportType = maybeGetImportType(ImportGUIDs, GUID);
+ if (!ImportType)
+ continue;
const bool ImportDefinition =
- ImportStatus == ImportStatus::ImportDefinition;
+ *ImportType == GlobalValueSummary::Definition;
LLVM_DEBUG(dbgs() << (ImportDefinition ? "Is" : "Not")
<< " importing function " << GUID << " " << F.getName()
<< " from " << SrcModule->getSourceFileName() << "\n");
@@ -1704,16 +1701,17 @@ Expected<bool> FunctionImporter::importFunctions(
SrcModule->getSourceFileName())}));
}
GlobalsToImport.insert(&F);
- } else if (ImportStatus == ImportDeclaration)
- GlobalDeclsToImport.insert(&F);
+ }
}
for (GlobalVariable &GV : SrcModule->globals()) {
if (!GV.hasName())
continue;
auto GUID = GV.getGUID();
- auto ImportStatus = getImportStatus(ImportGUIDs, GUID);
+ auto ImportType = maybeGetImportType(ImportGUIDs, GUID);
+ if (!ImportType)
+ continue;
const bool ImportDefinition =
- (ImportStatus == ImportStatus::ImportDefinition);
+ (*ImportType == GlobalValueSummary::Definition);
LLVM_DEBUG(dbgs() << (ImportDefinition ? "Is" : "Not")
<< " importing global " << GUID << " " << GV.getName()
<< " from " << SrcModule->getSourceFileName() << "\n");
@@ -1727,9 +1725,12 @@ Expected<bool> FunctionImporter::importFunctions(
if (!GA.hasName() || isa<GlobalIFunc>(GA.getAliaseeObject()))
continue;
auto GUID = GA.getGUID();
- auto ImportStatus = getImportStatus(ImportGUIDs, GUID);
+ auto ImportType = maybeGetImportType(ImportGUIDs, GUID);
+ if (!ImportType)
+ continue;
+
const bool ImportDefinition =
- (ImportStatus == ImportStatus::ImportDefinition);
+ (*ImportType == GlobalValueSummary::Definition);
LLVM_DEBUG(dbgs() << (ImportDefinition ? "Is" : "Not")
<< " importing alias " << GUID << " " << GA.getName()
<< " from " << SrcModule->getSourceFileName() << "\n");
@@ -1783,7 +1784,6 @@ Expected<bool> FunctionImporter::importFunctions(
<< " from " << SrcModule->getSourceFileName() << "\n";
}
- // FIXME: A later change will pass on GlobalDeclsToImport to IRMover.
if (Error Err = Mover.move(std::move(SrcModule),
GlobalsToImport.getArrayRef(), nullptr,
/*IsPerformingImport=*/true))
diff --git a/llvm/tools/llvm-link/llvm-link.cpp b/llvm/tools/llvm-link/llvm-link.cpp
index 9e28b3a217f8b..a6bee02b1302d 100644
--- a/llvm/tools/llvm-link/llvm-link.cpp
+++ b/llvm/tools/llvm-link/llvm-link.cpp
@@ -377,7 +377,7 @@ static bool importFunctions(const char *argv0, Module &DestModule) {
auto &Entry =
ImportList[FileNameStringCache.insert(FileName).first->getKey()];
- Entry[F->getGUID()].updateType(SummaryImportInfo::ImportType::Definition);
+ Entry[F->getGUID()] = GlobalValueSummary::Definition;
}
auto CachedModuleLoader = [&](StringRef Identifier) {
return ModuleLoaderCache.takeModule(std::string(Identifier));
>From a0277b43faf515c3aa21b290c09e51501fa796ac Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Tue, 16 Apr 2024 23:44:18 -0700
Subject: [PATCH 06/17] Flag gate the import-declaration change.
---
.../llvm/Transforms/IPO/FunctionImport.h | 3 +-
llvm/lib/Transforms/IPO/FunctionImport.cpp | 55 +++++++++++++++++--
2 files changed, 51 insertions(+), 7 deletions(-)
diff --git a/llvm/include/llvm/Transforms/IPO/FunctionImport.h b/llvm/include/llvm/Transforms/IPO/FunctionImport.h
index 6729fcc420a6e..fcf51ed5a4edd 100644
--- a/llvm/include/llvm/Transforms/IPO/FunctionImport.h
+++ b/llvm/include/llvm/Transforms/IPO/FunctionImport.h
@@ -31,8 +31,7 @@ class Module;
/// based on the provided summary informations.
class FunctionImporter {
public:
- /// 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.
+ /// The functions to import from a source module and their import type.
using FunctionsToImportTy =
DenseMap<GlobalValue::GUID, GlobalValueSummary::ImportKind>;
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index 1485259e642a0..eb0bf36ab26d1 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -140,6 +140,16 @@ static cl::opt<bool>
ImportAllIndex("import-all-index",
cl::desc("Import all external functions in index."));
+/// Test-only option.
+/// TODO: Implement selective import (based on combined summary analysis) to
+/// ensure the imported function has a use case in the postlink pipeline.
+/// Import each function declaration as a fallback in real build may grow map
+/// size unnecessarily in the indexing step.
+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
@@ -245,8 +255,10 @@ static auto qualifyCalleeCandidates(
}
/// Given a list of possible callee implementation for a call site, select one
-/// 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).
+/// 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). If caller wants to select eligible
+/// summary
///
/// FIXME: select "best" instead of first that fits. But what is "best"?
/// - The smallest: more likely to be inlined.
@@ -259,24 +271,32 @@ 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 if we can't inline it anyway.
+ // Don't bother importing the definition if we can't inline it anyway.
if (Summary->fflags().NoInline && !ForceImportAll) {
+ TooLargeOrNoInlineSummary = Summary;
Reason = FunctionImporter::ImportFailureReason::NoInline;
continue;
}
@@ -359,12 +379,17 @@ class GlobalsImporter final {
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);
// Only update stat and exports if we haven't already imported this
// variable.
if (!Inserted) {
+ // FIXME: Introduce a wrapper struct around ImportType, and provide
+ // an `updateType` method for better readability, just like how we
+ // update the hotness of a call edge.
Iter->second = std::min(GlobalValueSummary::Definition, Iter->second);
break;
}
@@ -776,9 +801,28 @@ static void computeImportForFunction(
}
FunctionImporter::ImportFailureReason Reason{};
- CalleeSummary = selectCallee(Index, VI.getSummaryList(), NewThreshold,
- Summary.modulePath(), Reason);
+
+ // `SummaryForDeclImport` is an summary eligible for declaration import.
+ const GlobalValueSummary *SummaryForDeclImport = nullptr;
+ CalleeSummary =
+ selectCallee(Index, VI.getSummaryList(), NewThreshold,
+ Summary.modulePath(), SummaryForDeclImport, 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.
@@ -798,6 +842,7 @@ static void computeImportForFunction(
FailureInfo = std::make_unique<FunctionImporter::ImportFailureInfo>(
VI, Edge.second.getHotness(), Reason, 1);
}
+
if (ForceImportAll) {
std::string Msg = std::string("Failed to import function ") +
VI.name().str() + " due to " +
>From 5edecccb0fb4dc4d6da54b2533484c7c176314c6 Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Tue, 23 Apr 2024 09:16:57 -0700
Subject: [PATCH 07/17] stage changes
---
llvm/lib/Transforms/IPO/FunctionImport.cpp | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index eb0bf36ab26d1..bc139e5a905bb 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -996,8 +996,6 @@ static unsigned numGlobalVarSummaries(const ModuleSummaryIndex &Index,
T &Cont) {
unsigned NumGVS = 0;
for (auto &[GUID, Type] : Cont) {
- assert(Type == GlobalValueSummary::Definition &&
- "Declaration type doesn't exist yet");
if (isGlobalVarSummary(Index, GUID))
++NumGVS;
}
@@ -1092,9 +1090,8 @@ void llvm::ComputeCrossModuleImport(
// we convert such variables initializers to "zeroinitializer".
// See processGlobalForThinLTO.
if (!Index.isWriteOnly(GVS))
- for (const auto &VI : GVS->refs()) {
+ for (const auto &VI : GVS->refs())
NewExports.try_emplace(VI, GlobalValueSummary::Declaration);
- }
} else {
auto *FS = cast<FunctionSummary>(S);
for (const auto &Edge : FS->calls())
>From a139e4afc02c1e9f5dfae86f2f4a540cd97d8e87 Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Thu, 2 May 2024 16:15:19 -0700
Subject: [PATCH 08/17] Add test coverage for import delcaration
---
llvm/lib/Transforms/IPO/FunctionImport.cpp | 16 ++-
.../ThinLTO/X86/import_callee_declaration.ll | 111 ++++++++++++++++++
llvm/tools/llvm-link/llvm-link.cpp | 4 +
3 files changed, 125 insertions(+), 6 deletions(-)
create mode 100644 llvm/test/ThinLTO/X86/import_callee_declaration.ll
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index bc139e5a905bb..b13770ed49d83 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -140,11 +140,12 @@ static cl::opt<bool>
ImportAllIndex("import-all-index",
cl::desc("Import all external functions in index."));
-/// Test-only option.
+/// 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.
-/// Import each function declaration as a fallback in real build may grow map
-/// size unnecessarily in the indexing step.
static cl::opt<bool> ImportDeclaration(
"import-declaration", cl::init(false), cl::Hidden,
cl::desc("If true, import function declaration as fallback if the function "
@@ -1070,8 +1071,6 @@ void llvm::ComputeCrossModuleImport(
const auto &DefinedGVSummaries =
ModuleToDefinedGVSummaries.lookup(ELI.first);
for (auto &[EI, Type] : ELI.second) {
- assert(Type == GlobalValueSummary::Definition &&
- "Declaration type doesn't exist yet");
// 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
@@ -1716,8 +1715,13 @@ Expected<bool> FunctionImporter::importFunctions(
continue;
auto GUID = F.getGUID();
auto ImportType = maybeGetImportType(ImportGUIDs, GUID);
- if (!ImportType)
+
+ if (!ImportType) {
+ LLVM_DEBUG(dbgs() << "Not importing function " << GUID << " "
+ << F.getName() << " from "
+ << SrcModule->getSourceFileName() << "\n");
continue;
+ }
const bool ImportDefinition =
*ImportType == GlobalValueSummary::Definition;
LLVM_DEBUG(dbgs() << (ImportDefinition ? "Is" : "Not")
diff --git a/llvm/test/ThinLTO/X86/import_callee_declaration.ll b/llvm/test/ThinLTO/X86/import_callee_declaration.ll
new file mode 100644
index 0000000000000..344bde1a1a07d
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/import_callee_declaration.ll
@@ -0,0 +1,111 @@
+; 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 using per-module summaries.
+; For function import, set 'import-instr-limit' to 5 and fall back to import
+; function declarations.
+;
+; RUN: llvm-lto2 run \
+; RUN: -import-instr-limit=5 \
+; RUN: -import-declaration \
+; RUN: -thinlto-distributed-indexes \
+; RUN: -thinlto-emit-imports \
+; RUN: -r=main.bc,main,px \
+; RUN: -r=main.bc,small_func, \
+; RUN: -r=main.bc,large_func, \
+; RUN: -r=lib.bc,callee,px \
+; 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,calleeAddrs,px -o summary main.bc lib.bc
+
+; main.ll should import {large_func, large_indirect_callee} as declarations.
+;
+; 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: [[LIBMOD:\^[0-9]+]] = module: (path: "lib.bc", hash: (0, 0, 0, 0, 0))
+; 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
+;
+; Secondly disassemble main's combined summary and verify the import type of
+; these two GUIDs are declaration.
+;
+; 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: [[LARGEFUNC:\^[0-9]+]] = gv: (guid: 2418497564662708935, summaries: (function: (module: [[LIBMOD]], flags: ({{.*}} importType: declaration), insts: 6, {{.*}})))
+; MAIN-DIS: [[LARGEINDIRECT:\^[0-9]+]] = gv: (guid: 14343440786664691134, summaries: (function: (module: [[LIBMOD]], flags: ({{.*}} importType: declaration), insts: 7, {{.*}})))
+
+; TODO: Add test coverage for function alias.
+
+;--- 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"
+
+ at calleeAddrs = global [2 x ptr] [ptr @large_indirect_callee, ptr @small_indirect_callee]
+
+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()
+ ret void
+}
+
+define internal void @small_indirect_callee() #0 {
+ ret void
+}
+
+define void @small_func() {
+entry:
+ %0 = load ptr, ptr @calleeAddrs
+ call void %0(), !prof !0
+ %1 = load ptr, ptr getelementptr inbounds ([2 x ptr], ptr @calleeAddrs, i64 0, i64 1)
+ call void %1(), !prof !1
+ ret void
+}
+
+define void @large_func() #0 {
+entry:
+ 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 2, i64 13568239288960714650, i64 1}
diff --git a/llvm/tools/llvm-link/llvm-link.cpp b/llvm/tools/llvm-link/llvm-link.cpp
index a6bee02b1302d..3a20103135152 100644
--- a/llvm/tools/llvm-link/llvm-link.cpp
+++ b/llvm/tools/llvm-link/llvm-link.cpp
@@ -375,6 +375,10 @@ 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;
>From 916dc96a66a1809ef2c6f5511e697489efbb505d Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Fri, 3 May 2024 14:30:10 -0700
Subject: [PATCH 09/17] add test coverage for function alias
---
.../ThinLTO/X86/import_callee_declaration.ll | 46 ++++++++++++++-----
1 file changed, 35 insertions(+), 11 deletions(-)
diff --git a/llvm/test/ThinLTO/X86/import_callee_declaration.ll b/llvm/test/ThinLTO/X86/import_callee_declaration.ll
index 344bde1a1a07d..9854e42494535 100644
--- a/llvm/test/ThinLTO/X86/import_callee_declaration.ll
+++ b/llvm/test/ThinLTO/X86/import_callee_declaration.ll
@@ -4,15 +4,29 @@
; RUN: opt -module-summary main.ll -o main.bc
; RUN: opt -module-summary lib.ll -o lib.bc
-; Generate the combined summary using per-module summaries.
-; For function import, set 'import-instr-limit' to 5 and fall back to import
-; function declarations.
+; 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.
+;
+; TODO: Extend this test case to test IR once postlink optimizer makes use of
+; the import type for declarations, and add test coverage for in-process thinlto.
;
; RUN: llvm-lto2 run \
-; RUN: -import-instr-limit=5 \
+; RUN: -import-instr-limit=7 \
; RUN: -import-declaration \
; RUN: -thinlto-distributed-indexes \
-; RUN: -thinlto-emit-imports \
; RUN: -r=main.bc,main,px \
; RUN: -r=main.bc,small_func, \
; RUN: -r=main.bc,large_func, \
@@ -20,6 +34,7 @@
; 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
; main.ll should import {large_func, large_indirect_callee} as declarations.
@@ -37,10 +52,11 @@
; 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: [[LARGEFUNC:\^[0-9]+]] = gv: (guid: 2418497564662708935, summaries: (function: (module: [[LIBMOD]], flags: ({{.*}} importType: declaration), insts: 6, {{.*}})))
-; MAIN-DIS: [[LARGEINDIRECT:\^[0-9]+]] = gv: (guid: 14343440786664691134, summaries: (function: (module: [[LIBMOD]], flags: ({{.*}} importType: declaration), insts: 7, {{.*}})))
+; MAIN-DIS: [[LARGEFUNC:\^[0-9]+]] = gv: (guid: 2418497564662708935, summaries: (function: (module: [[LIBMOD]], flags: ({{.*}} importType: declaration), insts: 8, {{.*}})))
+; MAIN-DIS: [[LARGEINDIRECT:\^[0-9]+]] = gv: (guid: 14343440786664691134, summaries: (function: (module: [[LIBMOD]], flags: ({{.*}} importType: declaration), insts: 8, {{.*}})))
+; MAIN-DIS: [[LARGEINDIRECTALIAS:\^[0-9]+]] = gv: (guid: 16730173943625350469, summaries: (alias: (module: [[LIBMOD]], flags: ({{.*}} importType: declaration), aliasee: [[LARGEINDIRECT]])))
-; TODO: Add test coverage for function alias.
+; TODO: add test coverage for llvm-lto.
;--- 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"
@@ -62,7 +78,7 @@ 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"
- at calleeAddrs = global [2 x ptr] [ptr @large_indirect_callee, ptr @small_indirect_callee]
+ at calleeAddrs = global [3 x ptr] [ptr @large_indirect_callee, ptr @small_indirect_callee, ptr @large_indirect_callee_alias]
define void @callee() #1 {
ret void
@@ -75,6 +91,7 @@ define void @large_indirect_callee()#2 {
call void @callee()
call void @callee()
call void @callee()
+ call void @callee()
ret void
}
@@ -82,12 +99,16 @@ define internal void @small_indirect_callee() #0 {
ret void
}
+ at 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 ([2 x ptr], ptr @calleeAddrs, i64 0, i64 1)
+ %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
}
@@ -98,6 +119,8 @@ entry:
call void @callee()
call void @callee()
call void @callee()
+ call void @callee()
+ call void @callee()
ret void
}
@@ -108,4 +131,5 @@ attributes #1 = { noinline }
attributes #2 = { norecurse }
!0 = !{!"VP", i32 0, i64 1, i64 14343440786664691134, i64 1}
-!1 = !{!"VP", i32 0, i64 2, i64 13568239288960714650, i64 1}
+!1 = !{!"VP", i32 0, i64 1, i64 13568239288960714650, i64 1}
+!2 = !{!"VP", i32 0, i64 1, i64 16730173943625350469, i64 1}
>From 248fbfb3de943ef056bf2ca430d4b4d485a60691 Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Sun, 5 May 2024 11:39:11 -0700
Subject: [PATCH 10/17] llvm-lto test coverage
---
llvm/include/llvm/LTO/legacy/ThinLTOCodeGenerator.h | 3 ++-
llvm/include/llvm/Transforms/IPO/FunctionImport.h | 12 +++++++++---
llvm/lib/LTO/LTO.cpp | 12 ++++++------
llvm/lib/LTO/LTOBackend.cpp | 4 +++-
llvm/lib/LTO/ThinLTOCodeGenerator.cpp | 3 +--
llvm/test/ThinLTO/X86/import_callee_declaration.ll | 6 +++++-
llvm/tools/llvm-lto/llvm-lto.cpp | 7 +++++--
7 files changed, 31 insertions(+), 16 deletions(-)
diff --git a/llvm/include/llvm/LTO/legacy/ThinLTOCodeGenerator.h b/llvm/include/llvm/LTO/legacy/ThinLTOCodeGenerator.h
index c450acda82ad0..6e197ae2f279c 100644
--- a/llvm/include/llvm/LTO/legacy/ThinLTOCodeGenerator.h
+++ b/llvm/include/llvm/LTO/legacy/ThinLTOCodeGenerator.h
@@ -276,7 +276,8 @@ class ThinLTOCodeGenerator {
void gatherImportedSummariesForModule(
Module &Module, ModuleSummaryIndex &Index,
std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex,
- const lto::InputFile &File);
+ const lto::InputFile &File,
+ ModuleToGVSummaryPtrSet &ModuleToDecSummaries);
/**
* Perform internalization. Index is updated to reflect linkage changes.
diff --git a/llvm/include/llvm/Transforms/IPO/FunctionImport.h b/llvm/include/llvm/Transforms/IPO/FunctionImport.h
index fcf51ed5a4edd..fb38c68b56df0 100644
--- a/llvm/include/llvm/Transforms/IPO/FunctionImport.h
+++ b/llvm/include/llvm/Transforms/IPO/FunctionImport.h
@@ -99,9 +99,11 @@ 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 being the value info, and the value indicates whether the definition
- /// or declaration is visible to another module.
+ /// 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.
using ExportSetTy = DenseMap<ValueInfo, GlobalValueSummary::ImportKind>;
/// A function of this type is used to load modules referenced by the index.
@@ -209,6 +211,10 @@ bool convertToDeclaration(GlobalValue &GV);
/// \p ModuleToSummariesForIndex will be populated with the needed summaries
/// from each required module path. Use a std::map instead of StringMap to get
/// stable order for bitcode emission.
+///
+/// \p ModuleToDecSummaries will be populated with the set of declarations \p
+/// ModulePath need from other modules. They key is module path, and the value
+/// is a set of summary pointers.
void gatherImportedSummariesForModule(
StringRef ModulePath,
const DenseMap<StringRef, GVSummaryMapTy> &ModuleToDefinedGVSummaries,
diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index ace533fe28c92..e2a765d0d52fa 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -158,8 +158,8 @@ void llvm::computeLTOCacheKey(
std::vector<uint64_t> ExportsGUID;
ExportsGUID.reserve(ExportList.size());
- for (const auto &VI : ExportList) {
- auto GUID = VI.first.getGUID();
+ for (const auto &[VI, UnusedImportType] : ExportList) {
+ auto GUID = VI.getGUID();
ExportsGUID.push_back(GUID);
}
@@ -204,8 +204,8 @@ void llvm::computeLTOCacheKey(
Hasher.update(ArrayRef<uint8_t>((uint8_t *)&ModHash[0], sizeof(ModHash)));
AddUint64(Entry.getFunctions().size());
- for (auto &Fn : Entry.getFunctions())
- AddUint64(Fn.first);
+ for (auto &[GUID, UnusedImportType] : Entry.getFunctions())
+ AddUint64(GUID);
}
// Include the hash for the resolved ODR.
@@ -275,9 +275,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 &ImpF : ImpM.getFunctions()) {
+ for (auto &[GUID, UnusedImportType] : ImpM.getFunctions()) {
GlobalValueSummary *S =
- Index.findSummaryInModule(ImpF.first, ImpM.getIdentifier());
+ Index.findSummaryInModule(GUID, ImpM.getIdentifier());
AddUsedThings(S);
// If this is an alias, we also care about any types/etc. that the aliasee
// may reference.
diff --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp
index 23d75c238fc29..a2cd672e0f0a1 100644
--- a/llvm/lib/LTO/LTOBackend.cpp
+++ b/llvm/lib/LTO/LTOBackend.cpp
@@ -715,8 +715,10 @@ bool lto::initImportList(const Module &M,
// e.g. record required linkage changes.
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());
diff --git a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
index 75aecc95e789f..0e6660133cce0 100644
--- a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
+++ b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
@@ -766,7 +766,7 @@ void ThinLTOCodeGenerator::crossModuleImport(Module &TheModule,
void ThinLTOCodeGenerator::gatherImportedSummariesForModule(
Module &TheModule, ModuleSummaryIndex &Index,
std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex,
- const lto::InputFile &File) {
+ const lto::InputFile &File, ModuleToGVSummaryPtrSet &ModuleToDecSummaries) {
auto ModuleCount = Index.modulePaths().size();
auto ModuleIdentifier = TheModule.getModuleIdentifier();
@@ -794,7 +794,6 @@ void ThinLTOCodeGenerator::gatherImportedSummariesForModule(
IsPrevailing(PrevailingCopy), ImportLists,
ExportLists);
- ModuleToGVSummaryPtrSet ModuleToDecSummaries;
llvm::gatherImportedSummariesForModule(
ModuleIdentifier, ModuleToDefinedGVSummaries,
ImportLists[ModuleIdentifier], ModuleToSummariesForIndex,
diff --git a/llvm/test/ThinLTO/X86/import_callee_declaration.ll b/llvm/test/ThinLTO/X86/import_callee_declaration.ll
index 9854e42494535..cdfaa67729619 100644
--- a/llvm/test/ThinLTO/X86/import_callee_declaration.ll
+++ b/llvm/test/ThinLTO/X86/import_callee_declaration.ll
@@ -56,7 +56,11 @@
; MAIN-DIS: [[LARGEINDIRECT:\^[0-9]+]] = gv: (guid: 14343440786664691134, summaries: (function: (module: [[LIBMOD]], flags: ({{.*}} importType: declaration), insts: 8, {{.*}})))
; MAIN-DIS: [[LARGEINDIRECTALIAS:\^[0-9]+]] = gv: (guid: 16730173943625350469, summaries: (alias: (module: [[LIBMOD]], flags: ({{.*}} importType: declaration), aliasee: [[LARGEINDIRECT]])))
-; TODO: add test coverage for llvm-lto.
+; TODO: add test coverage for lto indexing stats.
+
+; 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 -import-declaration -import-instr-limit=7 -thinlto-index combined.index.bc main.bc lib.bc
+; RUN: llvm-dis main.bc.thinlto.bc -o - | FileCheck %s --check-prefix=MAIN-DIS
;--- 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"
diff --git a/llvm/tools/llvm-lto/llvm-lto.cpp b/llvm/tools/llvm-lto/llvm-lto.cpp
index f310097eec634..ce32799150313 100644
--- a/llvm/tools/llvm-lto/llvm-lto.cpp
+++ b/llvm/tools/llvm-lto/llvm-lto.cpp
@@ -692,8 +692,10 @@ class ThinLTOProcessing {
// Build a map of module to the GUIDs and summary objects that should
// be written to its index.
std::map<std::string, GVSummaryMapTy> ModuleToSummariesForIndex;
+ ModuleToGVSummaryPtrSet ModuleToDecSummaries;
ThinGenerator.gatherImportedSummariesForModule(
- *TheModule, *Index, ModuleToSummariesForIndex, *Input);
+ *TheModule, *Index, ModuleToSummariesForIndex, *Input,
+ ModuleToDecSummaries);
std::string OutputName = OutputFilename;
if (OutputName.empty()) {
@@ -703,7 +705,8 @@ class ThinLTOProcessing {
std::error_code EC;
raw_fd_ostream OS(OutputName, EC, sys::fs::OpenFlags::OF_None);
error(EC, "error opening the file '" + OutputName + "'");
- writeIndexToFile(*Index, OS, &ModuleToSummariesForIndex);
+ writeIndexToFile(*Index, OS, &ModuleToSummariesForIndex,
+ &ModuleToDecSummaries);
}
}
>From 5e731ec0b4285f1a0ebbf9cc57cee75f13258c42 Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Mon, 6 May 2024 15:51:44 -0700
Subject: [PATCH 11/17] add test coverage for debugging log, and a RUN line for
internalization
---
llvm/lib/LTO/LTO.cpp | 1 +
llvm/lib/LTO/ThinLTOCodeGenerator.cpp | 5 +-
llvm/lib/Transforms/IPO/FunctionImport.cpp | 109 +++++++++++++-----
llvm/test/ThinLTO/X86/funcimport-stats.ll | 4 +-
.../ThinLTO/X86/import_callee_declaration.ll | 38 +++++-
.../Transforms/FunctionImport/funcimport.ll | 8 +-
6 files changed, 122 insertions(+), 43 deletions(-)
diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index e2a765d0d52fa..e7e5a6ab39277 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -1399,6 +1399,7 @@ class lto::ThinBackendProc {
sys::fs::OpenFlags::OF_None);
if (EC)
return errorCodeToError(EC);
+
writeIndexToFile(CombinedIndex, OS, &ModuleToSummariesForIndex,
&ModuleToDeclarationSummaries);
diff --git a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
index 0e6660133cce0..1227f26cc4280 100644
--- a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
+++ b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
@@ -834,7 +834,10 @@ void ThinLTOCodeGenerator::emitImports(Module &TheModule, StringRef OutputName,
ExportLists);
std::map<std::string, GVSummaryMapTy> ModuleToSummariesForIndex;
- // FIXME: Pass on `ModuleToDecSummaries` to `EmitImportFiles` below.
+ // 'EmitImportsFiles' emits the list of modules from which to import from, and
+ // the set of keys in `ModuleToSummariesForIndex` should be a superset of keys
+ // in `ModuleToDecSummaries`, so no need to use `ModuleToDecSummaries` in
+ // `EmitImportFiles`.
ModuleToGVSummaryPtrSet ModuleToDecSummaries;
llvm::gatherImportedSummariesForModule(
ModuleIdentifier, ModuleToDefinedGVSummaries,
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index b13770ed49d83..77d644dc97ab7 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -381,7 +381,7 @@ class GlobalsImporter final {
continue;
// If there isn't an entry for GUID, insert <GUID, Definition> pair.
- // Otherwise, definition should take precedence over declaration.
+ // Otherwise, definition should take precedence over declaration.
auto [Iter, Inserted] =
ImportList[RefSummary->modulePath()].try_emplace(
VI.getGUID(), GlobalValueSummary::Definition);
@@ -869,17 +869,23 @@ static void computeImportForFunction(
"selectCallee() didn't honor the threshold");
auto ExportModulePath = ResolvedCalleeSummary->modulePath();
- auto [Iter, Inserted] = ImportList[ExportModulePath].insert(
- std::make_pair(VI.getGUID(), GlobalValueSummary::Definition));
+
+ // Try emplace the definition entry, and update stats based on insertion
+ // status.
+ auto [Iter, Inserted] = ImportList[ExportModulePath].try_emplace(
+ VI.getGUID(), GlobalValueSummary::Definition);
+
// We previously decided to import this GUID definition if it was already
// inserted in the set of imports from the exporting module.
- if (Inserted) {
+ if (Inserted || Iter->second == GlobalValueSummary::Declaration) {
NumImportedFunctionsThinLink++;
if (IsHotCallsite)
NumImportedHotFunctionsThinLink++;
if (IsCriticalCallsite)
NumImportedCriticalFunctionsThinLink++;
- } else
+ }
+
+ if (Iter->second == GlobalValueSummary::Declaration)
Iter->second = GlobalValueSummary::Definition;
// Any calls/references made by this function will be marked exported
@@ -993,12 +999,19 @@ static bool isGlobalVarSummary(const ModuleSummaryIndex &Index,
}
template <class T>
-static unsigned numGlobalVarSummaries(const ModuleSummaryIndex &Index,
- T &Cont) {
+static unsigned numGlobalVarSummaries(const ModuleSummaryIndex &Index, T &Cont,
+ unsigned &DefinedGVS,
+ unsigned &DefinedFS) {
unsigned NumGVS = 0;
+ DefinedGVS = 0;
+ DefinedFS = 0;
for (auto &[GUID, Type] : Cont) {
- if (isGlobalVarSummary(Index, GUID))
+ if (isGlobalVarSummary(Index, GUID)) {
+ if (Type == GlobalValueSummary::Definition)
+ ++DefinedGVS;
++NumGVS;
+ } else if (Type == GlobalValueSummary::Definition)
+ ++DefinedFS;
}
return NumGVS;
}
@@ -1009,7 +1022,6 @@ static bool checkVariableImport(
const ModuleSummaryIndex &Index,
DenseMap<StringRef, FunctionImporter::ImportMapTy> &ImportLists,
DenseMap<StringRef, FunctionImporter::ExportSetTy> &ExportLists) {
-
DenseSet<GlobalValue::GUID> FlattenedImports;
for (auto &ImportPerModule : ImportLists)
@@ -1071,6 +1083,10 @@ void llvm::ComputeCrossModuleImport(
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;
// 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
@@ -1089,20 +1105,29 @@ void llvm::ComputeCrossModuleImport(
// we convert such variables initializers to "zeroinitializer".
// See processGlobalForThinLTO.
if (!Index.isWriteOnly(GVS))
- for (const auto &VI : GVS->refs())
+ 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);
+ }
} else {
auto *FS = cast<FunctionSummary>(S);
- for (const auto &Edge : FS->calls())
+ 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())
+ }
+ 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);
+ }
}
}
- // 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()))
NewExports.erase(EI++);
@@ -1119,18 +1144,29 @@ void llvm::ComputeCrossModuleImport(
for (auto &ModuleImports : ImportLists) {
auto ModName = ModuleImports.first;
auto &Exports = ExportLists[ModName];
- 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");
+ 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");
for (auto &Src : ModuleImports.second) {
auto SrcModName = Src.first;
- 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");
+ 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");
}
}
#endif
@@ -1144,11 +1180,17 @@ static void dumpImportListForModule(const ModuleSummaryIndex &Index,
<< ImportList.size() << " modules.\n");
for (auto &Src : ImportList) {
auto SrcModName = Src.first;
- 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");
+ 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");
}
}
#endif
@@ -1206,8 +1248,11 @@ static void ComputeCrossModuleImportForModuleFromIndexForTest(
// Add an entry to provoke importing by thinBackend.
auto [Iter, Inserted] = ImportList[Summary->modulePath()].try_emplace(
GUID, Summary->importType());
- if (!Inserted)
+ 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());
+ }
}
#ifndef NDEBUG
dumpImportListForModule(Index, ModulePath, ImportList);
diff --git a/llvm/test/ThinLTO/X86/funcimport-stats.ll b/llvm/test/ThinLTO/X86/funcimport-stats.ll
index 913b13004c1c2..7fcd33855fe1a 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]+]] functions imported from
-; CHECK-NEXT: - [[NUM_VARS:[0-9]+]] global vars imported from
+; 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]] 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
index cdfaa67729619..12286e5eb7a2f 100644
--- a/llvm/test/ThinLTO/X86/import_callee_declaration.ll
+++ b/llvm/test/ThinLTO/X86/import_callee_declaration.ll
@@ -1,3 +1,5 @@
+; "-debug-only" requires asserts.
+; REQUIRES: asserts
; RUN: rm -rf %t && split-file %s %t && cd %t
; Generate per-module summaries.
@@ -24,18 +26,25 @@
; the import type for declarations, and add test coverage for in-process thinlto.
;
; 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,px \
+; 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
+; 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
+; RUN: llvm-dis main.bc.thinlto.bc -o - | FileCheck %s --check-prefix=MAIN-DIS
+
+; DUMP: - 2 function definitions and 3 function declarations imported from lib.bc
; main.ll should import {large_func, large_indirect_callee} as declarations.
;
@@ -56,11 +65,28 @@
; MAIN-DIS: [[LARGEINDIRECT:\^[0-9]+]] = gv: (guid: 14343440786664691134, summaries: (function: (module: [[LIBMOD]], flags: ({{.*}} importType: declaration), insts: 8, {{.*}})))
; MAIN-DIS: [[LARGEINDIRECTALIAS:\^[0-9]+]] = gv: (guid: 16730173943625350469, summaries: (alias: (module: [[LIBMOD]], flags: ({{.*}} importType: declaration), aliasee: [[LARGEINDIRECT]])))
-; TODO: add test coverage for lto indexing stats.
+; TODO: add test coverage for lto indexing stats (-debug-only=function-import, and 'reqasserts').
-; 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 -import-declaration -import-instr-limit=7 -thinlto-index combined.index.bc main.bc lib.bc
-; RUN: llvm-dis main.bc.thinlto.bc -o - | FileCheck %s --check-prefix=MAIN-DIS
+; Run in-process ThinLTO and tests that `callee` remains internalized even if
+; the symbols of its callers (large_func and large_indirect_callee) are exported
+; and visible to main module.
+
+; RUN: llvm-lto2 run \
+; 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
+
+; RUN: llvm-dis in-process.2.2.internalize.bc -o - | FileCheck %s --check-prefix=INTERNALIZE
+; 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"
diff --git a/llvm/test/Transforms/FunctionImport/funcimport.ll b/llvm/test/Transforms/FunctionImport/funcimport.ll
index a0968a67f5ce8..4b8bcec2a18fb 100644
--- a/llvm/test/Transforms/FunctionImport/funcimport.ll
+++ b/llvm/test/Transforms/FunctionImport/funcimport.ll
@@ -166,7 +166,11 @@ declare void @variadic_va_start(...)
; GUID-DAG: GUID {{.*}} is linkoncefunc
; DUMP: Module [[M1:.*]] imports from 1 module
-; DUMP-NEXT: 15 functions imported from [[M2:.*]]
-; DUMP-NEXT: 4 vars imported from [[M2]]
+; DUMP-NEXT: 15 function definitions and 0 function declarations imported from [[M2:.*]]
+; DUMP-NEXT: 4 var definitions and 0 var declarations imported from [[M2]]
+
+; The following information are printed from `FunctionImporter::importFunctions`
+; in the postlink pipeline.
+; TODO: Update debug information for postlink pipeline.
; DUMP: Imported 15 functions for Module [[M1]]
; DUMP-NEXT: Imported 4 global variables for Module [[M1]]
>From 2c38b47e4526686a2353a1cdd282352a2927f055 Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Tue, 7 May 2024 13:52:11 -0700
Subject: [PATCH 12/17] resolve review feedback
---
llvm/include/llvm/Bitcode/BitcodeWriter.h | 16 +++++-----
llvm/include/llvm/IR/ModuleSummaryIndex.h | 3 --
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp | 38 ++++++++++-------------
3 files changed, 23 insertions(+), 34 deletions(-)
diff --git a/llvm/include/llvm/Bitcode/BitcodeWriter.h b/llvm/include/llvm/Bitcode/BitcodeWriter.h
index e94b074d9788e..a343f0e057631 100644
--- a/llvm/include/llvm/Bitcode/BitcodeWriter.h
+++ b/llvm/include/llvm/Bitcode/BitcodeWriter.h
@@ -103,7 +103,7 @@ class raw_ostream;
void writeIndex(
const ModuleSummaryIndex *Index,
const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex,
- const ModuleToGVSummaryPtrSet *ModuleToDecSummaries);
+ const GVSummaryPtrSet *DecSummaries);
};
/// Write the specified module to the specified raw output stream.
@@ -148,14 +148,12 @@ class raw_ostream;
/// where it will be written in a new bitcode block. This is used when
/// writing the combined index file for ThinLTO. When writing a subset of the
/// index for a distributed backend, provide the \p ModuleToSummariesForIndex
- /// map. For each module, \p ModuleToDecSummaries specifies the set of
- /// summaries for which the corresponding value should be imported as a
- /// declaration (prototype).
- void writeIndexToFile(
- const ModuleSummaryIndex &Index, raw_ostream &Out,
- const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex =
- nullptr,
- const ModuleToGVSummaryPtrSet *ModuleToDecSummaries = nullptr);
+ /// map. \p DecSummaries specifies the set of summaries for which the
+ /// corresponding value should be imported as a declaration (prototype).
+ void writeIndexToFile(const ModuleSummaryIndex &Index, raw_ostream &Out,
+ const std::map<std::string, GVSummaryMapTy>
+ *ModuleToSummariesForIndex = nullptr,
+ const GVSummaryPtrSet *DecSummaries = nullptr);
/// If EmbedBitcode is set, save a copy of the llvm IR as data in the
/// __LLVM,__bitcode section (.llvmbc on non-MacOS).
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index 66209b8cf3ea2..2080e1f18a6c4 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -1275,9 +1275,6 @@ using GVSummaryMapTy = DenseMap<GlobalValue::GUID, GlobalValueSummary *>;
/// A set of global value summary pointers.
using GVSummaryPtrSet = SmallPtrSet<GlobalValueSummary *, 4>;
-/// The key is module path, and value is a set of global value summary pointers.
-using ModuleToGVSummaryPtrSet = std::map<std::string, GVSummaryPtrSet>;
-
/// Map of a type GUID to type id string and summary (multimap used
/// in case of GUID conflicts).
using TypeIdSummaryMapTy =
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index a060265a9bc8d..7b89424194f9b 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -428,9 +428,10 @@ class IndexBitcodeWriter : public BitcodeWriterBase {
/// The combined index to write to bitcode.
const ModuleSummaryIndex &Index;
- /// For each module, provides the set of global value summaries for which the
- /// value (function, function alias, etc) should be imported as a declaration.
- const ModuleToGVSummaryPtrSet *ModuleToDecSummaries = nullptr;
+ /// When writing combined summaries, provides the set of global value
+ /// summaries for which the value (function, function alias, etc) should be
+ /// imported as a declaration.
+ const GVSummaryPtrSet *DecSummaries = nullptr;
/// When writing a subset of the index for distributed backends, client
/// provides a map of modules to the corresponding GUIDs/summaries to write.
@@ -459,14 +460,13 @@ class IndexBitcodeWriter : public BitcodeWriterBase {
/// If provided, \p ModuleToDecSummaries specifies the set of summaries for
/// which the corresponding functions or aliased functions should be imported
/// as a declaration (but not definition) for each module.
- IndexBitcodeWriter(
- BitstreamWriter &Stream, StringTableBuilder &StrtabBuilder,
- const ModuleSummaryIndex &Index,
- const ModuleToGVSummaryPtrSet *ModuleToDecSummaries = nullptr,
- const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex =
- nullptr)
+ IndexBitcodeWriter(BitstreamWriter &Stream, StringTableBuilder &StrtabBuilder,
+ const ModuleSummaryIndex &Index,
+ const GVSummaryPtrSet *DecSummaries = nullptr,
+ const std::map<std::string, GVSummaryMapTy>
+ *ModuleToSummariesForIndex = nullptr)
: BitcodeWriterBase(Stream, StrtabBuilder), Index(Index),
- ModuleToDecSummaries(ModuleToDecSummaries),
+ DecSummaries(DecSummaries),
ModuleToSummariesForIndex(ModuleToSummariesForIndex) {
// Assign unique value ids to all summaries to be written, for use
// in writing out the call graph edges. Save the mapping from GUID
@@ -4556,14 +4556,9 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
unsigned AllocAbbrev = Stream.EmitAbbrev(std::move(Abbv));
auto shouldImportValueAsDecl = [&](GlobalValueSummary *GVS) -> bool {
- if (ModuleToDecSummaries == nullptr)
+ if (DecSummaries == nullptr)
return false;
- auto Iter = ModuleToDecSummaries->find(GVS->modulePath().str());
- if (Iter == ModuleToDecSummaries->end())
- return false;
- // For the current module, the value for GVS should be imported as a
- // declaration.
- return Iter->second.contains(GVS);
+ return DecSummaries->contains(GVS);
};
// The aliases are emitted as a post-pass, and will point to the value
@@ -5062,9 +5057,8 @@ void BitcodeWriter::writeModule(const Module &M,
void BitcodeWriter::writeIndex(
const ModuleSummaryIndex *Index,
const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex,
- const ModuleToGVSummaryPtrSet *ModuleToDecSummaries) {
- IndexBitcodeWriter IndexWriter(*Stream, StrtabBuilder, *Index,
- ModuleToDecSummaries,
+ const GVSummaryPtrSet *DecSummaries) {
+ IndexBitcodeWriter IndexWriter(*Stream, StrtabBuilder, *Index, DecSummaries,
ModuleToSummariesForIndex);
IndexWriter.write();
}
@@ -5118,12 +5112,12 @@ void IndexBitcodeWriter::write() {
void llvm::writeIndexToFile(
const ModuleSummaryIndex &Index, raw_ostream &Out,
const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex,
- const ModuleToGVSummaryPtrSet *ModuleToDecSummaries) {
+ const GVSummaryPtrSet *DecSummaries) {
SmallVector<char, 0> Buffer;
Buffer.reserve(256 * 1024);
BitcodeWriter Writer(Buffer);
- Writer.writeIndex(&Index, ModuleToSummariesForIndex, ModuleToDecSummaries);
+ Writer.writeIndex(&Index, ModuleToSummariesForIndex, DecSummaries);
Writer.writeStrtab();
Out.write((char *)&Buffer.front(), Buffer.size());
>From f1d22e1d7ec2fbe8822658bb4b9a2ea0284cf3ce Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Tue, 7 May 2024 14:21:43 -0700
Subject: [PATCH 13/17] Diffbase is updated
(https://github.com/llvm/llvm-project/pull/87600/commits/2c38b47e4526686a2353a1cdd282352a2927f055).
Update this one accordingly
---
llvm/include/llvm/LTO/legacy/ThinLTOCodeGenerator.h | 3 +--
llvm/include/llvm/Transforms/IPO/FunctionImport.h | 2 +-
llvm/lib/LTO/LTO.cpp | 6 +++---
llvm/lib/LTO/ThinLTOCodeGenerator.cpp | 10 ++++------
llvm/lib/Transforms/IPO/FunctionImport.cpp | 4 ++--
llvm/tools/llvm-lto/llvm-lto.cpp | 8 +++-----
6 files changed, 14 insertions(+), 19 deletions(-)
diff --git a/llvm/include/llvm/LTO/legacy/ThinLTOCodeGenerator.h b/llvm/include/llvm/LTO/legacy/ThinLTOCodeGenerator.h
index 6e197ae2f279c..750d09b65bf90 100644
--- a/llvm/include/llvm/LTO/legacy/ThinLTOCodeGenerator.h
+++ b/llvm/include/llvm/LTO/legacy/ThinLTOCodeGenerator.h
@@ -276,8 +276,7 @@ class ThinLTOCodeGenerator {
void gatherImportedSummariesForModule(
Module &Module, ModuleSummaryIndex &Index,
std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex,
- const lto::InputFile &File,
- ModuleToGVSummaryPtrSet &ModuleToDecSummaries);
+ const lto::InputFile &File, GVSummaryPtrSet &DecSummaries);
/**
* Perform internalization. Index is updated to reflect linkage changes.
diff --git a/llvm/include/llvm/Transforms/IPO/FunctionImport.h b/llvm/include/llvm/Transforms/IPO/FunctionImport.h
index fb38c68b56df0..bc4deb29ba8d2 100644
--- a/llvm/include/llvm/Transforms/IPO/FunctionImport.h
+++ b/llvm/include/llvm/Transforms/IPO/FunctionImport.h
@@ -220,7 +220,7 @@ void gatherImportedSummariesForModule(
const DenseMap<StringRef, GVSummaryMapTy> &ModuleToDefinedGVSummaries,
const FunctionImporter::ImportMapTy &ImportList,
std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex,
- ModuleToGVSummaryPtrSet &ModuleToDecSummaries);
+ GVSummaryPtrSet &DecSummaries);
/// Emit into \p OutputFilename the files module \p ModulePath will import from.
std::error_code EmitImportsFiles(
diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index e3ab5c738cfd9..fd252acb3909a 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -1389,11 +1389,11 @@ class lto::ThinBackendProc {
llvm::StringRef ModulePath,
const std::string &NewModulePath) {
std::map<std::string, GVSummaryMapTy> ModuleToSummariesForIndex;
- ModuleToGVSummaryPtrSet ModuleToDeclarationSummaries;
+ GVSummaryPtrSet DeclarationSummaries;
std::error_code EC;
gatherImportedSummariesForModule(ModulePath, ModuleToDefinedGVSummaries,
ImportList, ModuleToSummariesForIndex,
- ModuleToDeclarationSummaries);
+ DeclarationSummaries);
raw_fd_ostream OS(NewModulePath + ".thinlto.bc", EC,
sys::fs::OpenFlags::OF_None);
@@ -1401,7 +1401,7 @@ class lto::ThinBackendProc {
return errorCodeToError(EC);
writeIndexToFile(CombinedIndex, OS, &ModuleToSummariesForIndex,
- &ModuleToDeclarationSummaries);
+ &DeclarationSummaries);
if (ShouldEmitImportsFiles) {
EC = EmitImportsFiles(ModulePath, NewModulePath + ".imports",
diff --git a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
index 1227f26cc4280..507c7e4048c79 100644
--- a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
+++ b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
@@ -766,7 +766,7 @@ void ThinLTOCodeGenerator::crossModuleImport(Module &TheModule,
void ThinLTOCodeGenerator::gatherImportedSummariesForModule(
Module &TheModule, ModuleSummaryIndex &Index,
std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex,
- const lto::InputFile &File, ModuleToGVSummaryPtrSet &ModuleToDecSummaries) {
+ const lto::InputFile &File, GVSummaryPtrSet &DecSummaries) {
auto ModuleCount = Index.modulePaths().size();
auto ModuleIdentifier = TheModule.getModuleIdentifier();
@@ -796,8 +796,7 @@ void ThinLTOCodeGenerator::gatherImportedSummariesForModule(
llvm::gatherImportedSummariesForModule(
ModuleIdentifier, ModuleToDefinedGVSummaries,
- ImportLists[ModuleIdentifier], ModuleToSummariesForIndex,
- ModuleToDecSummaries);
+ ImportLists[ModuleIdentifier], ModuleToSummariesForIndex, DecSummaries);
}
/**
@@ -838,11 +837,10 @@ void ThinLTOCodeGenerator::emitImports(Module &TheModule, StringRef OutputName,
// the set of keys in `ModuleToSummariesForIndex` should be a superset of keys
// in `ModuleToDecSummaries`, so no need to use `ModuleToDecSummaries` in
// `EmitImportFiles`.
- ModuleToGVSummaryPtrSet ModuleToDecSummaries;
+ GVSummaryPtrSet DecSummaries;
llvm::gatherImportedSummariesForModule(
ModuleIdentifier, ModuleToDefinedGVSummaries,
- ImportLists[ModuleIdentifier], ModuleToSummariesForIndex,
- ModuleToDecSummaries);
+ ImportLists[ModuleIdentifier], ModuleToSummariesForIndex, DecSummaries);
std::error_code EC;
if ((EC = EmitImportsFiles(ModuleIdentifier, OutputName,
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index 77d644dc97ab7..069a4e62a12ae 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -1436,7 +1436,7 @@ void llvm::gatherImportedSummariesForModule(
const DenseMap<StringRef, GVSummaryMapTy> &ModuleToDefinedGVSummaries,
const FunctionImporter::ImportMapTy &ImportList,
std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex,
- ModuleToGVSummaryPtrSet &ModuleToDecSummaries) {
+ GVSummaryPtrSet &DecSummaries) {
// Include all summaries from the importing module.
ModuleToSummariesForIndex[std::string(ModulePath)] =
ModuleToDefinedGVSummaries.lookup(ModulePath);
@@ -1444,7 +1444,7 @@ void llvm::gatherImportedSummariesForModule(
for (const auto &ILI : ImportList) {
std::string ModulePath(ILI.first);
auto &SummariesForIndex = ModuleToSummariesForIndex[ModulePath];
- auto &DecSummaries = ModuleToDecSummaries[ModulePath];
+
const auto &DefinedGVSummaries =
ModuleToDefinedGVSummaries.lookup(ILI.first);
for (const auto &[GUID, Type] : ILI.second) {
diff --git a/llvm/tools/llvm-lto/llvm-lto.cpp b/llvm/tools/llvm-lto/llvm-lto.cpp
index ce32799150313..4bda7f10c00ef 100644
--- a/llvm/tools/llvm-lto/llvm-lto.cpp
+++ b/llvm/tools/llvm-lto/llvm-lto.cpp
@@ -692,10 +692,9 @@ class ThinLTOProcessing {
// Build a map of module to the GUIDs and summary objects that should
// be written to its index.
std::map<std::string, GVSummaryMapTy> ModuleToSummariesForIndex;
- ModuleToGVSummaryPtrSet ModuleToDecSummaries;
+ GVSummaryPtrSet DecSummaries;
ThinGenerator.gatherImportedSummariesForModule(
- *TheModule, *Index, ModuleToSummariesForIndex, *Input,
- ModuleToDecSummaries);
+ *TheModule, *Index, ModuleToSummariesForIndex, *Input, DecSummaries);
std::string OutputName = OutputFilename;
if (OutputName.empty()) {
@@ -705,8 +704,7 @@ class ThinLTOProcessing {
std::error_code EC;
raw_fd_ostream OS(OutputName, EC, sys::fs::OpenFlags::OF_None);
error(EC, "error opening the file '" + OutputName + "'");
- writeIndexToFile(*Index, OS, &ModuleToSummariesForIndex,
- &ModuleToDecSummaries);
+ writeIndexToFile(*Index, OS, &ModuleToSummariesForIndex, &DecSummaries);
}
}
>From 86f3f445b4f5b14b19d5f23791e16906582bfc13 Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Fri, 10 May 2024 12:00:59 -0700
Subject: [PATCH 14/17] resolve review feedback
---
llvm/include/llvm/Bitcode/BitcodeWriter.h | 9 +-
llvm/include/llvm/IR/ModuleSummaryIndex.h | 4 +-
.../llvm/LTO/legacy/ThinLTOCodeGenerator.h | 2 +-
.../llvm/Transforms/IPO/FunctionImport.h | 8 +-
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp | 38 ++------
llvm/lib/LTO/LTO.cpp | 35 ++++---
llvm/lib/LTO/ThinLTOCodeGenerator.cpp | 11 +--
llvm/lib/Transforms/IPO/FunctionImport.cpp | 91 ++++++++++---------
.../ThinLTO/X86/import_callee_declaration.ll | 47 ++++++----
llvm/tools/llvm-lto/llvm-lto.cpp | 5 +-
10 files changed, 124 insertions(+), 126 deletions(-)
diff --git a/llvm/include/llvm/Bitcode/BitcodeWriter.h b/llvm/include/llvm/Bitcode/BitcodeWriter.h
index a343f0e057631..248d33f4502ef 100644
--- a/llvm/include/llvm/Bitcode/BitcodeWriter.h
+++ b/llvm/include/llvm/Bitcode/BitcodeWriter.h
@@ -102,8 +102,7 @@ class raw_ostream;
void writeIndex(
const ModuleSummaryIndex *Index,
- const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex,
- const GVSummaryPtrSet *DecSummaries);
+ const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex);
};
/// Write the specified module to the specified raw output stream.
@@ -148,12 +147,10 @@ class raw_ostream;
/// where it will be written in a new bitcode block. This is used when
/// writing the combined index file for ThinLTO. When writing a subset of the
/// index for a distributed backend, provide the \p ModuleToSummariesForIndex
- /// map. \p DecSummaries specifies the set of summaries for which the
- /// corresponding value should be imported as a declaration (prototype).
+ /// map.
void writeIndexToFile(const ModuleSummaryIndex &Index, raw_ostream &Out,
const std::map<std::string, GVSummaryMapTy>
- *ModuleToSummariesForIndex = nullptr,
- const GVSummaryPtrSet *DecSummaries = nullptr);
+ *ModuleToSummariesForIndex = nullptr);
/// If EmbedBitcode is set, save a copy of the llvm IR as data in the
/// __LLVM,__bitcode section (.llvmbc on non-MacOS).
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index a13c0ad5c5795..a6bb261af7522 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -585,12 +585,12 @@ class GlobalValueSummary {
return Flags.ImportType == GlobalValueSummary::ImportKind::Declaration;
}
+ void setImportKind(ImportKind IK) { Flags.ImportType = IK; }
+
GlobalValueSummary::ImportKind importType() const {
return static_cast<ImportKind>(Flags.ImportType);
}
- void setImportType(ImportKind IK) { Flags.ImportType = IK; }
-
GlobalValue::VisibilityTypes getVisibility() const {
return (GlobalValue::VisibilityTypes)Flags.Visibility;
}
diff --git a/llvm/include/llvm/LTO/legacy/ThinLTOCodeGenerator.h b/llvm/include/llvm/LTO/legacy/ThinLTOCodeGenerator.h
index 750d09b65bf90..c450acda82ad0 100644
--- a/llvm/include/llvm/LTO/legacy/ThinLTOCodeGenerator.h
+++ b/llvm/include/llvm/LTO/legacy/ThinLTOCodeGenerator.h
@@ -276,7 +276,7 @@ class ThinLTOCodeGenerator {
void gatherImportedSummariesForModule(
Module &Module, ModuleSummaryIndex &Index,
std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex,
- const lto::InputFile &File, GVSummaryPtrSet &DecSummaries);
+ const lto::InputFile &File);
/**
* Perform internalization. Index is updated to reflect linkage changes.
diff --git a/llvm/include/llvm/Transforms/IPO/FunctionImport.h b/llvm/include/llvm/Transforms/IPO/FunctionImport.h
index bc4deb29ba8d2..024bba8105b89 100644
--- a/llvm/include/llvm/Transforms/IPO/FunctionImport.h
+++ b/llvm/include/llvm/Transforms/IPO/FunctionImport.h
@@ -104,6 +104,7 @@ class FunctionImporter {
/// 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>;
/// A function of this type is used to load modules referenced by the index.
@@ -211,16 +212,11 @@ bool convertToDeclaration(GlobalValue &GV);
/// \p ModuleToSummariesForIndex will be populated with the needed summaries
/// from each required module path. Use a std::map instead of StringMap to get
/// stable order for bitcode emission.
-///
-/// \p ModuleToDecSummaries will be populated with the set of declarations \p
-/// ModulePath need from other modules. They key is module path, and the value
-/// is a set of summary pointers.
void gatherImportedSummariesForModule(
StringRef ModulePath,
const DenseMap<StringRef, GVSummaryMapTy> &ModuleToDefinedGVSummaries,
const FunctionImporter::ImportMapTy &ImportList,
- std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex,
- GVSummaryPtrSet &DecSummaries);
+ std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex);
/// Emit into \p OutputFilename the files module \p ModulePath will import from.
std::error_code EmitImportsFiles(
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index 7b89424194f9b..6d01e3b4d8218 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -428,11 +428,6 @@ class IndexBitcodeWriter : public BitcodeWriterBase {
/// The combined index to write to bitcode.
const ModuleSummaryIndex &Index;
- /// When writing combined summaries, provides the set of global value
- /// summaries for which the value (function, function alias, etc) should be
- /// imported as a declaration.
- const GVSummaryPtrSet *DecSummaries = nullptr;
-
/// When writing a subset of the index for distributed backends, client
/// provides a map of modules to the corresponding GUIDs/summaries to write.
const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex;
@@ -457,16 +452,11 @@ class IndexBitcodeWriter : public BitcodeWriterBase {
/// Constructs a IndexBitcodeWriter object for the given combined index,
/// writing to the provided \p Buffer. When writing a subset of the index
/// for a distributed backend, provide a \p ModuleToSummariesForIndex map.
- /// If provided, \p ModuleToDecSummaries specifies the set of summaries for
- /// which the corresponding functions or aliased functions should be imported
- /// as a declaration (but not definition) for each module.
IndexBitcodeWriter(BitstreamWriter &Stream, StringTableBuilder &StrtabBuilder,
const ModuleSummaryIndex &Index,
- const GVSummaryPtrSet *DecSummaries = nullptr,
const std::map<std::string, GVSummaryMapTy>
*ModuleToSummariesForIndex = nullptr)
: BitcodeWriterBase(Stream, StrtabBuilder), Index(Index),
- DecSummaries(DecSummaries),
ModuleToSummariesForIndex(ModuleToSummariesForIndex) {
// Assign unique value ids to all summaries to be written, for use
// in writing out the call graph edges. Save the mapping from GUID
@@ -1212,8 +1202,7 @@ static uint64_t getEncodedFFlags(FunctionSummary::FFlags Flags) {
// Decode the flags for GlobalValue in the summary. See getDecodedGVSummaryFlags
// in BitcodeReader.cpp.
-static uint64_t getEncodedGVSummaryFlags(GlobalValueSummary::GVFlags Flags,
- bool ImportAsDecl = false) {
+static uint64_t getEncodedGVSummaryFlags(GlobalValueSummary::GVFlags Flags) {
uint64_t RawFlags = 0;
RawFlags |= Flags.NotEligibleToImport; // bool
@@ -1228,8 +1217,7 @@ static uint64_t getEncodedGVSummaryFlags(GlobalValueSummary::GVFlags Flags,
RawFlags |= (Flags.Visibility << 8); // 2 bits
- unsigned ImportType = Flags.ImportType | ImportAsDecl;
- RawFlags |= (ImportType << 10); // 1 bit
+ RawFlags |= (Flags.ImportType << 10); // 1 bit
return RawFlags;
}
@@ -4555,12 +4543,6 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));
unsigned AllocAbbrev = Stream.EmitAbbrev(std::move(Abbv));
- auto shouldImportValueAsDecl = [&](GlobalValueSummary *GVS) -> bool {
- if (DecSummaries == nullptr)
- return false;
- return DecSummaries->contains(GVS);
- };
-
// The aliases are emitted as a post-pass, and will point to the value
// id of the aliasee. Save them in a vector for post-processing.
SmallVector<AliasSummary *, 64> Aliases;
@@ -4671,8 +4653,7 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
NameVals.push_back(*ValueId);
assert(ModuleIdMap.count(FS->modulePath()));
NameVals.push_back(ModuleIdMap[FS->modulePath()]);
- NameVals.push_back(
- getEncodedGVSummaryFlags(FS->flags(), shouldImportValueAsDecl(FS)));
+ NameVals.push_back(getEncodedGVSummaryFlags(FS->flags()));
NameVals.push_back(FS->instCount());
NameVals.push_back(getEncodedFFlags(FS->fflags()));
NameVals.push_back(FS->entryCount());
@@ -4721,8 +4702,7 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
NameVals.push_back(AliasValueId);
assert(ModuleIdMap.count(AS->modulePath()));
NameVals.push_back(ModuleIdMap[AS->modulePath()]);
- NameVals.push_back(
- getEncodedGVSummaryFlags(AS->flags(), shouldImportValueAsDecl(AS)));
+ NameVals.push_back(getEncodedGVSummaryFlags(AS->flags()));
auto AliaseeValueId = SummaryToValueIdMap[&AS->getAliasee()];
assert(AliaseeValueId);
NameVals.push_back(AliaseeValueId);
@@ -5056,9 +5036,8 @@ void BitcodeWriter::writeModule(const Module &M,
void BitcodeWriter::writeIndex(
const ModuleSummaryIndex *Index,
- const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex,
- const GVSummaryPtrSet *DecSummaries) {
- IndexBitcodeWriter IndexWriter(*Stream, StrtabBuilder, *Index, DecSummaries,
+ const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex) {
+ IndexBitcodeWriter IndexWriter(*Stream, StrtabBuilder, *Index,
ModuleToSummariesForIndex);
IndexWriter.write();
}
@@ -5111,13 +5090,12 @@ void IndexBitcodeWriter::write() {
// index for a distributed backend, provide a \p ModuleToSummariesForIndex map.
void llvm::writeIndexToFile(
const ModuleSummaryIndex &Index, raw_ostream &Out,
- const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex,
- const GVSummaryPtrSet *DecSummaries) {
+ const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex) {
SmallVector<char, 0> Buffer;
Buffer.reserve(256 * 1024);
BitcodeWriter Writer(Buffer);
- Writer.writeIndex(&Index, ModuleToSummariesForIndex, DecSummaries);
+ Writer.writeIndex(&Index, ModuleToSummariesForIndex);
Writer.writeStrtab();
Out.write((char *)&Buffer.front(), Buffer.size());
diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index fd252acb3909a..2bdfd03580145 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -121,6 +121,9 @@ 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
@@ -156,18 +159,23 @@ void llvm::computeLTOCacheKey(
auto ModHash = Index.getModuleHash(ModuleID);
Hasher.update(ArrayRef<uint8_t>((uint8_t *)&ModHash[0], sizeof(ModHash)));
- std::vector<uint64_t> ExportsGUID;
+ std::vector<std::pair<uint64_t, uint8_t>> ExportsGUID;
ExportsGUID.reserve(ExportList.size());
- for (const auto &[VI, UnusedImportType] : ExportList) {
- auto GUID = VI.getGUID();
- ExportsGUID.push_back(GUID);
- }
+ for (const auto &[VI, ExportType] : ExportList)
+ ExportsGUID.push_back(
+ std::make_pair(VI.getGUID(), static_cast<uint8_t>(ExportType)));
// Sort the export list elements GUIDs.
- llvm::sort(ExportsGUID);
- for (uint64_t GUID : ExportsGUID) {
+ llvm::sort(ExportsGUID, [](const std::pair<uint64_t, uint8_t> &LHS,
+ const std::pair<uint64_t, uint8_t> &RHS) {
+ if (LHS.first != RHS.first)
+ return LHS.first < RHS.first;
+ return LHS.second < RHS.second;
+ });
+ for (auto [GUID, ExportType] : 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
@@ -204,8 +212,10 @@ void llvm::computeLTOCacheKey(
Hasher.update(ArrayRef<uint8_t>((uint8_t *)&ModHash[0], sizeof(ModHash)));
AddUint64(Entry.getFunctions().size());
- for (auto &[GUID, UnusedImportType] : Entry.getFunctions())
+ for (auto &[GUID, ImportType] : Entry.getFunctions()) {
AddUint64(GUID);
+ AddUint8(ImportType);
+ }
}
// Include the hash for the resolved ODR.
@@ -1389,19 +1399,18 @@ class lto::ThinBackendProc {
llvm::StringRef ModulePath,
const std::string &NewModulePath) {
std::map<std::string, GVSummaryMapTy> ModuleToSummariesForIndex;
- GVSummaryPtrSet DeclarationSummaries;
+
std::error_code EC;
gatherImportedSummariesForModule(ModulePath, ModuleToDefinedGVSummaries,
- ImportList, ModuleToSummariesForIndex,
- DeclarationSummaries);
+ ImportList, ModuleToSummariesForIndex);
raw_fd_ostream OS(NewModulePath + ".thinlto.bc", EC,
sys::fs::OpenFlags::OF_None);
if (EC)
return errorCodeToError(EC);
- writeIndexToFile(CombinedIndex, OS, &ModuleToSummariesForIndex,
- &DeclarationSummaries);
+ // TODO: Serialize declaration bits to bitcode.
+ writeIndexToFile(CombinedIndex, OS, &ModuleToSummariesForIndex);
if (ShouldEmitImportsFiles) {
EC = EmitImportsFiles(ModulePath, NewModulePath + ".imports",
diff --git a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
index 507c7e4048c79..8f517eb50dc76 100644
--- a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
+++ b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
@@ -766,7 +766,7 @@ void ThinLTOCodeGenerator::crossModuleImport(Module &TheModule,
void ThinLTOCodeGenerator::gatherImportedSummariesForModule(
Module &TheModule, ModuleSummaryIndex &Index,
std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex,
- const lto::InputFile &File, GVSummaryPtrSet &DecSummaries) {
+ const lto::InputFile &File) {
auto ModuleCount = Index.modulePaths().size();
auto ModuleIdentifier = TheModule.getModuleIdentifier();
@@ -796,7 +796,7 @@ void ThinLTOCodeGenerator::gatherImportedSummariesForModule(
llvm::gatherImportedSummariesForModule(
ModuleIdentifier, ModuleToDefinedGVSummaries,
- ImportLists[ModuleIdentifier], ModuleToSummariesForIndex, DecSummaries);
+ ImportLists[ModuleIdentifier], ModuleToSummariesForIndex);
}
/**
@@ -833,14 +833,9 @@ void ThinLTOCodeGenerator::emitImports(Module &TheModule, StringRef OutputName,
ExportLists);
std::map<std::string, GVSummaryMapTy> ModuleToSummariesForIndex;
- // 'EmitImportsFiles' emits the list of modules from which to import from, and
- // the set of keys in `ModuleToSummariesForIndex` should be a superset of keys
- // in `ModuleToDecSummaries`, so no need to use `ModuleToDecSummaries` in
- // `EmitImportFiles`.
- GVSummaryPtrSet DecSummaries;
llvm::gatherImportedSummariesForModule(
ModuleIdentifier, ModuleToDefinedGVSummaries,
- ImportLists[ModuleIdentifier], ModuleToSummariesForIndex, DecSummaries);
+ ImportLists[ModuleIdentifier], ModuleToSummariesForIndex);
std::error_code EC;
if ((EC = EmitImportsFiles(ModuleIdentifier, OutputName,
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index 069a4e62a12ae..e99fb554cd654 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -258,8 +258,10 @@ 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). If caller wants to select eligible
-/// summary
+/// 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.
///
/// FIXME: select "best" instead of first that fits. But what is "best"?
/// - The smallest: more likely to be inlined.
@@ -843,7 +845,6 @@ static void computeImportForFunction(
FailureInfo = std::make_unique<FunctionImporter::ImportFailureInfo>(
VI, Edge.second.getHotness(), Reason, 1);
}
-
if (ForceImportAll) {
std::string Msg = std::string("Failed to import function ") +
VI.name().str() + " due to " +
@@ -1435,15 +1436,13 @@ void llvm::gatherImportedSummariesForModule(
StringRef ModulePath,
const DenseMap<StringRef, GVSummaryMapTy> &ModuleToDefinedGVSummaries,
const FunctionImporter::ImportMapTy &ImportList,
- std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex,
- GVSummaryPtrSet &DecSummaries) {
+ std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex) {
// Include all summaries from the importing module.
ModuleToSummariesForIndex[std::string(ModulePath)] =
ModuleToDefinedGVSummaries.lookup(ModulePath);
// Include summaries for imports.
for (const auto &ILI : ImportList) {
- std::string ModulePath(ILI.first);
- auto &SummariesForIndex = ModuleToSummariesForIndex[ModulePath];
+ auto &SummariesForIndex = ModuleToSummariesForIndex[std::string(ILI.first)];
const auto &DefinedGVSummaries =
ModuleToDefinedGVSummaries.lookup(ILI.first);
@@ -1451,9 +1450,10 @@ void llvm::gatherImportedSummariesForModule(
const auto &DS = DefinedGVSummaries.find(GUID);
assert(DS != DefinedGVSummaries.end() &&
"Expected a defined summary for imported global value");
- SummariesForIndex[GUID] = DS->second;
if (Type == GlobalValueSummary::Declaration)
- DecSummaries.insert(DS->second);
+ continue;
+
+ SummariesForIndex[GUID] = DS->second;
}
}
}
@@ -1726,8 +1726,8 @@ Expected<bool> FunctionImporter::importFunctions(
ModuleNameOrderedList.insert(FunctionsToImportPerModule.first);
}
- auto maybeGetImportType = [&](const FunctionsToImportTy &GUIDToImportType,
- GlobalValue::GUID GUID)
+ auto getImportType = [&](const FunctionsToImportTy &GUIDToImportType,
+ GlobalValue::GUID GUID)
-> std::optional<GlobalValueSummary::ImportKind> {
auto Iter = GUIDToImportType.find(GUID);
if (Iter == GUIDToImportType.end())
@@ -1759,19 +1759,19 @@ Expected<bool> FunctionImporter::importFunctions(
if (!F.hasName())
continue;
auto GUID = F.getGUID();
- auto ImportType = maybeGetImportType(ImportGUIDs, GUID);
-
- if (!ImportType) {
- LLVM_DEBUG(dbgs() << "Not importing function " << GUID << " "
- << F.getName() << " from "
- << SrcModule->getSourceFileName() << "\n");
- continue;
- }
- const bool ImportDefinition =
- *ImportType == GlobalValueSummary::Definition;
- LLVM_DEBUG(dbgs() << (ImportDefinition ? "Is" : "Not")
- << " importing function " << GUID << " " << F.getName()
- << " from " << SrcModule->getSourceFileName() << "\n");
+ auto MaybeImportType = getImportType(ImportGUIDs, GUID);
+
+ bool ImportDefinition =
+ (MaybeImportType &&
+ (*MaybeImportType == GlobalValueSummary::Definition));
+
+ LLVM_DEBUG(dbgs() << (MaybeImportType ? "Is" : "Not")
+ << " importing function"
+ << (ImportDefinition
+ ? " definition "
+ : (MaybeImportType ? " declaration " : " "))
+ << GUID << " " << F.getName() << " from "
+ << SrcModule->getSourceFileName() << "\n");
if (ImportDefinition) {
if (Error Err = F.materialize())
return std::move(Err);
@@ -1798,14 +1798,19 @@ Expected<bool> FunctionImporter::importFunctions(
if (!GV.hasName())
continue;
auto GUID = GV.getGUID();
- auto ImportType = maybeGetImportType(ImportGUIDs, GUID);
- if (!ImportType)
- continue;
- const bool ImportDefinition =
- (*ImportType == GlobalValueSummary::Definition);
- LLVM_DEBUG(dbgs() << (ImportDefinition ? "Is" : "Not")
- << " importing global " << GUID << " " << GV.getName()
- << " from " << SrcModule->getSourceFileName() << "\n");
+ auto MaybeImportType = getImportType(ImportGUIDs, GUID);
+
+ bool ImportDefinition =
+ (MaybeImportType &&
+ (*MaybeImportType == GlobalValueSummary::Definition));
+
+ LLVM_DEBUG(dbgs() << (MaybeImportType ? "Is" : "Not")
+ << " importing global"
+ << (ImportDefinition
+ ? " definition "
+ : (MaybeImportType ? " declaration " : " "))
+ << GUID << " " << GV.getName() << " from "
+ << SrcModule->getSourceFileName() << "\n");
if (ImportDefinition) {
if (Error Err = GV.materialize())
return std::move(Err);
@@ -1816,15 +1821,19 @@ Expected<bool> FunctionImporter::importFunctions(
if (!GA.hasName() || isa<GlobalIFunc>(GA.getAliaseeObject()))
continue;
auto GUID = GA.getGUID();
- auto ImportType = maybeGetImportType(ImportGUIDs, GUID);
- if (!ImportType)
- continue;
-
- const bool ImportDefinition =
- (*ImportType == GlobalValueSummary::Definition);
- LLVM_DEBUG(dbgs() << (ImportDefinition ? "Is" : "Not")
- << " importing alias " << GUID << " " << GA.getName()
- << " from " << SrcModule->getSourceFileName() << "\n");
+ auto MaybeImportType = getImportType(ImportGUIDs, GUID);
+
+ bool ImportDefinition =
+ (MaybeImportType &&
+ (*MaybeImportType == GlobalValueSummary::Definition));
+
+ LLVM_DEBUG(dbgs() << (MaybeImportType ? "Is" : "Not")
+ << " importing alias"
+ << (ImportDefinition
+ ? " definition "
+ : (MaybeImportType ? " declaration " : " "))
+ << GUID << " " << GA.getName() << " from "
+ << SrcModule->getSourceFileName() << "\n");
if (ImportDefinition) {
if (Error Err = GA.materialize())
return std::move(Err);
diff --git a/llvm/test/ThinLTO/X86/import_callee_declaration.ll b/llvm/test/ThinLTO/X86/import_callee_declaration.ll
index 12286e5eb7a2f..b4ce10270e026 100644
--- a/llvm/test/ThinLTO/X86/import_callee_declaration.ll
+++ b/llvm/test/ThinLTO/X86/import_callee_declaration.ll
@@ -23,7 +23,7 @@
; nothing to test more than the summary content.
;
; TODO: Extend this test case to test IR once postlink optimizer makes use of
-; the import type for declarations, and add test coverage for in-process thinlto.
+; the import type for declarations.
;
; RUN: llvm-lto2 run \
; RUN: -debug-only=function-import \
@@ -42,36 +42,34 @@
;
; 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
-; RUN: llvm-dis main.bc.thinlto.bc -o - | FileCheck %s --check-prefix=MAIN-DIS
; DUMP: - 2 function definitions and 3 function declarations imported from lib.bc
-; main.ll should import {large_func, large_indirect_callee} as declarations.
-;
; 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: [[LIBMOD:\^[0-9]+]] = module: (path: "lib.bc", hash: (0, 0, 0, 0, 0))
; 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 verify the import type of
-; these two GUIDs are declaration.
+; Secondly disassemble main's combined summary and test that large callees are
+; not imported as declarations yet.
+; TODO: Serialize declaration bit and test declaration bits are correctly set.
;
; 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: [[LARGEFUNC:\^[0-9]+]] = gv: (guid: 2418497564662708935, summaries: (function: (module: [[LIBMOD]], flags: ({{.*}} importType: declaration), insts: 8, {{.*}})))
-; MAIN-DIS: [[LARGEINDIRECT:\^[0-9]+]] = gv: (guid: 14343440786664691134, summaries: (function: (module: [[LIBMOD]], flags: ({{.*}} importType: declaration), insts: 8, {{.*}})))
-; MAIN-DIS: [[LARGEINDIRECTALIAS:\^[0-9]+]] = gv: (guid: 16730173943625350469, summaries: (alias: (module: [[LIBMOD]], flags: ({{.*}} importType: declaration), aliasee: [[LARGEINDIRECT]])))
-
-; TODO: add test coverage for lto indexing stats (-debug-only=function-import, and 'reqasserts').
+; 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 `callee` remains internalized even if
-; the symbols of its callers (large_func and large_indirect_callee) are exported
-; and visible to main module.
+; 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 \
@@ -83,9 +81,26 @@
; 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
+; RUN: -r=lib.bc,calleeAddrs,px -o in-process main.bc lib.bc 2>&1 | FileCheck %s --check-prefix=IMPORTDUMP
+
+; IMPORTDUMP: Not importing function 11825436545918268459 callee from lib.cc
+; IMPORTDUMP: Is importing function declaration 14343440786664691134 large_indirect_callee from lib.cc
+; IMPORTDUMP: Is importing function definition 13568239288960714650 small_indirect_callee from lib.cc
+; IMPORTDUMP: Is importing function definition 6976996067367342685 small_func from lib.cc
+; IMPORTDUMP: Is importing function declaration 2418497564662708935 large_func from lib.cc
+; IMPORTDUMP: Not importing global 7680325410415171624 calleeAddrs from lib.cc
+; IMPORTDUMP: 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
diff --git a/llvm/tools/llvm-lto/llvm-lto.cpp b/llvm/tools/llvm-lto/llvm-lto.cpp
index 4bda7f10c00ef..f310097eec634 100644
--- a/llvm/tools/llvm-lto/llvm-lto.cpp
+++ b/llvm/tools/llvm-lto/llvm-lto.cpp
@@ -692,9 +692,8 @@ class ThinLTOProcessing {
// Build a map of module to the GUIDs and summary objects that should
// be written to its index.
std::map<std::string, GVSummaryMapTy> ModuleToSummariesForIndex;
- GVSummaryPtrSet DecSummaries;
ThinGenerator.gatherImportedSummariesForModule(
- *TheModule, *Index, ModuleToSummariesForIndex, *Input, DecSummaries);
+ *TheModule, *Index, ModuleToSummariesForIndex, *Input);
std::string OutputName = OutputFilename;
if (OutputName.empty()) {
@@ -704,7 +703,7 @@ class ThinLTOProcessing {
std::error_code EC;
raw_fd_ostream OS(OutputName, EC, sys::fs::OpenFlags::OF_None);
error(EC, "error opening the file '" + OutputName + "'");
- writeIndexToFile(*Index, OS, &ModuleToSummariesForIndex, &DecSummaries);
+ writeIndexToFile(*Index, OS, &ModuleToSummariesForIndex);
}
}
>From ce353d05a3a1a05d50be56f8e2388f7c79ac173b Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Wed, 15 May 2024 10:39:26 -0700
Subject: [PATCH 15/17] resolve feedback
---
llvm/lib/LTO/LTO.cpp | 2 +-
llvm/lib/Transforms/IPO/FunctionImport.cpp | 8 ++++----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index 2bdfd03580145..19b120e23e260 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -121,7 +121,7 @@ void llvm::computeLTOCacheKey(
support::endian::write64le(Data, I);
Hasher.update(Data);
};
- auto AddUint8 = [&](const uint8_t &I) {
+ auto AddUint8 = [&](const uint8_t I) {
Hasher.update(ArrayRef<uint8_t>((const uint8_t *)&I, 1));
};
AddString(Conf.CPU);
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index e99fb554cd654..fa1977ce13040 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -1125,10 +1125,10 @@ void llvm::ComputeCrossModuleImport(
}
}
}
- // 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()))
NewExports.erase(EI++);
>From ee91670f7e195cbcea955a9e7a3dbc5dae4aa29e Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Thu, 16 May 2024 15:24:22 -0700
Subject: [PATCH 16/17] use 'DAG' for log lines and resolve review feedback
---
llvm/lib/Transforms/IPO/FunctionImport.cpp | 1 +
.../ThinLTO/X86/import_callee_declaration.ll | 22 +++++++++----------
.../Transforms/FunctionImport/funcimport.ll | 3 ---
3 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index fa1977ce13040..caae987599b86 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -1900,6 +1900,7 @@ 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/import_callee_declaration.ll b/llvm/test/ThinLTO/X86/import_callee_declaration.ll
index b4ce10270e026..df8a9ce6f7109 100644
--- a/llvm/test/ThinLTO/X86/import_callee_declaration.ll
+++ b/llvm/test/ThinLTO/X86/import_callee_declaration.ll
@@ -22,9 +22,6 @@
; import the declaration or de-serialize summary attributes yet) so there is
; nothing to test more than the summary content.
;
-; TODO: Extend this test case to test IR once postlink optimizer makes use of
-; the import type for declarations.
-;
; RUN: llvm-lto2 run \
; RUN: -debug-only=function-import \
; RUN: -import-instr-limit=7 \
@@ -54,7 +51,6 @@
;
; Secondly disassemble main's combined summary and test that large callees are
; not imported as declarations yet.
-; TODO: Serialize declaration bit and test declaration bits are correctly set.
;
; RUN: llvm-dis main.bc.thinlto.bc -o - | FileCheck %s --check-prefix=MAIN-DIS
;
@@ -83,13 +79,17 @@
; 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
-; IMPORTDUMP: Not importing function 11825436545918268459 callee from lib.cc
-; IMPORTDUMP: Is importing function declaration 14343440786664691134 large_indirect_callee from lib.cc
-; IMPORTDUMP: Is importing function definition 13568239288960714650 small_indirect_callee from lib.cc
-; IMPORTDUMP: Is importing function definition 6976996067367342685 small_func from lib.cc
-; IMPORTDUMP: Is importing function declaration 2418497564662708935 large_func from lib.cc
-; IMPORTDUMP: Not importing global 7680325410415171624 calleeAddrs from lib.cc
-; IMPORTDUMP: Is importing alias declaration 16730173943625350469 large_indirect_callee_alias from lib.cc
+; 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
diff --git a/llvm/test/Transforms/FunctionImport/funcimport.ll b/llvm/test/Transforms/FunctionImport/funcimport.ll
index 4b8bcec2a18fb..635750b33fff0 100644
--- a/llvm/test/Transforms/FunctionImport/funcimport.ll
+++ b/llvm/test/Transforms/FunctionImport/funcimport.ll
@@ -169,8 +169,5 @@ declare void @variadic_va_start(...)
; DUMP-NEXT: 15 function definitions and 0 function declarations imported from [[M2:.*]]
; DUMP-NEXT: 4 var definitions and 0 var declarations imported from [[M2]]
-; The following information are printed from `FunctionImporter::importFunctions`
-; in the postlink pipeline.
-; TODO: Update debug information for postlink pipeline.
; DUMP: Imported 15 functions for Module [[M1]]
; DUMP-NEXT: Imported 4 global variables for Module [[M1]]
>From 3456c504de1fca4a0078e3d231819a8ba079cd01 Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Sun, 19 May 2024 21:40:41 -0700
Subject: [PATCH 17/17] clang-format; and use the lexicographic comparison of
std::pair
---
llvm/lib/LTO/LTO.cpp | 9 ++-------
llvm/lib/Transforms/IPO/FunctionImport.cpp | 5 ++---
2 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index cc4826601b58d..e2754d74979e8 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -166,12 +166,7 @@ void llvm::computeLTOCacheKey(
std::make_pair(VI.getGUID(), static_cast<uint8_t>(ExportType)));
// Sort the export list elements GUIDs.
- llvm::sort(ExportsGUID, [](const std::pair<uint64_t, uint8_t> &LHS,
- const std::pair<uint64_t, uint8_t> &RHS) {
- if (LHS.first != RHS.first)
- return LHS.first < RHS.first;
- return LHS.second < RHS.second;
- });
+ llvm::sort(ExportsGUID);
for (auto [GUID, ExportType] : ExportsGUID) {
// The export list can impact the internalization, be conservative here
Hasher.update(ArrayRef<uint8_t>((uint8_t *)&GUID, sizeof(GUID)));
@@ -218,7 +213,7 @@ void llvm::computeLTOCacheKey(
for (auto &[Fn, ImportType] : Entry.getFunctions())
ImportedGUIDs.push_back(std::make_pair(Fn, ImportType));
llvm::sort(ImportedGUIDs);
- for (auto &[GUID, Type]: ImportedGUIDs) {
+ for (auto &[GUID, Type] : ImportedGUIDs) {
AddUint64(GUID);
AddUint8(Type);
}
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index caae987599b86..a116fd6535347 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -390,9 +390,8 @@ class GlobalsImporter final {
// Only update stat and exports if we haven't already imported this
// variable.
if (!Inserted) {
- // FIXME: Introduce a wrapper struct around ImportType, and provide
- // an `updateType` method for better readability, just like how we
- // update the hotness of a call edge.
+ // 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);
break;
}
More information about the llvm-commits
mailing list