[PATCH] D125023: [CSSPGO][Preinliner] Use linear threshold to drive inline decision.

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/tools/llvm-profgen/CSPreInliner.cpp:164
+    // 10% cutff to cap the threshold.
+    uint64_t CapCount = ProfileSummaryBuilder::getEntryForPercentile(
+                            Summary->getDetailedSummary(), 100000 /* 10% */)
----------------
wenlei wrote:
> > Use the count at 10% cutff to cap the threshold. 
> 
> nit: people should be able to figure out what this is all about, but this comment can be confusing. maybe let's just make it more clear that "we normalize hotness to be [0,1], then linearly adjust threshold based on normalized hotness. "
> 
> That comment couples with more explicit code for readability.
> 
> ```
> NormalizationUpperBound = ProfileSummaryBuilder::getEntryForPercentile(...)
> NormalizationLowerBound = ColdCountThreshold (or ProfileSummaryBuilder::getEntryForPercentile(...) )
> 
> NormalizedHotness = ...
> SampleThreshold = ...
> ```
> 
Sounds good. Also added comment for the 10% cutoff.


================
Comment at: llvm/tools/llvm-profgen/CSPreInliner.cpp:172
+    SampleThreshold = SampleHotCallSiteThreshold * NormalizedHotness * 100 +
+                      SampleColdCallSiteThreshold + 1;
+  }
----------------
wenlei wrote:
> Perhaps worth a comment for this `+1` (so later on others don't think it's non-critical and attempt to clean it up) 
Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125023