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

Wed Oct 27 18:57:00 PDT 2021

hoy added inline comments.

================
Comment at: llvm/lib/ProfileData/SampleProf.cpp:357
   for (const auto &I : ColdProfiles) {
     if (MergeColdContext) {
       auto MergedContext = I.second->getContext().getContextFrames();
----------------
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!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112489