Skip to content

Commit 50fea99

Browse files
Reland "[ThinLTO][Bitcode] Generate import type in bitcode" (#97253)
#87600 was reverted in order to revert 6262763. Now #95482 is fix forward for 6262763. This patch is a reland for #87600 **Changes on top of original patch** In `llvm/include/llvm/IR/ModuleSummaryIndex.h`, make the type of `GVSummaryPtrSet` an `unordered_set` which is more memory efficient when the number of elements is smaller than 128 [1] **Original commit message** For distributed ThinLTO, the LTO indexing step generates combined summary for each module, and postlink pipeline reads the combined summary which stores the information for link-time optimization. This patch populates the 'import type' of a summary in bitcode, and updates bitcode reader to parse the bit correctly. [1] https://github.com/llvm/llvm-project/blob/393eff4e02e7ab3d234d246a8d6912c8e745e6f9/llvm/lib/Support/SmallPtrSet.cpp#L43
1 parent 8474194 commit 50fea99

File tree

10 files changed

+121
-42
lines changed

10 files changed

+121
-42
lines changed

llvm/include/llvm/Bitcode/BitcodeWriter.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,8 @@ class BitcodeWriter {
102102

103103
void writeIndex(
104104
const ModuleSummaryIndex *Index,
105-
const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex);
105+
const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex,
106+
const GVSummaryPtrSet *DecSummaries);
106107
};
107108

108109
/// Write the specified module to the specified raw output stream.
@@ -147,10 +148,12 @@ void writeThinLinkBitcodeToFile(const Module &M, raw_ostream &Out,
147148
/// where it will be written in a new bitcode block. This is used when
148149
/// writing the combined index file for ThinLTO. When writing a subset of the
149150
/// index for a distributed backend, provide the \p ModuleToSummariesForIndex
150-
/// map.
151+
/// map. \p DecSummaries specifies the set of summaries for which the
152+
/// corresponding value should be imported as a declaration (prototype).
151153
void writeIndexToFile(const ModuleSummaryIndex &Index, raw_ostream &Out,
152154
const std::map<std::string, GVSummaryMapTy>
153-
*ModuleToSummariesForIndex = nullptr);
155+
*ModuleToSummariesForIndex = nullptr,
156+
const GVSummaryPtrSet *DecSummaries = nullptr);
154157

155158
/// If EmbedBitcode is set, save a copy of the llvm IR as data in the
156159
/// __LLVM,__bitcode section (.llvmbc on non-MacOS).

llvm/include/llvm/IR/ModuleSummaryIndex.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
#include <optional>
4242
#include <set>
4343
#include <string>
44+
#include <unordered_set>
4445
#include <utility>
4546
#include <vector>
4647

@@ -1277,7 +1278,7 @@ using ModulePathStringTableTy = StringMap<ModuleHash>;
12771278
using GVSummaryMapTy = DenseMap<GlobalValue::GUID, GlobalValueSummary *>;
12781279

12791280
/// A set of global value summary pointers.
1280-
using GVSummaryPtrSet = SmallPtrSet<GlobalValueSummary *, 4>;
1281+
using GVSummaryPtrSet = std::unordered_set<GlobalValueSummary *>;
12811282

12821283
/// Map of a type GUID to type id string and summary (multimap used
12831284
/// in case of GUID conflicts).

llvm/include/llvm/LTO/legacy/ThinLTOCodeGenerator.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -271,12 +271,13 @@ class ThinLTOCodeGenerator {
271271
const lto::InputFile &File);
272272

273273
/**
274-
* Compute the list of summaries needed for importing into module.
274+
* Compute the list of summaries and the subset of declaration summaries
275+
* needed for importing into module.
275276
*/
276277
void gatherImportedSummariesForModule(
277278
Module &Module, ModuleSummaryIndex &Index,
278279
std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex,
279-
const lto::InputFile &File);
280+
GVSummaryPtrSet &DecSummaries, const lto::InputFile &File);
280281

281282
/**
282283
* Perform internalization. Index is updated to reflect linkage changes.

llvm/include/llvm/Transforms/IPO/FunctionImport.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,11 +214,15 @@ bool convertToDeclaration(GlobalValue &GV);
214214
/// \p ModuleToSummariesForIndex will be populated with the needed summaries
215215
/// from each required module path. Use a std::map instead of StringMap to get
216216
/// stable order for bitcode emission.
217+
///
218+
/// \p DecSummaries will be popluated with the subset of of summary pointers
219+
/// that have 'declaration' import type among all summaries the module need.
217220
void gatherImportedSummariesForModule(
218221
StringRef ModulePath,
219222
const DenseMap<StringRef, GVSummaryMapTy> &ModuleToDefinedGVSummaries,
220223
const FunctionImporter::ImportMapTy &ImportList,
221-
std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex);
224+
std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex,
225+
GVSummaryPtrSet &DecSummaries);
222226

223227
/// Emit into \p OutputFilename the files module \p ModulePath will import from.
224228
std::error_code EmitImportsFiles(

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,11 @@ class IndexBitcodeWriter : public BitcodeWriterBase {
425425
/// The combined index to write to bitcode.
426426
const ModuleSummaryIndex &Index;
427427

428+
/// When writing combined summaries, provides the set of global value
429+
/// summaries for which the value (function, function alias, etc) should be
430+
/// imported as a declaration.
431+
const GVSummaryPtrSet *DecSummaries = nullptr;
432+
428433
/// When writing a subset of the index for distributed backends, client
429434
/// provides a map of modules to the corresponding GUIDs/summaries to write.
430435
const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex;
@@ -453,11 +458,16 @@ class IndexBitcodeWriter : public BitcodeWriterBase {
453458
/// Constructs a IndexBitcodeWriter object for the given combined index,
454459
/// writing to the provided \p Buffer. When writing a subset of the index
455460
/// for a distributed backend, provide a \p ModuleToSummariesForIndex map.
461+
/// If provided, \p DecSummaries specifies the set of summaries for which
462+
/// the corresponding functions or aliased functions should be imported as a
463+
/// declaration (but not definition) for each module.
456464
IndexBitcodeWriter(BitstreamWriter &Stream, StringTableBuilder &StrtabBuilder,
457465
const ModuleSummaryIndex &Index,
466+
const GVSummaryPtrSet *DecSummaries = nullptr,
458467
const std::map<std::string, GVSummaryMapTy>
459468
*ModuleToSummariesForIndex = nullptr)
460469
: BitcodeWriterBase(Stream, StrtabBuilder), Index(Index),
470+
DecSummaries(DecSummaries),
461471
ModuleToSummariesForIndex(ModuleToSummariesForIndex) {
462472

463473
// See if the StackIdIndex was already added to the StackId map and
@@ -1226,7 +1236,8 @@ static uint64_t getEncodedFFlags(FunctionSummary::FFlags Flags) {
12261236

12271237
// Decode the flags for GlobalValue in the summary. See getDecodedGVSummaryFlags
12281238
// in BitcodeReader.cpp.
1229-
static uint64_t getEncodedGVSummaryFlags(GlobalValueSummary::GVFlags Flags) {
1239+
static uint64_t getEncodedGVSummaryFlags(GlobalValueSummary::GVFlags Flags,
1240+
bool ImportAsDecl = false) {
12301241
uint64_t RawFlags = 0;
12311242

12321243
RawFlags |= Flags.NotEligibleToImport; // bool
@@ -1241,7 +1252,8 @@ static uint64_t getEncodedGVSummaryFlags(GlobalValueSummary::GVFlags Flags) {
12411252

12421253
RawFlags |= (Flags.Visibility << 8); // 2 bits
12431254

1244-
RawFlags |= (Flags.ImportType << 10); // 1 bit
1255+
unsigned ImportType = Flags.ImportType | ImportAsDecl;
1256+
RawFlags |= (ImportType << 10); // 1 bit
12451257

12461258
return RawFlags;
12471259
}
@@ -4568,6 +4580,12 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
45684580
Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));
45694581
unsigned AllocAbbrev = Stream.EmitAbbrev(std::move(Abbv));
45704582

4583+
auto shouldImportValueAsDecl = [&](GlobalValueSummary *GVS) -> bool {
4584+
if (DecSummaries == nullptr)
4585+
return false;
4586+
return DecSummaries->count(GVS);
4587+
};
4588+
45714589
// The aliases are emitted as a post-pass, and will point to the value
45724590
// id of the aliasee. Save them in a vector for post-processing.
45734591
SmallVector<AliasSummary *, 64> Aliases;
@@ -4678,7 +4696,8 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
46784696
NameVals.push_back(*ValueId);
46794697
assert(ModuleIdMap.count(FS->modulePath()));
46804698
NameVals.push_back(ModuleIdMap[FS->modulePath()]);
4681-
NameVals.push_back(getEncodedGVSummaryFlags(FS->flags()));
4699+
NameVals.push_back(
4700+
getEncodedGVSummaryFlags(FS->flags(), shouldImportValueAsDecl(FS)));
46824701
NameVals.push_back(FS->instCount());
46834702
NameVals.push_back(getEncodedFFlags(FS->fflags()));
46844703
NameVals.push_back(FS->entryCount());
@@ -4727,7 +4746,8 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
47274746
NameVals.push_back(AliasValueId);
47284747
assert(ModuleIdMap.count(AS->modulePath()));
47294748
NameVals.push_back(ModuleIdMap[AS->modulePath()]);
4730-
NameVals.push_back(getEncodedGVSummaryFlags(AS->flags()));
4749+
NameVals.push_back(
4750+
getEncodedGVSummaryFlags(AS->flags(), shouldImportValueAsDecl(AS)));
47314751
auto AliaseeValueId = SummaryToValueIdMap[&AS->getAliasee()];
47324752
assert(AliaseeValueId);
47334753
NameVals.push_back(AliaseeValueId);
@@ -5068,8 +5088,9 @@ void BitcodeWriter::writeModule(const Module &M,
50685088

50695089
void BitcodeWriter::writeIndex(
50705090
const ModuleSummaryIndex *Index,
5071-
const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex) {
5072-
IndexBitcodeWriter IndexWriter(*Stream, StrtabBuilder, *Index,
5091+
const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex,
5092+
const GVSummaryPtrSet *DecSummaries) {
5093+
IndexBitcodeWriter IndexWriter(*Stream, StrtabBuilder, *Index, DecSummaries,
50735094
ModuleToSummariesForIndex);
50745095
IndexWriter.write();
50755096
}
@@ -5124,12 +5145,13 @@ void IndexBitcodeWriter::write() {
51245145
// index for a distributed backend, provide a \p ModuleToSummariesForIndex map.
51255146
void llvm::writeIndexToFile(
51265147
const ModuleSummaryIndex &Index, raw_ostream &Out,
5127-
const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex) {
5148+
const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex,
5149+
const GVSummaryPtrSet *DecSummaries) {
51285150
SmallVector<char, 0> Buffer;
51295151
Buffer.reserve(256 * 1024);
51305152

51315153
BitcodeWriter Writer(Buffer);
5132-
Writer.writeIndex(&Index, ModuleToSummariesForIndex);
5154+
Writer.writeIndex(&Index, ModuleToSummariesForIndex, DecSummaries);
51335155
Writer.writeStrtab();
51345156

51355157
Out.write((char *)&Buffer.front(), Buffer.size());

llvm/lib/LTO/LTO.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1400,18 +1400,20 @@ class lto::ThinBackendProc {
14001400
llvm::StringRef ModulePath,
14011401
const std::string &NewModulePath) {
14021402
std::map<std::string, GVSummaryMapTy> ModuleToSummariesForIndex;
1403+
GVSummaryPtrSet DeclarationSummaries;
14031404

14041405
std::error_code EC;
14051406
gatherImportedSummariesForModule(ModulePath, ModuleToDefinedGVSummaries,
1406-
ImportList, ModuleToSummariesForIndex);
1407+
ImportList, ModuleToSummariesForIndex,
1408+
DeclarationSummaries);
14071409

14081410
raw_fd_ostream OS(NewModulePath + ".thinlto.bc", EC,
14091411
sys::fs::OpenFlags::OF_None);
14101412
if (EC)
14111413
return errorCodeToError(EC);
14121414

1413-
// TODO: Serialize declaration bits to bitcode.
1414-
writeIndexToFile(CombinedIndex, OS, &ModuleToSummariesForIndex);
1415+
writeIndexToFile(CombinedIndex, OS, &ModuleToSummariesForIndex,
1416+
&DeclarationSummaries);
14151417

14161418
if (ShouldEmitImportsFiles) {
14171419
EC = EmitImportsFiles(ModulePath, NewModulePath + ".imports",

llvm/lib/LTO/ThinLTOCodeGenerator.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -767,7 +767,7 @@ void ThinLTOCodeGenerator::crossModuleImport(Module &TheModule,
767767
void ThinLTOCodeGenerator::gatherImportedSummariesForModule(
768768
Module &TheModule, ModuleSummaryIndex &Index,
769769
std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex,
770-
const lto::InputFile &File) {
770+
GVSummaryPtrSet &DecSummaries, const lto::InputFile &File) {
771771
auto ModuleCount = Index.modulePaths().size();
772772
auto ModuleIdentifier = TheModule.getModuleIdentifier();
773773

@@ -797,7 +797,7 @@ void ThinLTOCodeGenerator::gatherImportedSummariesForModule(
797797

798798
llvm::gatherImportedSummariesForModule(
799799
ModuleIdentifier, ModuleToDefinedGVSummaries,
800-
ImportLists[ModuleIdentifier], ModuleToSummariesForIndex);
800+
ImportLists[ModuleIdentifier], ModuleToSummariesForIndex, DecSummaries);
801801
}
802802

803803
/**
@@ -833,10 +833,14 @@ void ThinLTOCodeGenerator::emitImports(Module &TheModule, StringRef OutputName,
833833
IsPrevailing(PrevailingCopy), ImportLists,
834834
ExportLists);
835835

836+
// 'EmitImportsFiles' emits the list of modules from which to import from, and
837+
// the set of keys in `ModuleToSummariesForIndex` should be a superset of keys
838+
// in `DecSummaries`, so no need to use `DecSummaries` in `EmitImportFiles`.
839+
GVSummaryPtrSet DecSummaries;
836840
std::map<std::string, GVSummaryMapTy> ModuleToSummariesForIndex;
837841
llvm::gatherImportedSummariesForModule(
838842
ModuleIdentifier, ModuleToDefinedGVSummaries,
839-
ImportLists[ModuleIdentifier], ModuleToSummariesForIndex);
843+
ImportLists[ModuleIdentifier], ModuleToSummariesForIndex, DecSummaries);
840844

841845
std::error_code EC;
842846
if ((EC = EmitImportsFiles(ModuleIdentifier, OutputName,

llvm/lib/Transforms/IPO/FunctionImport.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1421,7 +1421,8 @@ void llvm::gatherImportedSummariesForModule(
14211421
StringRef ModulePath,
14221422
const DenseMap<StringRef, GVSummaryMapTy> &ModuleToDefinedGVSummaries,
14231423
const FunctionImporter::ImportMapTy &ImportList,
1424-
std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex) {
1424+
std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex,
1425+
GVSummaryPtrSet &DecSummaries) {
14251426
// Include all summaries from the importing module.
14261427
ModuleToSummariesForIndex[std::string(ModulePath)] =
14271428
ModuleToDefinedGVSummaries.lookup(ModulePath);
@@ -1436,7 +1437,7 @@ void llvm::gatherImportedSummariesForModule(
14361437
assert(DS != DefinedGVSummaries.end() &&
14371438
"Expected a defined summary for imported global value");
14381439
if (Type == GlobalValueSummary::Declaration)
1439-
continue;
1440+
DecSummaries.insert(DS->second);
14401441

14411442
SummariesForIndex[GUID] = DS->second;
14421443
}

0 commit comments

Comments
 (0)