[PATCH] D127026: [CSSPGO][llvm-profgen] Reimplement computeSummaryAndThreshold using context trie

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:442
 
+void ProfileGenerator::collectPFForLLVMSampleProfile(
+    std::unordered_set<const BinaryFunction *> &ProfiledFunctions) {
----------------
wlei wrote:
> hoy wrote:
> > Sorry for not making it clear. Can we do something like below? Basically keep the original `collectProfiledFunctions` as is and add a new override for CSProfileGenerator.
> > 
> > 
> > ```
> > void ProfileGeneratorBase::collectProfiledFunctions() {
> >   std::unordered_set<const BinaryFunction *> ProfiledFunctions;
> >   if (SampleCounters) {
> >      ....
> >   } else if (!ProfileMap.empty())
> >      ....
> >   }
> >   Binary->setProfiledFunctions(ProfiledFunctions);
> > }
> > 
> > void CSProfileGenerator::collectProfiledFunctions() {
> >   deal with ContextTrieNode;
> >   Binary->setProfiledFunctions(ProfiledFunctions);
> > }
> > ```
> > 
> > 
> ```
>   if (SampleCounters) {
>      ....
>   }
> ```
> This part of code is also needed for `CSProfileGenerator` because `collectProfiledFunctions ` is called before the `generateProbeBasedProfile`, at that time none of `ContextTrieNode` is created.
I see, would this work?

```

void CSProfileGenerator::collectProfiledFunctions() {
  if (getRootContext) {
    ... 
    Binary->setProfiledFunctions(ProfiledFunctions);
   }
   else {
     ProfileGeneratorBase::collectProfiledFunctions();
   }
}
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127026