-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[ctxprof] Prepare profile format for flat profiles #129626
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -190,8 +190,12 @@ class PGOCtxProfileReader final { | |
Error unsupported(const Twine &); | ||
|
||
Expected<std::pair<std::optional<uint32_t>, PGOCtxProfContext>> | ||
readContext(bool ExpectIndex); | ||
bool canReadContext(); | ||
readProfile(PGOCtxProfileBlockIDs Kind); | ||
|
||
bool canEnterBlockWithID(PGOCtxProfileBlockIDs ID); | ||
Error enterBlockWithID(PGOCtxProfileBlockIDs ID); | ||
|
||
Error loadContexts(CtxProfContextualProfiles &); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: name the parameter. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. addressed in PR #129626 |
||
|
||
public: | ||
PGOCtxProfileReader(StringRef Buffer) | ||
|
@@ -201,7 +205,6 @@ class PGOCtxProfileReader final { | |
Expected<PGOCtxProfile> loadProfiles(); | ||
}; | ||
|
||
void convertCtxProfToYaml(raw_ostream &OS, | ||
const PGOCtxProfContext::CallTargetMapTy &); | ||
void convertCtxProfToYaml(raw_ostream &OS, const PGOCtxProfile &); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: name the second param too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. addressed in PR #129626 |
||
} // namespace llvm | ||
#endif |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,7 +23,9 @@ enum PGOCtxProfileRecords { Invalid = 0, Version, Guid, CalleeIndex, Counters }; | |
|
||
enum PGOCtxProfileBlockIDs { | ||
ProfileMetadataBlockID = bitc::FIRST_APPLICATION_BLOCKID, | ||
ContextNodeBlockID = ProfileMetadataBlockID + 1 | ||
ContextsSectionBlockID = ProfileMetadataBlockID + 1, | ||
ContextRootBlockID = ContextsSectionBlockID + 1, | ||
ContextNodeBlockID = ContextRootBlockID + 1, | ||
}; | ||
|
||
/// Write one or more ContextNodes to the provided raw_fd_stream. | ||
|
@@ -60,23 +62,30 @@ enum PGOCtxProfileBlockIDs { | |
/// like value profiling - which would appear as additional records. For | ||
/// example, value profiling would produce a new record with a new record ID, | ||
/// containing the profiled values (much like the counters) | ||
class PGOCtxProfileWriter final { | ||
class PGOCtxProfileWriter : public ctx_profile::ProfileWriter { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can still keep the final keyword here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. addressed in PR #129626 |
||
enum class EmptyContextCriteria { None, EntryIsZero, AllAreZero }; | ||
|
||
BitstreamWriter Writer; | ||
const bool IncludeEmpty; | ||
|
||
void writeCounters(const ctx_profile::ContextNode &Node); | ||
void writeGuid(ctx_profile::GUID Guid); | ||
void writeCounters(ArrayRef<uint64_t> Counters); | ||
void writeImpl(std::optional<uint32_t> CallerIndex, | ||
const ctx_profile::ContextNode &Node); | ||
|
||
public: | ||
PGOCtxProfileWriter(raw_ostream &Out, | ||
std::optional<unsigned> VersionOverride = std::nullopt); | ||
std::optional<unsigned> VersionOverride = std::nullopt, | ||
bool IncludeEmpty = false); | ||
~PGOCtxProfileWriter() { Writer.ExitBlock(); } | ||
|
||
void write(const ctx_profile::ContextNode &); | ||
void startContextSection() override; | ||
void writeContextual(const ctx_profile::ContextNode &RootNode) override; | ||
void endContextSection() override; | ||
|
||
// constants used in writing which a reader may find useful. | ||
static constexpr unsigned CodeLen = 2; | ||
static constexpr uint32_t CurrentVersion = 1; | ||
static constexpr uint32_t CurrentVersion = 2; | ||
static constexpr unsigned VBREncodingBits = 6; | ||
static constexpr StringRef ContainerMagic = "CTXP"; | ||
}; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could have this method return an object which runs
endContextSection
on destruction. Then we wouldn't have to worry about pairing up the calls to start and end each time. Perhaps it's too much for a simple interface like this though.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought of that but yes, seemed overkill. It's a low-level interface, really meant to go between compiler-rt and whatever service handles writing the profile.