[PATCH] D118515: [llvm-profgen] On-demand track optimized-away inlinees for preinliner.

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/tools/llvm-profgen/ProfiledBinary.cpp:351
+  if (UsePseudoProbes && TrackFuncContextSize) {
+    for (const auto &Child : ProbeDecoder.getDummyInlineRoot().getChildren()) {
+      auto *Frame = Child.second.get();
----------------
wenlei wrote:
> hoy wrote:
> > wenlei wrote:
> > > Actually ProbeDecoder.getDummyInlineRoot().getChildren() is already a map, wondering can we make it possible to look up by function name without building an extra map? 
> > > 
> > > I can see that the key is Guid, ProbeId pair - we can get Guid from names, but ProbeId under dummy root is index. What was the reason for top level nodes to have different probe Id instead of a dummy probe Id? For top level, we don't expect same name to appear more than once. 
> > Top level nodes should have a dummy probe id, i.e, 0. The key in ProbeDecoder.getDummyInlineRoot().getChildren()  is the guid. and we need a guid to look up a node in the children map. Unfortunately, there isn't a func name to guid map available currently. What we have is the `GUID2FuncDescMap`.  So I'm building a name to top level node map. Alternatively we can build a name to guid map here but that'll incur two hash lookups to get the node. Since either map is only used by llvm-profgen, I'm not placing the map to `MCPseudoProbe` which is also shared by the Bolt probe decorder.
> > Unfortunately, there isn't a func name to guid map available currently
> 
> I thought function guid can be computed from names? 
> 
> > Top level nodes should have a dummy probe id, i.e, 0 
> 
> if i read the code correctly in `MCPseudoProbeDecoder::buildAddress2ProbeMap`, that's not the case:
> 
> ```
> while (Data < End) {
>     if (Root == Cur) {
>       // Use a sequential id for top level inliner.
>       Index = Root->getChildren().size();
>     }
> ```
> 
> 
Ah, you are right on both. Now I completely remember why using guid based lookup didn't work. The key of the children map is <caller's guid, callsite probe id> and I was using callee's guid to lookup.

```
// A DFS-based decoding
  while (Data < End) {
    if (Root == Cur) {
      // Use a sequential id for top level inliner.
      Index = Root->getChildren().size();
    } else {
      ...
    }
    // Switch/add to a new tree node(inlinee)
    Cur = Cur->getOrAddNode(std::make_tuple(Cur->Guid, Index));
```

We are now building callee name to callee node map here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118515