[PATCH] D107529: [llvm-profgen] Fix bug of loop scope mismatch

[Lei Wang via Phabricator via llvm-commits]

wlei added inline comments.


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:523
   extractProbesFromRange(RangeCounter, ProbeCounter, Binary);
   std::unordered_map<MCDecodedPseudoProbeInlineTree *, FunctionSamples *>
       FrameSamples;
----------------
wlei wrote:
> wlei wrote:
> > hoy wrote:
> > > wlei wrote:
> > > > I'm wondering here if `FunctionSamples *` should be `Vector<FunctionSamples *>` as same frame can point to different  FunctionSamples?
> > > An inline context under a given calling context should uniquely identify an inline frame, therefore can only point to one leaf profile. Am I missing anything?
> > Sorry I meant `std::set<FunctionSamples *>`.
> > 
> > I'm thinking the multiple probes cases from different call stack and they have the same leaf probe. like
> > [Probe1, Probe3] and [Probe1, Probe2]
> > 
> > Here Probe1 will have two `FunctionSamples`.
> > 
> > 
> Oh. Here in `populateBodySamplesWithProbes` has already guaranteed to have unique call stack, so no multiple probe cases. Sorry for the confusing.
@hoy I found that the `probe->getInlineTreeNode()` is not the leaf frame so here it's not unique, the unique one should be like "probe + leaf function" and the leaf function is given from its FuncDesc. It happened that there are different the leaf functions.

See the reference below, "Note that the context from probe doesn't include leaf frame, hence we need to retrieve and prepend leaf if requested.", when we extract the inline context, we still need to append the leaf frame.
```
void MCPseudoProbeDecoder::getInlineContextForProbe(
    const MCDecodedPseudoProbe *Probe,
    SmallVectorImpl<std::string> &InlineContextStack, bool IncludeLeaf) const {
  Probe->getInlineContext(InlineContextStack, GUID2FuncDescMap, true);
  if (!IncludeLeaf)
    return;
  // Note that the context from probe doesn't include leaf frame,
  // hence we need to retrieve and prepend leaf if requested.
  const auto *FuncDesc = getFuncDescForGUID(Probe->getGuid());
  InlineContextStack.emplace_back(FuncDesc->FuncName + ":" +
                                  Twine(Probe->getIndex()).str());
}
``` 

So I changed to use `unordered_set` for multiple leaf frame FunctionSample. See if this looks good to you?










Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107529