Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@teresajohnson
teresajohnson committed Jul 11, 2024
1 parent f320adf commit 8be1c3c
Show file tree
Hide file tree
Showing 10 changed files with 71 additions and 103 deletions.
15 changes: 3 additions & 12 deletions llvm/include/llvm/Bitcode/LLVMBitCodes.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,8 @@ enum GlobalValueSummarySymtabCodes {
// [valueid, n x stackidindex]
FS_PERMODULE_CALLSITE_INFO = 26,
// Summary of per-module allocation memprof metadata.
// [n x (alloc type, nummib, nummib x stackidindex)]
// [nummib, nummib x (alloc type, nummib, nummib x stackidindex),
// [nummib x total size]?]
FS_PERMODULE_ALLOC_INFO = 27,
// Summary of combined index memprof callsite metadata.
// [valueid, numstackindices, numver,
Expand All @@ -316,19 +317,9 @@ enum GlobalValueSummarySymtabCodes {
// Summary of combined index allocation memprof metadata.
// [nummib, numver,
// nummib x (alloc type, numstackids, numstackids x stackidindex),
// numver x version]
// numver x version, [nummib x total size]?]
FS_COMBINED_ALLOC_INFO = 29,
FS_STACK_IDS = 30,
// Summary of per-module allocation memprof metadata when we are tracking
// total sizes.
// [n x (alloc type, total size, nummib, nummib x stackidindex)]
FS_PERMODULE_ALLOC_INFO_TOTAL_SIZES = 31,
// Summary of combined index allocation memprof metadata when we are tracking
// total sizes.
// [nummib, numver,
// nummib x (alloc type, total size, numstackids,
// numstackids x stackidindex), numver x version]
FS_COMBINED_ALLOC_INFO_TOTAL_SIZES = 32,
};

enum MetadataCodes {
Expand Down
10 changes: 5 additions & 5 deletions llvm/include/llvm/IR/ModuleSummaryIndex.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -404,8 +404,8 @@ struct AllocInfo {
std::vector<MIBInfo> MIBs;

// If requested, keep track of total profiled sizes for each MIB. This will be
// a vector of the same length and order as the MIBs vector.
std::unique_ptr<std::vector<uint64_t>> TotalSizes;
// a vector of the same length and order as the MIBs vector, if non-empty.
std::vector<uint64_t> TotalSizes;

AllocInfo(std::vector<MIBInfo> MIBs) : MIBs(std::move(MIBs)) {
Versions.push_back(0);
Expand All @@ -427,10 +427,10 @@ inline raw_ostream &operator<<(raw_ostream &OS, const AllocInfo &AE) {
for (auto &M : AE.MIBs) {
OS << "\t\t" << M << "\n";
}
if (AE.TotalSizes) {
if (!AE.TotalSizes.empty()) {
OS << " TotalSizes per MIB:\n\t\t";
First = true;
for (auto &TS : *AE.TotalSizes) {
for (auto &TS : AE.TotalSizes) {
if (!First)
OS << ", ";
First = false;
Expand Down Expand Up @@ -1445,7 +1445,7 @@ class ModuleSummaryIndex {
// in the way some record are interpreted, like flags for instance.
// Note that incrementing this may require changes in both BitcodeReader.cpp
// and BitcodeWriter.cpp.
static constexpr uint64_t BitcodeSummaryVersion = 9;
static constexpr uint64_t BitcodeSummaryVersion = 10;

// Regular LTO module name for ASM writer
static constexpr const char *getRegularLTOModuleName() {
Expand Down
3 changes: 1 addition & 2 deletions llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,6 @@ template <> struct CustomMappingTraits<GlobalValueSummaryMapTy> {
V.emplace(RefGUID, /*IsAnalysis=*/false);
Refs.push_back(ValueInfo(/*IsAnalysis=*/false, &*V.find(RefGUID)));
}
std::vector<AllocInfo> EmptyAllocInfo;
Elem.SummaryList.push_back(std::make_unique<FunctionSummary>(
GlobalValueSummary::GVFlags(
static_cast<GlobalValue::LinkageTypes>(FSum.Linkage),
Expand All @@ -239,7 +238,7 @@ template <> struct CustomMappingTraits<GlobalValueSummaryMapTy> {
std::move(FSum.TypeTestAssumeConstVCalls),
std::move(FSum.TypeCheckedLoadConstVCalls),
ArrayRef<FunctionSummary::ParamAccess>{}, ArrayRef<CallsiteInfo>{},
std::move(EmptyAllocInfo)));
ArrayRef<AllocInfo>{}));
}
}
static void output(IO &io, GlobalValueSummaryMapTy &V) {
Expand Down
6 changes: 2 additions & 4 deletions llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -548,8 +548,7 @@ static void computeFunctionSummary(
Allocs.push_back(AllocInfo(std::move(MIBs)));
if (MemProfReportHintedSizes) {
assert(Allocs.back().MIBs.size() == TotalSizes.size());
Allocs.back().TotalSizes =
std::make_unique<std::vector<uint64_t>>(std::move(TotalSizes));
Allocs.back().TotalSizes = std::move(TotalSizes);
}
} else if (!InstCallsite.empty()) {
SmallVector<unsigned> StackIdIndices;
Expand Down Expand Up @@ -948,7 +947,6 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex(
CantBePromoted.insert(GV->getGUID());
// Create the appropriate summary type.
if (Function *F = dyn_cast<Function>(GV)) {
std::vector<AllocInfo> EmptyAllocInfo;
std::unique_ptr<FunctionSummary> Summary =
std::make_unique<FunctionSummary>(
GVFlags, /*InstCount=*/0,
Expand All @@ -971,7 +969,7 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex(
ArrayRef<FunctionSummary::ConstVCall>{},
ArrayRef<FunctionSummary::ConstVCall>{},
ArrayRef<FunctionSummary::ParamAccess>{},
ArrayRef<CallsiteInfo>{}, std::move(EmptyAllocInfo));
ArrayRef<CallsiteInfo>{}, ArrayRef<AllocInfo>{});
Index.addGlobalValueSummary(*GV, std::move(Summary));
} else {
std::unique_ptr<GlobalVarSummary> Summary =
Expand Down
2 changes: 0 additions & 2 deletions llvm/lib/Bitcode/Reader/BitcodeAnalyzer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -328,8 +328,6 @@ GetCodeName(unsigned CodeID, unsigned BlockID,
STRINGIFY_CODE(FS, COMBINED_CALLSITE_INFO)
STRINGIFY_CODE(FS, COMBINED_ALLOC_INFO)
STRINGIFY_CODE(FS, STACK_IDS)
STRINGIFY_CODE(FS, PERMODULE_ALLOC_INFO_TOTAL_SIZES)
STRINGIFY_CODE(FS, COMBINED_ALLOC_INFO_TOTAL_SIZES)
}
case bitc::METADATA_ATTACHMENT_ID:
switch (CodeID) {
Expand Down
58 changes: 31 additions & 27 deletions llvm/lib/Bitcode/Reader/BitcodeReader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7991,19 +7991,17 @@ Error ModuleSummaryIndexBitcodeReader::parseEntireSummary(unsigned ID) {
break;
}

case bitc::FS_PERMODULE_ALLOC_INFO:
case bitc::FS_PERMODULE_ALLOC_INFO_TOTAL_SIZES: {
bool HasTotalSizes = BitCode == bitc::FS_PERMODULE_ALLOC_INFO_TOTAL_SIZES;
case bitc::FS_PERMODULE_ALLOC_INFO: {
unsigned I = 0;
std::vector<MIBInfo> MIBs;
std::vector<uint64_t> TotalSizes;
while (I < Record.size()) {
assert(Record.size() - I >= (HasTotalSizes ? 3 : 2));
unsigned NumMIBs = 0;
if (Version >= 10)
NumMIBs = Record[I++];
unsigned MIBsRead = 0;
while ((Version >= 10 && MIBsRead++ < NumMIBs) ||
(Version < 10 && I < Record.size())) {
assert(Record.size() - I >= 2);
AllocationType AllocType = (AllocationType)Record[I++];
if (HasTotalSizes) {
TotalSizes.push_back(Record[I++]);
assert(TotalSizes.back());
}
unsigned NumStackEntries = Record[I++];
assert(Record.size() - I >= NumStackEntries);
SmallVector<unsigned> StackIdList;
Expand All @@ -8014,32 +8012,31 @@ Error ModuleSummaryIndexBitcodeReader::parseEntireSummary(unsigned ID) {
}
MIBs.push_back(MIBInfo(AllocType, std::move(StackIdList)));
}
std::vector<uint64_t> TotalSizes;
// We either have no sizes or NumMIBs of them.
assert(I == Record.size() || Record.size() - I == NumMIBs);
if (I < Record.size()) {
MIBsRead = 0;
while (MIBsRead++ < NumMIBs)
TotalSizes.push_back(Record[I++]);
}
PendingAllocs.push_back(AllocInfo(std::move(MIBs)));
assert(HasTotalSizes != TotalSizes.empty());
if (HasTotalSizes) {
if (!TotalSizes.empty()) {
assert(PendingAllocs.back().MIBs.size() == TotalSizes.size());
PendingAllocs.back().TotalSizes =
std::make_unique<std::vector<uint64_t>>(std::move(TotalSizes));
PendingAllocs.back().TotalSizes = std::move(TotalSizes);
}
break;
}

case bitc::FS_COMBINED_ALLOC_INFO:
case bitc::FS_COMBINED_ALLOC_INFO_TOTAL_SIZES: {
bool HasTotalSizes = BitCode == bitc::FS_COMBINED_ALLOC_INFO_TOTAL_SIZES;
case bitc::FS_COMBINED_ALLOC_INFO: {
unsigned I = 0;
std::vector<MIBInfo> MIBs;
std::vector<uint64_t> TotalSizes;
unsigned NumMIBs = Record[I++];
unsigned NumVersions = Record[I++];
unsigned MIBsRead = 0;
while (MIBsRead++ < NumMIBs) {
assert(Record.size() - I >= (HasTotalSizes ? 3 : 2));
assert(Record.size() - I >= 2);
AllocationType AllocType = (AllocationType)Record[I++];
if (HasTotalSizes) {
TotalSizes.push_back(Record[I++]);
assert(TotalSizes.back());
}
unsigned NumStackEntries = Record[I++];
assert(Record.size() - I >= NumStackEntries);
SmallVector<unsigned> StackIdList;
Expand All @@ -8054,13 +8051,20 @@ Error ModuleSummaryIndexBitcodeReader::parseEntireSummary(unsigned ID) {
SmallVector<uint8_t> Versions;
for (unsigned J = 0; J < NumVersions; J++)
Versions.push_back(Record[I++]);
std::vector<uint64_t> TotalSizes;
// We either have no sizes or NumMIBs of them.
assert(I == Record.size() || Record.size() - I == NumMIBs);
if (I < Record.size()) {
MIBsRead = 0;
while (MIBsRead++ < NumMIBs) {
TotalSizes.push_back(Record[I++]);
}
}
PendingAllocs.push_back(
AllocInfo(std::move(Versions), std::move(MIBs)));
assert(HasTotalSizes != TotalSizes.empty());
if (HasTotalSizes) {
if (!TotalSizes.empty()) {
assert(PendingAllocs.back().MIBs.size() == TotalSizes.size());
PendingAllocs.back().TotalSizes =
std::make_unique<std::vector<uint64_t>>(std::move(TotalSizes));
PendingAllocs.back().TotalSizes = std::move(TotalSizes);
}
break;
}
Expand Down
72 changes: 25 additions & 47 deletions llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,7 @@ class ModuleBitcodeWriterBase : public BitcodeWriterBase {
void writePerModuleFunctionSummaryRecord(
SmallVector<uint64_t, 64> &NameVals, GlobalValueSummary *Summary,
unsigned ValueID, unsigned FSCallsAbbrev, unsigned FSCallsProfileAbbrev,
unsigned CallsiteAbbrev, unsigned AllocAbbrev,
unsigned AllocTotalSizeAbbrev, const Function &F);
unsigned CallsiteAbbrev, unsigned AllocAbbrev, const Function &F);
void writeModuleLevelReferences(const GlobalVariable &V,
SmallVector<uint64_t, 64> &NameVals,
unsigned FSModRefsAbbrev,
Expand Down Expand Up @@ -4159,7 +4158,7 @@ static void writeTypeIdCompatibleVtableSummaryRecord(

static void writeFunctionHeapProfileRecords(
BitstreamWriter &Stream, FunctionSummary *FS, unsigned CallsiteAbbrev,
unsigned AllocAbbrev, unsigned AllocTotalSizeAbbrev, bool PerModule,
unsigned AllocAbbrev, bool PerModule,
std::function<unsigned(const ValueInfo &VI)> GetValueID,
std::function<unsigned(unsigned)> GetStackIndex) {
SmallVector<uint64_t> Record;
Expand Down Expand Up @@ -4190,35 +4189,27 @@ static void writeFunctionHeapProfileRecords(
// Per module alloc versions should always have a single entry of
// value 0.
assert(!PerModule || (AI.Versions.size() == 1 && AI.Versions[0] == 0));
if (!PerModule) {
Record.push_back(AI.MIBs.size());
Record.push_back(AI.MIBs.size());
if (!PerModule)
Record.push_back(AI.Versions.size());
}
unsigned I = 0;
assert(!AI.TotalSizes || AI.TotalSizes->size() == AI.MIBs.size());
for (auto &MIB : AI.MIBs) {
Record.push_back((uint8_t)MIB.AllocType);
if (AI.TotalSizes) {
Record.push_back((*AI.TotalSizes)[I]);
assert((*AI.TotalSizes)[I]);
}
Record.push_back(MIB.StackIdIndices.size());
for (auto Id : MIB.StackIdIndices)
Record.push_back(GetStackIndex(Id));
I++;
}
if (!PerModule) {
for (auto V : AI.Versions)
Record.push_back(V);
}
unsigned Code =
PerModule ? (AI.TotalSizes ? bitc::FS_PERMODULE_ALLOC_INFO_TOTAL_SIZES
: bitc::FS_PERMODULE_ALLOC_INFO)
: (AI.TotalSizes ? bitc::FS_COMBINED_ALLOC_INFO_TOTAL_SIZES
: bitc::FS_COMBINED_ALLOC_INFO);
unsigned AllocAbbrevToUse =
AI.TotalSizes ? AllocTotalSizeAbbrev : AllocAbbrev;
Stream.EmitRecord(Code, Record, AllocAbbrevToUse);
assert(AI.TotalSizes.empty() || AI.TotalSizes.size() == AI.MIBs.size());
if (!AI.TotalSizes.empty()) {
for (auto Size : AI.TotalSizes)
Record.push_back(Size);
}
Stream.EmitRecord(PerModule ? bitc::FS_PERMODULE_ALLOC_INFO
: bitc::FS_COMBINED_ALLOC_INFO,
Record, AllocAbbrev);
}
}

Expand All @@ -4227,7 +4218,7 @@ void ModuleBitcodeWriterBase::writePerModuleFunctionSummaryRecord(
SmallVector<uint64_t, 64> &NameVals, GlobalValueSummary *Summary,
unsigned ValueID, unsigned FSCallsRelBFAbbrev,
unsigned FSCallsProfileAbbrev, unsigned CallsiteAbbrev,
unsigned AllocAbbrev, unsigned AllocTotalSizeAbbrev, const Function &F) {
unsigned AllocAbbrev, const Function &F) {
NameVals.push_back(ValueID);

FunctionSummary *FS = cast<FunctionSummary>(Summary);
Expand All @@ -4238,7 +4229,7 @@ void ModuleBitcodeWriterBase::writePerModuleFunctionSummaryRecord(
});

writeFunctionHeapProfileRecords(
Stream, FS, CallsiteAbbrev, AllocAbbrev, AllocTotalSizeAbbrev,
Stream, FS, CallsiteAbbrev, AllocAbbrev,
/*PerModule*/ true,
/*GetValueId*/ [&](const ValueInfo &VI) { return getValueId(VI); },
/*GetStackIndex*/ [&](unsigned I) { return I; });
Expand Down Expand Up @@ -4445,18 +4436,13 @@ void ModuleBitcodeWriterBase::writePerModuleGlobalValueSummary() {

Abbv = std::make_shared<BitCodeAbbrev>();
Abbv->Add(BitCodeAbbrevOp(bitc::FS_PERMODULE_ALLOC_INFO));
Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 4)); // nummib
// n x (alloc type, numstackids, numstackids x stackidindex)
// optional: nummib x total size
Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));
unsigned AllocAbbrev = Stream.EmitAbbrev(std::move(Abbv));

Abbv = std::make_shared<BitCodeAbbrev>();
Abbv->Add(BitCodeAbbrevOp(bitc::FS_PERMODULE_ALLOC_INFO_TOTAL_SIZES));
// n x (alloc type, total size, numstackids, numstackids x stackidindex)
Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));
unsigned AllocTotalSizeAbbrev = Stream.EmitAbbrev(std::move(Abbv));

SmallVector<uint64_t, 64> NameVals;
// Iterate over the list of functions instead of the Index to
// ensure the ordering is stable.
Expand All @@ -4474,10 +4460,9 @@ void ModuleBitcodeWriterBase::writePerModuleGlobalValueSummary() {
continue;
}
auto *Summary = VI.getSummaryList()[0].get();
writePerModuleFunctionSummaryRecord(NameVals, Summary, VE.getValueID(&F),
FSCallsRelBFAbbrev,
FSCallsProfileAbbrev, CallsiteAbbrev,
AllocAbbrev, AllocTotalSizeAbbrev, F);
writePerModuleFunctionSummaryRecord(
NameVals, Summary, VE.getValueID(&F), FSCallsRelBFAbbrev,
FSCallsProfileAbbrev, CallsiteAbbrev, AllocAbbrev, F);
}

// Capture references from GlobalVariable initializers, which are outside
Expand Down Expand Up @@ -4597,6 +4582,7 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 4)); // numver
// nummib x (alloc type, numstackids, numstackids x stackidindex),
// numver x version
// optional: nummib x total size
Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));
unsigned AllocAbbrev = Stream.EmitAbbrev(std::move(Abbv));
Expand All @@ -4607,16 +4593,6 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
return DecSummaries->count(GVS);
};

Abbv = std::make_shared<BitCodeAbbrev>();
Abbv->Add(BitCodeAbbrevOp(bitc::FS_COMBINED_ALLOC_INFO_TOTAL_SIZES));
Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 4)); // nummib
Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 4)); // numver
// nummib x (alloc type, total size, numstackids, numstackids x stackidindex),
// numver x version
Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));
unsigned AllocTotalSizeAbbrev = Stream.EmitAbbrev(std::move(Abbv));

// 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;
Expand Down Expand Up @@ -4704,9 +4680,10 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
getReferencedTypeIds(FS, ReferencedTypeIds);

writeFunctionHeapProfileRecords(
Stream, FS, CallsiteAbbrev, AllocAbbrev, AllocTotalSizeAbbrev,
Stream, FS, CallsiteAbbrev, AllocAbbrev,
/*PerModule*/ false,
/*GetValueId*/ [&](const ValueInfo &VI) -> unsigned {
/*GetValueId*/
[&](const ValueInfo &VI) -> unsigned {
std::optional<unsigned> ValueID = GetValueId(VI);
// This can happen in shared index files for distributed ThinLTO if
// the callee function summary is not included. Record 0 which we
Expand All @@ -4716,7 +4693,8 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
return 0;
return *ValueID;
},
/*GetStackIndex*/ [&](unsigned I) {
/*GetStackIndex*/
[&](unsigned I) {
// Get the corresponding index into the list of StackIds actually
// being written for this combined index (which may be a subset in
// the case of distributed indexes).
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1766,14 +1766,14 @@ IndexCallsiteContextGraph::IndexCallsiteContextGraph(
EmptyContext;
unsigned I = 0;
assert(!MemProfReportHintedSizes ||
(AN.TotalSizes && AN.TotalSizes->size() == AN.MIBs.size()));
AN.TotalSizes.size() == AN.MIBs.size());
// Now add all of the MIBs and their stack nodes.
for (auto &MIB : AN.MIBs) {
CallStack<MIBInfo, SmallVector<unsigned>::const_iterator>
StackContext(&MIB);
uint64_t TotalSize = 0;
if (MemProfReportHintedSizes)
TotalSize = (*AN.TotalSizes)[I];
TotalSize = AN.TotalSizes[I];
addStackNodesForMIB<MIBInfo, SmallVector<unsigned>::const_iterator>(
AllocNode, StackContext, EmptyContext, MIB.AllocType,
TotalSize);
Expand Down
Loading

0 comments on commit 8be1c3c

Please sign in to comment.