[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:1010
+  ContextTracker.createContextLessProfileMap(ContextLessProfiles);
+  ProfileGeneratorBase::computeSummaryAndThreshold(ContextLessProfiles, false);
+}
----------------
hoy wrote:
> wlei wrote:
> > 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?
> I guess my questions should be why this the extra parameter needed. Is the original implementation that checks `FunctionSamples::ProfileIsCS` inside `SampleProfileSummaryBuilder::computeSummaryForProfiles` not working?
OK, I see your point, to save the unnecessary merge. I'd say if this is not at a visible cost, let's avoid changing the interface. Otherwise maybe we can reset `UseContextLessSummary` here? A comment would be helpful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127026