[llvm] [SampleFDO][NFC] Refactoring sample reader to support on-demand read profiles for given functions (PR #104654)
Lei Wang via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 19 10:48:28 PDT 2024
https://github.com/wlei-llvm updated https://github.com/llvm/llvm-project/pull/104654
>From db555d5231f99082b1bf83b43d612336c6e6cdb5 Mon Sep 17 00:00:00 2001
From: wlei <wlei at fb.com>
Date: Fri, 16 Aug 2024 17:28:24 -0700
Subject: [PATCH 1/2] [SampleFDO] Refactoring to sample reader to support
on-demand read profiles for given functions
---
.../llvm/ProfileData/SampleProfReader.h | 39 +++
llvm/lib/ProfileData/SampleProfReader.cpp | 225 +++++++++++-------
2 files changed, 179 insertions(+), 85 deletions(-)
diff --git a/llvm/include/llvm/ProfileData/SampleProfReader.h b/llvm/include/llvm/ProfileData/SampleProfReader.h
index f4bdc6525308d2..7a29cda9d4fcc5 100644
--- a/llvm/include/llvm/ProfileData/SampleProfReader.h
+++ b/llvm/include/llvm/ProfileData/SampleProfReader.h
@@ -380,6 +380,22 @@ class SampleProfileReader {
return sampleprof_error::success;
}
+ /// Read sample profiles for the given functions. Currently it's only used for
+ /// extended binary format to load the profiles on-demand.
+ std::error_code read(const DenseSet<StringRef> &FuncsToUse) {
+ if (std::error_code EC = read(FuncsToUse, Profiles))
+ return EC;
+ return sampleprof_error::success;
+ };
+
+ /// Read sample profiles for the given functions and write them to the given
+ /// profile map. Currently it's only used for extended binary format to load
+ /// the profiles on-demand.
+ virtual std::error_code read(const DenseSet<StringRef> &FuncsToUse,
+ SampleProfileMap &Profiles) {
+ return sampleprof_error::not_implemented;
+ };
+
/// The implementaion to read sample profiles from the associated file.
virtual std::error_code readImpl() = 0;
@@ -522,6 +538,16 @@ class SampleProfileReader {
std::unique_ptr<SampleProfileReaderItaniumRemapper> Remapper;
+ // A map from a function's context hash to its meta data section range, used
+ // for on-demand read function profile metadata.
+ std::unordered_map<uint64_t, std::pair<const uint8_t *, const uint8_t *>>
+ FuncMetadataIndex;
+
+ std::pair<const uint8_t *, const uint8_t *> LBRProfileSecRange;
+
+ /// Whether the profile has attribute metadata.
+ bool ProfileHasAttribute = false;
+
/// \brief Whether samples are collected based on pseudo probes.
bool ProfileIsProbeBased = false;
@@ -621,6 +647,8 @@ class SampleProfileReaderBinary : public SampleProfileReader {
/// Read the next function profile instance.
std::error_code readFuncProfile(const uint8_t *Start);
+ std::error_code readFuncProfile(const uint8_t *Start,
+ SampleProfileMap &Profiles);
/// Read the contents of the given profile instance.
std::error_code readProfile(FunctionSamples &FProfile);
@@ -720,11 +748,15 @@ class SampleProfileReaderExtBinaryBase : public SampleProfileReaderBinary {
std::error_code readSecHdrTableEntry(uint64_t Idx);
std::error_code readSecHdrTable();
+ std::error_code readFuncMetadata(bool ProfileHasAttribute,
+ SampleProfileMap &Profiles);
std::error_code readFuncMetadata(bool ProfileHasAttribute);
std::error_code readFuncMetadata(bool ProfileHasAttribute,
FunctionSamples *FProfile);
std::error_code readFuncOffsetTable();
std::error_code readFuncProfiles();
+ std::error_code readFuncProfiles(const DenseSet<StringRef> &FuncsToUse,
+ SampleProfileMap &Profiles);
std::error_code readNameTableSec(bool IsMD5, bool FixedLengthMD5);
std::error_code readCSNameTableSec();
std::error_code readProfileSymbolList();
@@ -776,6 +808,13 @@ class SampleProfileReaderExtBinaryBase : public SampleProfileReaderBinary {
/// the reader has been given a module.
bool collectFuncsFromModule() override;
+ /// Read the profiles on-demand for the given functions. This is used after
+ /// stale call graph matching finds new functions whose profiles aren't loaded
+ /// at the beginning and we need to loaded the profiles explicitly for
+ /// potential matching.
+ std::error_code read(const DenseSet<StringRef> &FuncsToUse,
+ SampleProfileMap &Profiles) override;
+
std::unique_ptr<ProfileSymbolList> getProfileSymbolList() override {
return std::move(ProfSymList);
};
diff --git a/llvm/lib/ProfileData/SampleProfReader.cpp b/llvm/lib/ProfileData/SampleProfReader.cpp
index 4752465fc072e0..48c4c6ad558ccc 100644
--- a/llvm/lib/ProfileData/SampleProfReader.cpp
+++ b/llvm/lib/ProfileData/SampleProfReader.cpp
@@ -653,7 +653,8 @@ SampleProfileReaderBinary::readProfile(FunctionSamples &FProfile) {
}
std::error_code
-SampleProfileReaderBinary::readFuncProfile(const uint8_t *Start) {
+SampleProfileReaderBinary::readFuncProfile(const uint8_t *Start,
+ SampleProfileMap &Profiles) {
Data = Start;
auto NumHeadSamples = readNumber<uint64_t>();
if (std::error_code EC = NumHeadSamples.getError())
@@ -678,6 +679,11 @@ SampleProfileReaderBinary::readFuncProfile(const uint8_t *Start) {
return sampleprof_error::success;
}
+std::error_code
+SampleProfileReaderBinary::readFuncProfile(const uint8_t *Start) {
+ return readFuncProfile(Start, Profiles);
+}
+
std::error_code SampleProfileReaderBinary::readImpl() {
ProfileIsFS = ProfileIsFSDisciminator;
FunctionSamples::ProfileIsFS = ProfileIsFS;
@@ -725,6 +731,7 @@ std::error_code SampleProfileReaderExtBinaryBase::readOneSection(
break;
}
case SecLBRProfile:
+ LBRProfileSecRange = std::make_pair(Data, End);
if (std::error_code EC = readFuncProfiles())
return EC;
break;
@@ -745,9 +752,9 @@ std::error_code SampleProfileReaderExtBinaryBase::readOneSection(
ProfileIsProbeBased =
hasSecFlag(Entry, SecFuncMetadataFlags::SecFlagIsProbeBased);
FunctionSamples::ProfileIsProbeBased = ProfileIsProbeBased;
- bool HasAttribute =
+ ProfileHasAttribute =
hasSecFlag(Entry, SecFuncMetadataFlags::SecFlagHasAttribute);
- if (std::error_code EC = readFuncMetadata(HasAttribute))
+ if (std::error_code EC = readFuncMetadata(ProfileHasAttribute))
return EC;
break;
}
@@ -791,6 +798,19 @@ bool SampleProfileReaderExtBinaryBase::useFuncOffsetList() const {
return false;
}
+std::error_code
+SampleProfileReaderExtBinaryBase::read(const DenseSet<StringRef> &FuncsToUse,
+ SampleProfileMap &Profiles) {
+ Data = LBRProfileSecRange.first;
+ End = LBRProfileSecRange.second;
+ if (std::error_code EC = readFuncProfiles(FuncsToUse, Profiles))
+ return EC;
+ End = Data;
+
+ if (std::error_code EC = readFuncMetadata(ProfileHasAttribute, Profiles))
+ return EC;
+ return sampleprof_error::success;
+}
bool SampleProfileReaderExtBinaryBase::collectFuncsFromModule() {
if (!M)
@@ -838,6 +858,97 @@ std::error_code SampleProfileReaderExtBinaryBase::readFuncOffsetTable() {
return sampleprof_error::success;
}
+std::error_code SampleProfileReaderExtBinaryBase::readFuncProfiles(
+ const DenseSet<StringRef> &FuncsToUse, SampleProfileMap &Profiles) {
+ const uint8_t *Start = Data;
+
+ if (Remapper) {
+ for (auto Name : FuncsToUse) {
+ Remapper->insert(Name);
+ }
+ }
+
+ if (ProfileIsCS) {
+ assert(useFuncOffsetList());
+ DenseSet<uint64_t> FuncGuidsToUse;
+ if (useMD5()) {
+ for (auto Name : FuncsToUse)
+ FuncGuidsToUse.insert(Function::getGUID(Name));
+ }
+
+ // For each function in current module, load all context profiles for
+ // the function as well as their callee contexts which can help profile
+ // guided importing for ThinLTO. This can be achieved by walking
+ // through an ordered context container, where contexts are laid out
+ // as if they were walked in preorder of a context trie. While
+ // traversing the trie, a link to the highest common ancestor node is
+ // kept so that all of its decendants will be loaded.
+ const SampleContext *CommonContext = nullptr;
+ for (const auto &NameOffset : FuncOffsetList) {
+ const auto &FContext = NameOffset.first;
+ FunctionId FName = FContext.getFunction();
+ StringRef FNameString;
+ if (!useMD5())
+ FNameString = FName.stringRef();
+
+ // For function in the current module, keep its farthest ancestor
+ // context. This can be used to load itself and its child and
+ // sibling contexts.
+ if ((useMD5() && FuncGuidsToUse.count(FName.getHashCode())) ||
+ (!useMD5() && (FuncsToUse.count(FNameString) ||
+ (Remapper && Remapper->exist(FNameString))))) {
+ if (!CommonContext || !CommonContext->isPrefixOf(FContext))
+ CommonContext = &FContext;
+ }
+
+ if (CommonContext == &FContext ||
+ (CommonContext && CommonContext->isPrefixOf(FContext))) {
+ // Load profile for the current context which originated from
+ // the common ancestor.
+ const uint8_t *FuncProfileAddr = Start + NameOffset.second;
+ if (std::error_code EC = readFuncProfile(FuncProfileAddr))
+ return EC;
+ }
+ }
+ } else if (useMD5()) {
+ assert(!useFuncOffsetList());
+ for (auto Name : FuncsToUse) {
+ auto GUID = MD5Hash(Name);
+ auto iter = FuncOffsetTable.find(GUID);
+ if (iter == FuncOffsetTable.end())
+ continue;
+ const uint8_t *FuncProfileAddr = Start + iter->second;
+ if (std::error_code EC = readFuncProfile(FuncProfileAddr, Profiles))
+ return EC;
+ }
+ } else if (Remapper) {
+ assert(useFuncOffsetList());
+ for (auto NameOffset : FuncOffsetList) {
+ SampleContext FContext(NameOffset.first);
+ auto FuncName = FContext.getFunction();
+ StringRef FuncNameStr = FuncName.stringRef();
+ if (!FuncsToUse.count(FuncNameStr) && !Remapper->exist(FuncNameStr))
+ continue;
+ const uint8_t *FuncProfileAddr = Start + NameOffset.second;
+ if (std::error_code EC = readFuncProfile(FuncProfileAddr, Profiles))
+ return EC;
+ }
+ } else {
+ assert(!useFuncOffsetList());
+ for (auto Name : FuncsToUse) {
+
+ auto iter = FuncOffsetTable.find(MD5Hash(Name));
+ if (iter == FuncOffsetTable.end())
+ continue;
+ const uint8_t *FuncProfileAddr = Start + iter->second;
+ if (std::error_code EC = readFuncProfile(FuncProfileAddr, Profiles))
+ return EC;
+ }
+ }
+
+ return sampleprof_error::success;
+}
+
std::error_code SampleProfileReaderExtBinaryBase::readFuncProfiles() {
// Collect functions used by current module if the Reader has been
// given a module.
@@ -858,88 +969,8 @@ std::error_code SampleProfileReaderExtBinaryBase::readFuncProfiles() {
assert(Data == End && "More data is read than expected");
} else {
// Load function profiles on demand.
- if (Remapper) {
- for (auto Name : FuncsToUse) {
- Remapper->insert(Name);
- }
- }
-
- if (ProfileIsCS) {
- assert(useFuncOffsetList());
- DenseSet<uint64_t> FuncGuidsToUse;
- if (useMD5()) {
- for (auto Name : FuncsToUse)
- FuncGuidsToUse.insert(Function::getGUID(Name));
- }
-
- // For each function in current module, load all context profiles for
- // the function as well as their callee contexts which can help profile
- // guided importing for ThinLTO. This can be achieved by walking
- // through an ordered context container, where contexts are laid out
- // as if they were walked in preorder of a context trie. While
- // traversing the trie, a link to the highest common ancestor node is
- // kept so that all of its decendants will be loaded.
- const SampleContext *CommonContext = nullptr;
- for (const auto &NameOffset : FuncOffsetList) {
- const auto &FContext = NameOffset.first;
- FunctionId FName = FContext.getFunction();
- StringRef FNameString;
- if (!useMD5())
- FNameString = FName.stringRef();
-
- // For function in the current module, keep its farthest ancestor
- // context. This can be used to load itself and its child and
- // sibling contexts.
- if ((useMD5() && FuncGuidsToUse.count(FName.getHashCode())) ||
- (!useMD5() && (FuncsToUse.count(FNameString) ||
- (Remapper && Remapper->exist(FNameString))))) {
- if (!CommonContext || !CommonContext->isPrefixOf(FContext))
- CommonContext = &FContext;
- }
-
- if (CommonContext == &FContext ||
- (CommonContext && CommonContext->isPrefixOf(FContext))) {
- // Load profile for the current context which originated from
- // the common ancestor.
- const uint8_t *FuncProfileAddr = Start + NameOffset.second;
- if (std::error_code EC = readFuncProfile(FuncProfileAddr))
- return EC;
- }
- }
- } else if (useMD5()) {
- assert(!useFuncOffsetList());
- for (auto Name : FuncsToUse) {
- auto GUID = MD5Hash(Name);
- auto iter = FuncOffsetTable.find(GUID);
- if (iter == FuncOffsetTable.end())
- continue;
- const uint8_t *FuncProfileAddr = Start + iter->second;
- if (std::error_code EC = readFuncProfile(FuncProfileAddr))
- return EC;
- }
- } else if (Remapper) {
- assert(useFuncOffsetList());
- for (auto NameOffset : FuncOffsetList) {
- SampleContext FContext(NameOffset.first);
- auto FuncName = FContext.getFunction();
- StringRef FuncNameStr = FuncName.stringRef();
- if (!FuncsToUse.count(FuncNameStr) && !Remapper->exist(FuncNameStr))
- continue;
- const uint8_t *FuncProfileAddr = Start + NameOffset.second;
- if (std::error_code EC = readFuncProfile(FuncProfileAddr))
- return EC;
- }
- } else {
- assert(!useFuncOffsetList());
- for (auto Name : FuncsToUse) {
- auto iter = FuncOffsetTable.find(MD5Hash(Name));
- if (iter == FuncOffsetTable.end())
- continue;
- const uint8_t *FuncProfileAddr = Start + iter->second;
- if (std::error_code EC = readFuncProfile(FuncProfileAddr))
- return EC;
- }
- }
+ if (std::error_code EC = readFuncProfiles(FuncsToUse, Profiles))
+ return EC;
Data = End;
}
assert((CSProfileCount == 0 || CSProfileCount == Profiles.size()) &&
@@ -1245,6 +1276,27 @@ SampleProfileReaderExtBinaryBase::readFuncMetadata(bool ProfileHasAttribute,
return sampleprof_error::success;
}
+std::error_code
+SampleProfileReaderExtBinaryBase::readFuncMetadata(bool ProfileHasAttribute,
+ SampleProfileMap &Profiles) {
+ if (FuncMetadataIndex.empty())
+ return sampleprof_error::success;
+
+ for (auto &I : Profiles) {
+ FunctionSamples *FProfile = &I.second;
+ auto R = FuncMetadataIndex.find(FProfile->getContext().getHashCode());
+ if (R == FuncMetadataIndex.end())
+ continue;
+
+ Data = R->second.first;
+ End = R->second.second;
+ if (std::error_code EC = readFuncMetadata(ProfileHasAttribute, FProfile))
+ return EC;
+ assert(Data == End && "More data is read than expected");
+ }
+ return sampleprof_error::success;
+}
+
std::error_code
SampleProfileReaderExtBinaryBase::readFuncMetadata(bool ProfileHasAttribute) {
while (Data < End) {
@@ -1257,8 +1309,11 @@ SampleProfileReaderExtBinaryBase::readFuncMetadata(bool ProfileHasAttribute) {
if (It != Profiles.end())
FProfile = &It->second;
+ const uint8_t *Start = Data;
if (std::error_code EC = readFuncMetadata(ProfileHasAttribute, FProfile))
return EC;
+
+ FuncMetadataIndex[FContext.getHashCode()] = {Start, Data};
}
assert(Data == End && "More data is read than expected");
>From 928bcd4dfc19e66727920e2df67d4cd9772fa6bd Mon Sep 17 00:00:00 2001
From: wlei <wlei at fb.com>
Date: Mon, 19 Aug 2024 10:45:22 -0700
Subject: [PATCH 2/2] check whether a profile is already loaded
---
llvm/include/llvm/ProfileData/SampleProfReader.h | 15 +++++++++------
llvm/lib/ProfileData/SampleProfReader.cpp | 7 +++----
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/llvm/include/llvm/ProfileData/SampleProfReader.h b/llvm/include/llvm/ProfileData/SampleProfReader.h
index 7a29cda9d4fcc5..5ce4518858e875 100644
--- a/llvm/include/llvm/ProfileData/SampleProfReader.h
+++ b/llvm/include/llvm/ProfileData/SampleProfReader.h
@@ -380,13 +380,16 @@ class SampleProfileReader {
return sampleprof_error::success;
}
- /// Read sample profiles for the given functions. Currently it's only used for
- /// extended binary format to load the profiles on-demand.
+ /// Read sample profiles for the given functions.
std::error_code read(const DenseSet<StringRef> &FuncsToUse) {
- if (std::error_code EC = read(FuncsToUse, Profiles))
+ DenseSet<StringRef> S;
+ for (StringRef F : FuncsToUse)
+ if (Profiles.find(FunctionId(F)) == Profiles.end())
+ S.insert(F);
+ if (std::error_code EC = read(S, Profiles))
return EC;
return sampleprof_error::success;
- };
+ }
/// Read sample profiles for the given functions and write them to the given
/// profile map. Currently it's only used for extended binary format to load
@@ -394,7 +397,7 @@ class SampleProfileReader {
virtual std::error_code read(const DenseSet<StringRef> &FuncsToUse,
SampleProfileMap &Profiles) {
return sampleprof_error::not_implemented;
- };
+ }
/// The implementaion to read sample profiles from the associated file.
virtual std::error_code readImpl() = 0;
@@ -543,7 +546,7 @@ class SampleProfileReader {
std::unordered_map<uint64_t, std::pair<const uint8_t *, const uint8_t *>>
FuncMetadataIndex;
- std::pair<const uint8_t *, const uint8_t *> LBRProfileSecRange;
+ std::pair<const uint8_t *, const uint8_t *> ProfileSecRange;
/// Whether the profile has attribute metadata.
bool ProfileHasAttribute = false;
diff --git a/llvm/lib/ProfileData/SampleProfReader.cpp b/llvm/lib/ProfileData/SampleProfReader.cpp
index 48c4c6ad558ccc..71464e8dae65ce 100644
--- a/llvm/lib/ProfileData/SampleProfReader.cpp
+++ b/llvm/lib/ProfileData/SampleProfReader.cpp
@@ -731,7 +731,7 @@ std::error_code SampleProfileReaderExtBinaryBase::readOneSection(
break;
}
case SecLBRProfile:
- LBRProfileSecRange = std::make_pair(Data, End);
+ ProfileSecRange = std::make_pair(Data, End);
if (std::error_code EC = readFuncProfiles())
return EC;
break;
@@ -801,8 +801,8 @@ bool SampleProfileReaderExtBinaryBase::useFuncOffsetList() const {
std::error_code
SampleProfileReaderExtBinaryBase::read(const DenseSet<StringRef> &FuncsToUse,
SampleProfileMap &Profiles) {
- Data = LBRProfileSecRange.first;
- End = LBRProfileSecRange.second;
+ Data = ProfileSecRange.first;
+ End = ProfileSecRange.second;
if (std::error_code EC = readFuncProfiles(FuncsToUse, Profiles))
return EC;
End = Data;
@@ -960,7 +960,6 @@ std::error_code SampleProfileReaderExtBinaryBase::readFuncProfiles() {
// When LoadFuncsToBeUsed is false, we are using LLVM tool, need to read all
// profiles.
- const uint8_t *Start = Data;
if (!LoadFuncsToBeUsed) {
while (Data < End) {
if (std::error_code EC = readFuncProfile(Data))
More information about the llvm-commits
mailing list