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

[Wenlei He via Phabricator via llvm-commits]

wenlei added inline comments.


================
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:
> > > 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);
> > ```
> > 
> Sounds good, I actually found there are two space strings between each LBR entry, so I need to change to split 2 times :)
Or `Line.split(Records, "  ", 1, false);`? two space instead of one as the separator. Works either way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108153