[nfc][ctx_prof] Rename PGOContextualProfile to PGOCtxProfContext
#102209
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This better captures what the class is - just a node. The profile has a bit more info - for example, we can glean how many counters and callsites each captured function have. This is good information to have when maintaining the profile during ICP, and while it can be recomputed, it fits better (and avoids extra compute) under a to-be-introduced `PGOContextualProfile` - which is what `loadContexts()` will return (effectivelly today's map + the extra info)
mtrofin
requested review from
snehasish,
teresajohnson,
boomanaiden154 and
kazutakahirata
|
@llvm/pr-subscribers-pgo Author: Mircea Trofin (mtrofin) ChangesThis better captures what the class is - just a node. The profile has a bit more info - for example, we can glean how many counters and callsites each captured function have. This is good information to have when maintaining the profile during ICP, and while it can be recomputed, it fits better (and avoids extra compute) under a to-be-introduced Full diff: https://github.com/llvm/llvm-project/pull/102209.diff 3 Files Affected:
diff --git a/llvm/include/llvm/ProfileData/PGOCtxProfReader.h b/llvm/include/llvm/ProfileData/PGOCtxProfReader.h
index 28f05e9073a8af..b25bf7291c9dca 100644
--- a/llvm/include/llvm/ProfileData/PGOCtxProfReader.h
+++ b/llvm/include/llvm/ProfileData/PGOCtxProfReader.h
@@ -24,16 +24,16 @@
#include <vector>
namespace llvm {
-/// The loaded contextual profile, suitable for mutation during IPO passes. We
-/// generally expect a fraction of counters and of callsites to be populated.
-/// We continue to model counters as vectors, but callsites are modeled as a map
-/// of a map. The expectation is that, typically, there is a small number of
-/// indirect targets (usually, 1 for direct calls); but potentially a large
-/// number of callsites, and, as inlining progresses, the callsite count of a
-/// caller will grow.
-class PGOContextualProfile final {
+/// A node (context) in the loaded contextual profile, suitable for mutation
+/// during IPO passes. We generally expect a fraction of counters and of
+/// callsites to be populated. We continue to model counters as vectors, but
+/// callsites are modeled as a map of a map. The expectation is that, typically,
+/// there is a small number of indirect targets (usually, 1 for direct calls);
+/// but potentially a large number of callsites, and, as inlining progresses,
+/// the callsite count of a caller will grow.
+class PGOCtxProfContext final {
public:
- using CallTargetMapTy = std::map<GlobalValue::GUID, PGOContextualProfile>;
+ using CallTargetMapTy = std::map<GlobalValue::GUID, PGOCtxProfContext>;
using CallsiteMapTy = DenseMap<uint32_t, CallTargetMapTy>;
private:
@@ -42,19 +42,18 @@ class PGOContextualProfile final {
SmallVector<uint64_t, 16> Counters;
CallsiteMapTy Callsites;
- PGOContextualProfile(GlobalValue::GUID G,
- SmallVectorImpl<uint64_t> &&Counters)
+ PGOCtxProfContext(GlobalValue::GUID G, SmallVectorImpl<uint64_t> &&Counters)
: GUID(G), Counters(std::move(Counters)) {}
- Expected<PGOContextualProfile &>
+ Expected<PGOCtxProfContext &>
getOrEmplace(uint32_t Index, GlobalValue::GUID G,
SmallVectorImpl<uint64_t> &&Counters);
public:
- PGOContextualProfile(const PGOContextualProfile &) = delete;
- PGOContextualProfile &operator=(const PGOContextualProfile &) = delete;
- PGOContextualProfile(PGOContextualProfile &&) = default;
- PGOContextualProfile &operator=(PGOContextualProfile &&) = default;
+ PGOCtxProfContext(const PGOCtxProfContext &) = delete;
+ PGOCtxProfContext &operator=(const PGOCtxProfContext &) = delete;
+ PGOCtxProfContext(PGOCtxProfContext &&) = default;
+ PGOCtxProfContext &operator=(PGOCtxProfContext &&) = default;
GlobalValue::GUID guid() const { return GUID; }
const SmallVectorImpl<uint64_t> &counters() const { return Counters; }
@@ -80,7 +79,7 @@ class PGOCtxProfileReader final {
Error wrongValue(const Twine &);
Error unsupported(const Twine &);
- Expected<std::pair<std::optional<uint32_t>, PGOContextualProfile>>
+ Expected<std::pair<std::optional<uint32_t>, PGOCtxProfContext>>
readContext(bool ExpectIndex);
bool canReadContext();
@@ -89,7 +88,7 @@ class PGOCtxProfileReader final {
: Magic(Buffer.substr(0, PGOCtxProfileWriter::ContainerMagic.size())),
Cursor(Buffer.substr(PGOCtxProfileWriter::ContainerMagic.size())) {}
- Expected<std::map<GlobalValue::GUID, PGOContextualProfile>> loadContexts();
+ Expected<std::map<GlobalValue::GUID, PGOCtxProfContext>> loadContexts();
};
} // namespace llvm
#endif
diff --git a/llvm/lib/ProfileData/PGOCtxProfReader.cpp b/llvm/lib/ProfileData/PGOCtxProfReader.cpp
index 0a0e7db457fa87..8354e30bd30679 100644
--- a/llvm/lib/ProfileData/PGOCtxProfReader.cpp
+++ b/llvm/lib/ProfileData/PGOCtxProfReader.cpp
@@ -33,18 +33,18 @@ using namespace llvm;
if (auto Err = (EXPR)) \
return Err;
-Expected<PGOContextualProfile &>
-PGOContextualProfile::getOrEmplace(uint32_t Index, GlobalValue::GUID G,
- SmallVectorImpl<uint64_t> &&Counters) {
- auto [Iter, Inserted] = Callsites[Index].insert(
- {G, PGOContextualProfile(G, std::move(Counters))});
+Expected<PGOCtxProfContext &>
+PGOCtxProfContext::getOrEmplace(uint32_t Index, GlobalValue::GUID G,
+ SmallVectorImpl<uint64_t> &&Counters) {
+ auto [Iter, Inserted] =
+ Callsites[Index].insert({G, PGOCtxProfContext(G, std::move(Counters))});
if (!Inserted)
return make_error<InstrProfError>(instrprof_error::invalid_prof,
"Duplicate GUID for same callsite.");
return Iter->second;
}
-void PGOContextualProfile::getContainedGuids(
+void PGOCtxProfContext::getContainedGuids(
DenseSet<GlobalValue::GUID> &Guids) const {
Guids.insert(GUID);
for (const auto &[_, Callsite] : Callsites)
@@ -74,7 +74,7 @@ bool PGOCtxProfileReader::canReadContext() {
Blk->ID == PGOCtxProfileBlockIDs::ContextNodeBlockID;
}
-Expected<std::pair<std::optional<uint32_t>, PGOContextualProfile>>
+Expected<std::pair<std::optional<uint32_t>, PGOCtxProfContext>>
PGOCtxProfileReader::readContext(bool ExpectIndex) {
RET_ON_ERR(Cursor.EnterSubBlock(PGOCtxProfileBlockIDs::ContextNodeBlockID));
@@ -125,7 +125,7 @@ PGOCtxProfileReader::readContext(bool ExpectIndex) {
}
}
- PGOContextualProfile Ret(*Guid, std::move(*Counters));
+ PGOCtxProfContext Ret(*Guid, std::move(*Counters));
while (canReadContext()) {
EXPECT_OR_RET(SC, readContext(true));
@@ -174,9 +174,9 @@ Error PGOCtxProfileReader::readMetadata() {
return Error::success();
}
-Expected<std::map<GlobalValue::GUID, PGOContextualProfile>>
+Expected<std::map<GlobalValue::GUID, PGOCtxProfContext>>
PGOCtxProfileReader::loadContexts() {
- std::map<GlobalValue::GUID, PGOContextualProfile> Ret;
+ std::map<GlobalValue::GUID, PGOCtxProfContext> Ret;
RET_ON_ERR(readMetadata());
while (canReadContext()) {
EXPECT_OR_RET(E, readContext(false));
diff --git a/llvm/unittests/ProfileData/PGOCtxProfReaderWriterTest.cpp b/llvm/unittests/ProfileData/PGOCtxProfReaderWriterTest.cpp
index 476f293780d849..7be01445558eca 100644
--- a/llvm/unittests/ProfileData/PGOCtxProfReaderWriterTest.cpp
+++ b/llvm/unittests/ProfileData/PGOCtxProfReaderWriterTest.cpp
@@ -64,7 +64,7 @@ class PGOCtxProfRWTest : public ::testing::Test {
const std::map<GUID, const ContextNode *> &roots() const { return Roots; }
};
-void checkSame(const ContextNode &Raw, const PGOContextualProfile &Profile) {
+void checkSame(const ContextNode &Raw, const PGOCtxProfContext &Profile) {
EXPECT_EQ(Raw.guid(), Profile.guid());
ASSERT_EQ(Raw.counters_size(), Profile.counters().size());
for (auto I = 0U; I < Raw.counters_size(); ++I)
|
boomanaiden154
approved these changes
Aug 6, 2024
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/16/builds/3015 Here is the relevant piece of the build log for the reference: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This better captures what the class is - just a node. The profile has a bit more info - for example, we can glean how many counters and callsites each captured function have. This is good information to have when maintaining the profile during ICP, and while it can be recomputed, it fits better (and avoids extra compute) under a to-be-introduced
PGOContextualProfile- which is whatloadContexts()will return (effectivelly today's map + the extra info)