[PATCH] D112489: [CSSPGO] Trim cold base profiles for the CS preinliner.

Wed Oct 27 19:08:28 PDT 2021

wlei accepted this revision.
wlei added a comment.

LGTM.

================
Comment at: llvm/lib/ProfileData/SampleProf.cpp:357
   for (const auto &I : ColdProfiles) {
     if (MergeColdContext) {
       auto MergedContext = I.second->getContext().getContextFrames();
----------------
hoy wrote:
> wlei wrote:
> > wlei wrote:
> > > `if(MergeColdContext && !TrimBaseProfileOnly)`
> > > 
> > > So that `MergedProfileMap` will be empty for `TrimBaseProfileOnly`
> > How about this comments? is `MergeColdContext ` always false when `TrimBaseProfileOnly` is true?
> Sorry for overlooking this comment. `MergeColdContext` and `TrimBaseProfileOnly` can be both true when user specifies the switches for both merging and trimming. We were talking about whether to turn off merging for preinliner completely and we agree to let user decide. In that case, I think we should ignore `TrimBaseProfileOnly` since merging is meant to not honor the preinliner decision.
> 
> It is a good catch, thanks!
Thanks for the clarification!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112489