[PATCH] D98921: [CSSPGO][llvm-profgen] Use profile summary based threshold for context trimming and merging

Mon Mar 22 08:42:25 PDT 2021

wenlei added inline comments.

================
Comment at: llvm/test/tools/llvm-profgen/merge-cold-profile.test:54
+; CHECK-UNMERGED-NEXT: !CFGChecksum: 72617220756
+

----------------
wlei wrote:
> How about we add:
> ```
> ; CHECK-UNMERGED-NOT: [fa]
> ; CHECK-UNMERGED-NOT: [fb]
> ```
> to make sure the merging not happen.
Sounds good, added.

================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:239
+  // context profile merging/trimming.
+  computeSummaryAndThreshold();
 }
----------------
wlei wrote:
> There is two places to call this function: for probe and non-probe.
> How about to move this to `CSProfileGenerator::write(...)` so that we only keep one call for this?
I think it doesn't belong to writing, though moving there may save one call from the code. The thresholds will be needed for estimating inlining, which I will send up next, so I'd like to keep it this way for now if it's ok, and try to refactor in the upcoming change. 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98921