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

[Wenlei He via Phabricator via llvm-commits]

wenlei added inline comments.


================
Comment at: llvm/tools/llvm-profgen/CSPreInliner.cpp:273
+  // Remove cold profiles from ProfileMap.
+  if (CSProfTrimColdContext) {
+    LLVM_DEBUG(dbgs() << "Trimming cold base profiles with HotCountThreshold="
----------------
I think it's cleaner if we defer all trimming to SampleContextTrimmer. We can add TrimBaseProfileOnly flag to trimAndMergeColdContextProfiles interface, or have a separate interface (will still need to make sure we reuse code). We create SampleContextTrimmer right above anyways. 

With this change, CSProfMergeColdContext will always be ignored when preinliner is on, but that's not the case before. So alternatively, if TrimBaseProfileOnly is added to the interface, we can call the trimmer outside of preinliner like before, but set TrimBaseProfileOnly to true when preinliner is on.


================
Comment at: llvm/tools/llvm-profgen/CSPreInliner.cpp:274
+  if (CSProfTrimColdContext) {
+    LLVM_DEBUG(dbgs() << "Trimming cold base profiles with HotCountThreshold="
+                      << HotCountThreshold << "\n");
----------------
This logging is a bit inconsistent - we don't have logging for this level of activity from SampleContextTrimmer. However if we unify as suggested above, we won't have such inconsistency. I'm not sure if we need such logging though. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112489