Skip to content

[ctxprof] ProfileWriter abstraction #129590

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 4, 2025
Merged

[ctxprof] ProfileWriter abstraction #129590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions compiler-rt/lib/ctx_profile/CtxInstrContextNode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,13 @@ class ContextNode final {

uint64_t entrycount() const { return counters()[0]; }
};

/// Abstraction for the parameter passed to `__llvm_ctx_profile_fetch`.
class ProfileWriter {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment to describe the interface and expected usage?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done - briefly. Updated further up the change stack, where more members appear.

public:
virtual void writeContextual(const ctx_profile::ContextNode &RootNode) = 0;
virtual ~ProfileWriter() = default;
};
} // namespace ctx_profile
} // namespace llvm
#endif
7 changes: 2 additions & 5 deletions compiler-rt/lib/ctx_profile/CtxInstrProfiling.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,7 @@ void __llvm_ctx_profile_start_collection() {
__sanitizer::Printf("[ctxprof] Initial NumMemUnits: %zu \n", NumMemUnits);
}

bool __llvm_ctx_profile_fetch(void *Data,
bool (*Writer)(void *W, const ContextNode &)) {
assert(Writer);
bool __llvm_ctx_profile_fetch(ProfileWriter &Writer) {
__sanitizer::GenericScopedLock<__sanitizer::SpinMutex> Lock(
&AllContextsMutex);

Expand All @@ -308,8 +306,7 @@ bool __llvm_ctx_profile_fetch(void *Data,
__sanitizer::Printf("[ctxprof] Contextual Profile is %s\n", "invalid");
return false;
}
if (!Writer(Data, *Root->FirstNode))
return false;
Writer.writeContextual(*Root->FirstNode);
}
return true;
}
Expand Down
3 changes: 1 addition & 2 deletions compiler-rt/lib/ctx_profile/CtxInstrProfiling.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ void __llvm_ctx_profile_free();
/// The Writer's first parameter plays the role of closure for Writer, and is
/// what the caller of __llvm_ctx_profile_fetch passes as the Data parameter.
/// The second parameter is the root of a context tree.
bool __llvm_ctx_profile_fetch(void *Data,
bool (*Writer)(void *, const ContextNode &));
bool __llvm_ctx_profile_fetch(ProfileWriter &);
}
#endif // CTX_PROFILE_CTXINSTRPROFILING_H_
21 changes: 9 additions & 12 deletions compiler-rt/lib/ctx_profile/tests/CtxInstrProfilingTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,13 +179,15 @@ TEST_F(ContextTest, Dump) {
(void)Subctx;
__llvm_ctx_profile_release_context(&Root);

struct Writer {
class TestProfileWriter : public ProfileWriter {
public:
ContextRoot *const Root;
const size_t Entries;
bool State = false;
Writer(ContextRoot *Root, size_t Entries) : Root(Root), Entries(Entries) {}
TestProfileWriter(ContextRoot *Root, size_t Entries)
: Root(Root), Entries(Entries) {}

bool write(const ContextNode &Node) {
void writeContextual(const ContextNode &Node) override {
EXPECT_FALSE(Root->Taken.TryLock());
EXPECT_EQ(Node.guid(), 1U);
EXPECT_EQ(Node.counters()[0], Entries);
Expand All @@ -202,22 +204,17 @@ TEST_F(ContextTest, Dump) {
EXPECT_EQ(SN.callsites_size(), 1U);
EXPECT_EQ(SN.subContexts()[0], nullptr);
State = true;
return true;
}
};
Writer W(&Root, 1);
TestProfileWriter W(&Root, 1);
EXPECT_FALSE(W.State);
__llvm_ctx_profile_fetch(&W, [](void *W, const ContextNode &Node) -> bool {
return reinterpret_cast<Writer *>(W)->write(Node);
});
__llvm_ctx_profile_fetch(W);
EXPECT_TRUE(W.State);

// this resets all counters but not the internal structure.
__llvm_ctx_profile_start_collection();
Writer W2(&Root, 0);
TestProfileWriter W2(&Root, 0);
EXPECT_FALSE(W2.State);
__llvm_ctx_profile_fetch(&W2, [](void *W, const ContextNode &Node) -> bool {
return reinterpret_cast<Writer *>(W)->write(Node);
});
__llvm_ctx_profile_fetch(W2);
EXPECT_TRUE(W2.State);
}
50 changes: 26 additions & 24 deletions compiler-rt/test/ctx_profile/TestCases/generate-context.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@
#include <iostream>

using namespace llvm::ctx_profile;
extern "C" bool __llvm_ctx_profile_fetch(void *Data,
bool (*Writer)(void *,
const ContextNode &));
extern "C" bool __llvm_ctx_profile_fetch(ProfileWriter &);

// avoid name mangling
extern "C" {
Expand Down Expand Up @@ -46,22 +44,29 @@ __attribute__((noinline)) void theRoot() {
// CHECK-NEXT: check even
// CHECK-NEXT: check odd

void printProfile(const ContextNode &Node, const std::string &Indent,
const std::string &Increment) {
std::cout << Indent << "Guid: " << Node.guid() << std::endl;
std::cout << Indent << "Entries: " << Node.entrycount() << std::endl;
std::cout << Indent << Node.counters_size() << " counters and "
<< Node.callsites_size() << " callsites" << std::endl;
std::cout << Indent << "Counter values: ";
for (uint32_t I = 0U; I < Node.counters_size(); ++I)
std::cout << Node.counters()[I] << " ";
std::cout << std::endl;
for (uint32_t I = 0U; I < Node.callsites_size(); ++I)
for (const auto *N = Node.subContexts()[I]; N; N = N->next()) {
std::cout << Indent << "At Index " << I << ":" << std::endl;
printProfile(*N, Indent + Increment, Increment);
}
}
class TestProfileWriter : public ProfileWriter {
void printProfile(const ContextNode &Node, const std::string &Indent,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be usefult to pass in a llvm::raw_ostream& and then writing to it. I think this will give you more flexibility based when used in tests (e.g using a string buffer).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a .cpp file that gets compiled (and run) as part of the test, so we basically have access to std:: things.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make things clear, here's an example of the usage I'm thinking of: https://github.com/llvm/llvm-project/blob/main/llvm/unittests/IR/MetadataTest.cpp#L164-L172

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make sure we're talking about the same thing: this here is a lit test. Note the RUN lines at the top of the file.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed offline and since this in compiler-rt its not useful to use ostream.

const std::string &Increment) {
std::cout << Indent << "Guid: " << Node.guid() << std::endl;
std::cout << Indent << "Entries: " << Node.entrycount() << std::endl;
std::cout << Indent << Node.counters_size() << " counters and "
<< Node.callsites_size() << " callsites" << std::endl;
std::cout << Indent << "Counter values: ";
for (uint32_t I = 0U; I < Node.counters_size(); ++I)
std::cout << Node.counters()[I] << " ";
std::cout << std::endl;
for (uint32_t I = 0U; I < Node.callsites_size(); ++I)
for (const auto *N = Node.subContexts()[I]; N; N = N->next()) {
std::cout << Indent << "At Index " << I << ":" << std::endl;
printProfile(*N, Indent + Increment, Increment);
}
}

public:
void writeContextual(const ContextNode &RootNode) override {
printProfile(RootNode, "", "");
}
};

// 8657661246551306189 is theRoot. We expect 2 callsites and 2 counters - one
// for the entry basic block and one for the loop.
Expand All @@ -88,11 +93,8 @@ void printProfile(const ContextNode &Node, const std::string &Indent,
// CHECK-NEXT: Counter values: 2 1

bool profileWriter() {
return __llvm_ctx_profile_fetch(
nullptr, +[](void *, const ContextNode &Node) {
printProfile(Node, "", " ");
return true;
});
TestProfileWriter W;
return __llvm_ctx_profile_fetch(W);
}

int main(int argc, char **argv) {
Expand Down
7 changes: 7 additions & 0 deletions llvm/include/llvm/ProfileData/CtxInstrContextNode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,13 @@ class ContextNode final {

uint64_t entrycount() const { return counters()[0]; }
};

/// Abstraction for the parameter passed to `__llvm_ctx_profile_fetch`.
class ProfileWriter {
public:
virtual void writeContextual(const ctx_profile::ContextNode &RootNode) = 0;
virtual ~ProfileWriter() = default;
};
} // namespace ctx_profile
} // namespace llvm
#endif