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

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/tools/llvm-profgen/CSPreInliner.cpp:275-283
+  if (!CSProfMergeColdContext.getNumOccurrences())
+    CSProfMergeColdContext = false;
+
+  // Trim or merge cold profiles from ProfileMap.
+  if (CSProfTrimColdContext || CSProfMergeColdContext) {
+    Trimmer.trimAndMergeColdContextProfiles(
+        HotCountThreshold, CSProfTrimColdContext, CSProfMergeColdContext,
----------------
hoy wrote:
> wenlei wrote:
> > Sorry if I wasn't clear.
> > 
> > > 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.
> > 
> > What I meant by the above to do the trimming outside of preinliner. So preinliner does strictly preinlining, and trimming is done at the original place, with TrimBaseProfileOnly set to EnableCSPreInliner. This way we don't have trimming functionality scattered.
> > 
> > The setting of CSProfMergeColdContext also better be done outside of preinliner. 
> > 
> I found that separating the logic for the preinlining and non-preinlining path is easier to read and less error prone. Don't have a strong preference. Can do what you suggested.
Actually the logic looks better after moving this out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112489