[PATCH] D147287: [InstrProf] Temporal Profiling

[Snehasish Kumar via Phabricator via llvm-commits]

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