[PATCH] D108153: [llvm-profgen] Support LBR only perf script

Wenlei He via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 30 12:37:10 PDT 2021


wenlei accepted this revision.
wenlei added a comment.

lgtm with minor comments, thanks.



================
Comment at: llvm/tools/llvm-profgen/PerfReader.h:612-613
   virtual void generateRawProfile() = 0;
+  virtual void showRawProfile(StringRef Filename);
+  virtual void showRawProfile(raw_fd_ostream &OS) = 0;
 
----------------
nit: now that this is output to a file instead of stdout, rename it as writeRawProfile?


================
Comment at: llvm/tools/llvm-profgen/PerfReader.h:550
+    SmallVector<StringRef, 32> Records;
+    Line.split(Records, " ", -1, false);
+    if (Records.size() < 2)
----------------
wlei wrote:
> wenlei wrote:
> > wlei wrote:
> > > hoy wrote:
> > > > Nit: splitting once is enough, instead of unlimited times (-1)?
> > >  -1(maximum number of times split) is the default value, here is for the 4th parameter which means to not keep the empty string, otherwise there are many empty record, it's not direct to get the first leading pointer. Seems C++ can't skip the middle default value.
> > `Line.split(Records, " ", 1, false);` instead?
> Sorry, I misunderstood. I tired  `Line.split(Records, " ", 1, false);` but it showed it doesn't work, there are several leading empty space, splitting once will only separate one empty space and the remaining still start with empty space.
not a big deal, but this could be faster as it avoids scanning and splitting the entire string:

```
Line = Line.trim(" ");
Line.split(Records, " ", 1, false);
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108153



More information about the llvm-commits mailing list