[PATCH] D147287: [InstrProf] Temporal Profiling

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: compiler-rt/include/profile/InstrProfData.inc:657
 
 /* Profile version is always of type uint64_t. Reserve the upper 8 bits in the
  * version for other variants of profile. We set the lowest bit of the upper 8
----------------
8 bits --> 12 bits


================
Comment at: compiler-rt/lib/profile/InstrProfiling.c:27
+void INSTR_PROF_PROFILE_SET_TIMESTAMP(uint64_t *Probe) {
+  if (*Probe == 0 || *Probe == (uint64_t)-1)
+    *Probe = __llvm_profile_global_timestamp++;
----------------
what is -1 value reserved for?


================
Comment at: llvm/include/llvm/ProfileData/InstrProfData.inc:657
 
 /* Profile version is always of type uint64_t. Reserve the upper 8 bits in the
  * version for other variants of profile. We set the lowest bit of the upper 8
----------------
8->12


================
Comment at: llvm/lib/ProfileData/InstrProfReader.cpp:281
 
+Error TextInstrProfReader::readTraceData() {
+  if ((++Line).is_at_end())
----------------
Describe trace text format as comments.


================
Comment at: llvm/lib/ProfileData/InstrProfReader.cpp:644
+      if (hasSingleByteCoverage())
+        I += 7;
+      continue;
----------------
what is this increment?


================
Comment at: llvm/lib/ProfileData/InstrProfWriter.cpp:311
+
+void InstrProfWriter::addFunctionTraces(SmallVector<InstrProfTraceTy> SrcTraces,
+                                        uint64_t SrcTraceStreamSize) {
----------------
Is it better to do  deeper merge, e.g.   b -c -d  can be merged with a-b-c-d-e,   and a-b and be merged with b-c.   A trace count can also be added for each merged trace to show the weight.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147287/new/

https://reviews.llvm.org/D147287