[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.h:179
+      : ProfileGeneratorBase(Binary), ContextTracker(Profiles, nullptr) {
+    // This is for the case the input is a llvm sample profile.
+    std::unordered_set<const BinaryFunction *> ProfiledFunctions;
----------------
wlei wrote:
> hoy wrote:
> > This piece of code is needed because `Profiles` is not saved as a field of `ProfileGeneratorBase` anymore. Can we still do that to avoid duplicate this code here?
> Before it copied to `ProfileMap` and now we need to make `ProfileMap` empty because the `buildProfileMap`  will convert the optimized trie into ProfileMap. Non-empty ProfileMap will mess up the results.
> 
> Maybe we can add a new field to `ProfileGeneratorBase ` like `SampleProfileMap & LLVMSampleProfiles;`?
> 
> now we need to make ProfileMap empty because the buildProfileMap will convert the optimized trie into ProfileMap. Non-empty ProfileMap will mess up the results.

Just clean it inside `buildProfileMap`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127026