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

Hongtao Yu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 5 22:58:47 PDT 2022


hoy added inline comments.


================
Comment at: llvm/tools/llvm-profgen/CSPreInliner.cpp:173
+    // Linearly adjust threshold based on call site hotness
+    auto Range = Summary->getMaxCount() - ColdCountThreshold;
+    double Position =
----------------
wenlei wrote:
> One concern about using `getMaxCount` -- that will have more instability and variation run to run. How about using HotCountThreshold (or N * HotCountThresold, or a set percentile) to stabilize?
> 
> The `NormalizedHotness` doesn't have to be between 0 and 1. Or if we want it to be between 0 and 1, we can also do `min(NormalizedHotness, 1)`.
> (or N * HotCountThresold, or a set percentile) to stabilize?

You mean something like Position = N* HotCountThresold ? BTW, what is N? 

Using a percentile should have a better stability. Position is similar to (1-percentile).  




================
Comment at: llvm/tools/llvm-profgen/CSPreInliner.cpp:174-175
+    auto Range = Summary->getMaxCount() - ColdCountThreshold;
+    double Position =
+        (Candidate.CallsiteCount - ColdCountThreshold) / (double)Range;
+    double Multiplier = PreInlineHotCallsiteThresholdMultiplier * Position;
----------------
wenlei wrote:
> Perhaps rename `Position` as `NormalizedHotness`? 
Sounds good.


================
Comment at: llvm/tools/llvm-profgen/CSPreInliner.cpp:176-179
+    double Multiplier = PreInlineHotCallsiteThresholdMultiplier * Position;
+    SampleThreshold = SampleHotCallSiteThreshold * Multiplier +
+                      SampleColdCallSiteThreshold +
+                      PreInlineHotCallsiteThresholdConstant;
----------------
wenlei wrote:
> How about we simplify the knobs, i.e. remove flag `preinline-hot-callsite-threshold-multiplier` and `preinline-hot-callsite-threshold-constant`. 
> 
> Make `Multiplier` a constant, and we can still effectively tune the multiplier via setting `sample-profile-hot-inline-threshold`.
> Make `PreInlineHotCallsiteThresholdConstant` a constant and we can still effectively tune it via `sample-profile-cold-inline-threshold`. We could also consider setting `SampleColdCallSiteThreshold = 1` in PreInliner (current we set it to 0).
> 
> With the above, I think if we can simply switches (avoid adding new ones) without scarifying flexibility for tuning. 
> Make Multiplier a constant, and we can still effectively tune the multiplier via setting sample-profile-hot-inline-threshold.

This should work since SampleHotCallSiteThreshold is only used here. One thing is the hot threshold may look very different from what the compiler uses, because of the multiplier. We could use the same heuristics on the compiler as well.

> Make PreInlineHotCallsiteThresholdConstant a constant and we can still effectively tune it via sample-profile-cold-inline-threshold

This may also affect cold contexts. Enlarging `sample-profile-cold-inline-threshold` , say from 0 to 5, may result in a lot more cold contexts in the profile.  This is because we currently treat cold functions as having the same hotness, and only use the cold threshold for them instead of computing a linear threshold. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125023



More information about the llvm-commits mailing list