[PATCH] D147287: [InstrProf] Temporal Profiling
Snehasish Kumar via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 6 16:26:41 PDT 2023
snehasish added inline comments.
================
Comment at: llvm/include/llvm/ProfileData/InstrProfReader.h:138
+ /// Return true if this is a temporal profile.
+ virtual bool hasTemporalProfile() const = 0;
----------------
nit: s/is/has
================
Comment at: llvm/lib/ProfileData/InstrProfWriter.cpp:440
Header.BinaryIdOffset = 0;
+ Header.FunctionTracesOffset = 0;
int N = sizeof(IndexedInstrProf::Header) / sizeof(uint64_t);
----------------
ellis wrote:
> snehasish wrote:
> > There seem to be a few different phrases referring to the same functionality, e.g. `TemporalProfile`, `FunctionTraces` and just `Trace` in other places. It would be nice to have a single prefix for all the var and function names to be able to quickly navigate the code. What do you think?
> I see what you mean. I'm using `FunctionTrace` and `Trace` interchangeably and they aren't very descriptive. What do you think of the `TP` (Temporal Profiling) prefix? So I would use `TPTrace` in code and `:tp_traces` for the text format.
I prefer something more verbose like "TemporalProf", but up to you to decide, "TP" as the prefix is fine too.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D147287/new/
https://reviews.llvm.org/D147287
More information about the llvm-commits
mailing list