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

[Lei Wang via Phabricator via llvm-commits]

wlei added inline comments.


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:435
-  } else {
-    // This is for the case the input is a llvm sample profile.
-    for (const auto &FS : ProfileMap) {
----------------
hoy wrote:
> why delete this?
It's not completely deleted, it's moved to the ctor(see:  ProfileGenerator.h: 180), this is because we don't ever save the llvm-sample-profile into ProfileMap, I made the llvm-sample-profile  a temporary variable, it's converted to trie right in the ctor and dropped. So I also moved this part of code there.


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:1010
+  ContextTracker.createContextLessProfileMap(ContextLessProfiles);
+  ProfileGeneratorBase::computeSummaryAndThreshold(ContextLessProfiles, false);
+}
----------------
hoy wrote:
> Is `false` needed here? We are passing in contextless profiles so merging them again inside `computeSummaryAndThreshold` should be fine?
Yeah, we can merge the contextless profiles twice, but is there any concern to save one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127026