[PATCH] D111750: [llvm-profgen] Allow unsymbolized profile as perf input

[Wenlei He via Phabricator via llvm-commits]

wenlei accepted this revision.
wenlei added a comment.
This revision is now accepted and ready to land.

lgtm, thanks.



================
Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:797
+    }
+    Key->genHashCode();
+    auto Ret =
----------------
wlei wrote:
> wenlei wrote:
> > wlei wrote:
> > > wenlei wrote:
> > > > Would it better if hash code is lazily generated instead of requiring an explicit call? 
> > > > 
> > > > ```
> > > > getHashCode() {
> > > >   if (HashCode == 0) 
> > > >      genHashCode()
> > > >   ...
> > > > }
> > > > ```
> > > Here to explicitly call `genHashCode` is intentional, the reason is we want to avoid making `genHashCode` a virtual function, i, e, avoid call `genHashCode `after casting to base class. so we separate like:
> > > ```
> > > derived class:  HashCode = genHashCode();
> > > 
> > > base class :  getHashCode{return HashCode;}
> > > ```
> > > 
> > > 
> > Not sure if I understand. Why do we want to avoid call genHashCode after casting to base class?
> Currently we have two type of key, `StrKey` and `ProbeKey` and they derived from the base  `ContextKey`.
> 
> We have a hash map to store them, like unordered_map<ContextKey, ...>, the logic of insert is like
> ```
> StrKey* key = ..
> 
> hashmap[key] = ...;   
> ```
> In the hashmap,  `StrKey*/ProbeKey*` implicitly be cast to a ContextKey* class then call `ContextKey->getHashCode` , so `getHashCode` should be a virtual function which has overhead because  `StrKey` and `ProbeKey` have different genHashCode. 
> 
> So to avoid this, we can explicitly call StrKey->genHashCode() before being casting to base and store it into a variable `HashCode`
> 
> then base's getHashCode just read the HasCode, no need a virtual function.
> 
> 
> 
> 
> 
> 
> so getHashCode should be a virtual function which has overhead because StrKey and ProbeKey have different genHashCode.

> So to avoid this

Why do we want to avoid this? performance reason? It looks to me that having getHashCode as virtual function is natural and clean. 

This pattern perhaps is not related to this patch though. 


================
Comment at: llvm/tools/llvm-profgen/PerfReader.h:654
+      src_n->dst_n:count_n
+    [context stack2]
+      ......
----------------
nit: this does not align with `[context stack1]`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111750