[llvm] 776bb27 - [llvm-profdata] ProfileReader cleanup - preparation for MD5 refactoring - 2
William Huang via llvm-commits
llvm-commits at lists.llvm.org
Mon May 8 21:38:27 PDT 2023
Author: William Huang
Date: 2023-05-09T04:38:01Z
New Revision: 776bb279d6429ae8a47f05a903aef34dcd7f7657
URL: https://github.com/llvm/llvm-project/commit/776bb279d6429ae8a47f05a903aef34dcd7f7657
DIFF: https://github.com/llvm/llvm-project/commit/776bb279d6429ae8a47f05a903aef34dcd7f7657.diff
LOG: [llvm-profdata] ProfileReader cleanup - preparation for MD5 refactoring - 2
Cleanup profile reader classes to prepare for complex refactoring as propsed in D147740, continuing D148868
This is patch 2/n. This patch refactors CSNameTable and related things
The decision to move CSNameTable up to the base class is because a planned improvement (D147740) to use MD5 to lookup Functions/Context frames. In this case we want a unified data structure between contextless function or Context frames, so that it can be mapped by MD5 value. Since Context Frames can represent contextless functions, it is being used for MD5 lookup, therefore exposing it to the base class
Reviewed By: snehasish, wenlei
Differential Revision: https://reviews.llvm.org/D148872
Added:
Modified:
llvm/include/llvm/ProfileData/SampleProfReader.h
llvm/lib/ProfileData/SampleProfReader.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/ProfileData/SampleProfReader.h b/llvm/include/llvm/ProfileData/SampleProfReader.h
index 1849417f31ee2..a490e9ca6e723 100644
--- a/llvm/include/llvm/ProfileData/SampleProfReader.h
+++ b/llvm/include/llvm/ProfileData/SampleProfReader.h
@@ -657,6 +657,13 @@ class SampleProfileReaderBinary : public SampleProfileReader {
/// Read a string indirectly via the name table.
ErrorOr<StringRef> readStringFromTable();
+ /// Read a context indirectly via the CSNameTable.
+ ErrorOr<SampleContextFrames> readContextFromTable();
+
+ /// Read a context indirectly via the CSNameTable if the profile has context,
+ /// otherwise same as readStringFromTable.
+ ErrorOr<SampleContext> readSampleContextFromTable();
+
/// Points to the current location in the buffer.
const uint8_t *Data = nullptr;
@@ -678,7 +685,9 @@ class SampleProfileReaderBinary : public SampleProfileReader {
/// The starting address of NameTable containing fixed length MD5.
const uint8_t *MD5NameMemStart = nullptr;
- virtual ErrorOr<SampleContext> readSampleContextFromTable();
+ /// CSNameTable is used to save full context vectors. It is the backing buffer
+ /// for SampleContextFrames.
+ std::vector<SampleContextFrameVector> CSNameTable;
private:
std::error_code readSummaryEntry(std::vector<ProfileSummaryEntry> &Entries);
@@ -746,8 +755,6 @@ class SampleProfileReaderExtBinaryBase : public SampleProfileReaderBinary {
const SecHdrTableEntry &Entry);
// placeholder for subclasses to dispatch their own section readers.
virtual std::error_code readCustomSection(const SecHdrTableEntry &Entry) = 0;
- ErrorOr<SampleContext> readSampleContextFromTable() override;
- ErrorOr<SampleContextFrames> readContextFromTable();
std::unique_ptr<ProfileSymbolList> ProfSymList;
@@ -762,10 +769,6 @@ class SampleProfileReaderExtBinaryBase : public SampleProfileReaderBinary {
/// The set containing the functions to use when compiling a module.
DenseSet<StringRef> FuncsToUse;
- /// CSNameTable is used to save full context vectors. This serves as an
- /// underlying immutable buffer for all clients.
- std::unique_ptr<const std::vector<SampleContextFrameVector>> CSNameTable;
-
/// If SkipFlatProf is true, skip the sections with
/// SecFlagFlat flag.
bool SkipFlatProf = false;
diff --git a/llvm/lib/ProfileData/SampleProfReader.cpp b/llvm/lib/ProfileData/SampleProfReader.cpp
index 977ea90039b00..964f7606d0c13 100644
--- a/llvm/lib/ProfileData/SampleProfReader.cpp
+++ b/llvm/lib/ProfileData/SampleProfReader.cpp
@@ -546,11 +546,27 @@ ErrorOr<StringRef> SampleProfileReaderBinary::readStringFromTable() {
return SR;
}
-ErrorOr<SampleContext> SampleProfileReaderBinary::readSampleContextFromTable() {
- auto FName(readStringFromTable());
- if (std::error_code EC = FName.getError())
+ErrorOr<SampleContextFrames> SampleProfileReaderBinary::readContextFromTable() {
+ auto ContextIdx = readNumber<size_t>();
+ if (std::error_code EC = ContextIdx.getError())
return EC;
- return SampleContext(*FName);
+ if (*ContextIdx >= CSNameTable.size())
+ return sampleprof_error::truncated_name_table;
+ return CSNameTable[*ContextIdx];
+}
+
+ErrorOr<SampleContext> SampleProfileReaderBinary::readSampleContextFromTable() {
+ if (ProfileIsCS) {
+ auto FContext(readContextFromTable());
+ if (std::error_code EC = FContext.getError())
+ return EC;
+ return SampleContext(*FContext);
+ } else {
+ auto FName(readStringFromTable());
+ if (std::error_code EC = FName.getError())
+ return EC;
+ return SampleContext(*FName);
+ }
}
std::error_code
@@ -671,31 +687,6 @@ std::error_code SampleProfileReaderBinary::readImpl() {
return sampleprof_error::success;
}
-ErrorOr<SampleContextFrames>
-SampleProfileReaderExtBinaryBase::readContextFromTable() {
- auto ContextIdx = readNumber<size_t>();
- if (std::error_code EC = ContextIdx.getError())
- return EC;
- if (*ContextIdx >= CSNameTable->size())
- return sampleprof_error::truncated_name_table;
- return (*CSNameTable)[*ContextIdx];
-}
-
-ErrorOr<SampleContext>
-SampleProfileReaderExtBinaryBase::readSampleContextFromTable() {
- if (ProfileIsCS) {
- auto FContext(readContextFromTable());
- if (std::error_code EC = FContext.getError())
- return EC;
- return SampleContext(*FContext);
- } else {
- auto FName(readStringFromTable());
- if (std::error_code EC = FName.getError())
- return EC;
- return SampleContext(*FName);
- }
-}
-
std::error_code SampleProfileReaderExtBinaryBase::readOneSection(
const uint8_t *Start, uint64_t Size, const SecHdrTableEntry &Entry) {
Data = Start;
@@ -1035,7 +1026,7 @@ std::error_code
SampleProfileReaderExtBinaryBase::readNameTableSec(bool IsMD5,
bool FixedLengthMD5) {
if (FixedLengthMD5) {
- if (IsMD5)
+ if (!IsMD5)
errs() << "If FixedLengthMD5 is true, UseMD5 has to be true";
auto Size = readNumber<size_t>();
if (std::error_code EC = Size.getError())
@@ -1089,11 +1080,10 @@ std::error_code SampleProfileReaderExtBinaryBase::readCSNameTableSec() {
if (std::error_code EC = Size.getError())
return EC;
- std::vector<SampleContextFrameVector> *PNameVec =
- new std::vector<SampleContextFrameVector>();
- PNameVec->reserve(*Size);
+ CSNameTable.clear();
+ CSNameTable.reserve(*Size);
for (size_t I = 0; I < *Size; ++I) {
- PNameVec->emplace_back(SampleContextFrameVector());
+ CSNameTable.emplace_back(SampleContextFrameVector());
auto ContextSize = readNumber<uint32_t>();
if (std::error_code EC = ContextSize.getError())
return EC;
@@ -1112,18 +1102,15 @@ std::error_code SampleProfileReaderExtBinaryBase::readCSNameTableSec() {
if (std::error_code EC = Discriminator.getError())
return EC;
- PNameVec->back().emplace_back(
+ CSNameTable.back().emplace_back(
FName.get(), LineLocation(LineOffset.get(), Discriminator.get()));
}
}
- // From this point the underlying object of CSNameTable should be immutable.
- CSNameTable.reset(PNameVec);
return sampleprof_error::success;
}
std::error_code
-
SampleProfileReaderExtBinaryBase::readFuncMetadata(bool ProfileHasAttribute,
FunctionSamples *FProfile) {
if (Data < End) {
More information about the llvm-commits
mailing list