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

[Wenlei He via Phabricator via llvm-commits]

wenlei added inline comments.


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:47
+
+static cl::opt<bool> CSProfTrimColdContext(
+    "csprof-trim-cold-context", cl::init(true), cl::ZeroOrMore,
----------------
hoy wrote:
> I'm wondering if the original switches can be moved into a common place, such as ProfileSummaryBuilder.cpp, so that they can be reused here. Some code in CSProfileGenerator::computeSummaryAndThreshold may also be sharable.
That would be nice, however for normal path of ProfileSummaryInfo, the actual summary is retrieved from module metadata which is read from binary profile directly instead of being built from ProfileSummaryBuilder on the spot. 

I think it probably makes sense to just use ProfileSummaryInfo here, as Wei suggested. However, looks like the switches from ProfileSummaryInfo.cpp doesn't get exposed that way.. I guess that's because ProfileData is linked into llvm-profogen, instead of being part of the tool? 


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:422-431
+  auto &HotEntry = ProfileSummaryBuilder::getEntryForPercentile(
+      DetailedSummary, ProfileSummaryCutoffHot);
+  HotCountThreshold = HotEntry.MinCount;
+  if (ProfileSummaryHotCount.getNumOccurrences() > 0)
+    HotCountThreshold = ProfileSummaryHotCount;
+  auto &ColdEntry = ProfileSummaryBuilder::getEntryForPercentile(
+      DetailedSummary, ProfileSummaryCutoffCold);
----------------
wmi wrote:
> Similar as Hongtao's question. Can ProfileSummaryInfo::computeThresholds be used to set the hot/cold threshold? 
Sounds good, changed to use PSI directly here, so we don't need keep thresholds separately. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98921