[PATCH] D95480: [NFC] Fix bug with parsing hybrid sample trace line

Tue Jan 26 23:35:51 PST 2021

hoy added inline comments.

================
Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:414
       TraceIt.advance();
-      break;
+      return false;
     }
----------------
wenlei wrote:
> wlei wrote:
> > wenlei wrote:
> > > This change means we no longer handle a comment line mixed in stack sample? Although I'm not sure if we ever need a comment line.. 
> > > 
> > > 
> > yeah, I think so. For the `perf` generated perfscript, I saw the only case is a log line at the head of the file, shouldn't have comments in other place. Here the comments are most for the regression test, but as you see we have to make sure it's not mixed in the sample.
> Ok, update the comment then, and we can also assert that if `FrameStr.getAsInteger(16, FrameAddr)` is true, it must be empty line. 
A comment line at the beginning of a callstack sample should still be OK, but not for a comment in the middle of a callstack sample. Previously we would just skip the frames following a comment. Will a `continue` work here if we need comments?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95480