[llvm] [llvm-profgen] Improve sample profile density (PR #92144)

Lei Wang via llvm-commits llvm-commits at lists.llvm.org
Mon May 20 17:54:13 PDT 2024


================
@@ -768,9 +745,79 @@ void ProfileGenerator::populateBoundarySamplesForAllFunctions(
   }
 }
 
+void ProfileGeneratorBase::calculateDensity(
+    const FunctionSamples &FSamples,
+    std::vector<std::pair<double, uint64_t>> &DensityList,
+    uint64_t &TotalProfileSamples) {
+  uint64_t TotalBodySamples = 0;
+  uint64_t FuncBodySize = 0;
+  for (const auto &I : FSamples.getBodySamples()) {
+    TotalBodySamples += I.second.getSamples();
+    FuncBodySize++;
+  }
+
+  // The whole function could be inlined and optimized out, use the callsite
+  // head samples instead to estimate the body count.
+  if (FuncBodySize == 0) {
+    for (const auto &CallsiteSamples : FSamples.getCallsiteSamples()) {
+      FuncBodySize++;
+      for (const auto &Callee : CallsiteSamples.second) {
+        calculateDensity(Callee.second, DensityList, TotalProfileSamples);
----------------
wlei-llvm wrote:

Try to clarify this first, will address other comments later.

I don't have a particular reason to exclude the inlinees. The story is before we computed density on a full CS profile(or a merged context-less profile), so there is actually no inlinees in it.(see the original code: https://github.com/llvm/llvm-project/blob/main/llvm/tools/llvm-profgen/ProfileGenerator.cpp#L1060-L1064. this is before the nested profile conversion), so I just thought we need to base on an un-merged profile, didn't consider the density scope. 

>I guess the question is why do we want to exclude inlinees? we can count total samples including inlinees, and also size including inliness. Otherwise we don't cover such inlinees in density metrics -- what if inlinee has a low density problem?

another thing to clarify, this version is not to drop the inlinees profile, but compute as an independent function instead of part of the caller function. It should be able to catch the low inlinee profile .e.g.
```
main:101:100
  1:100 
  2:foo:1
   1:1
foo:100:1
 1:100   
```
In this version, it will compute three function density `<main:100>` `<foo:1>``<foo:100>`, we still have the low density inlinee `<foo:1>`
IIUC, your suggestion is to compute like a `<main: 50 (101/2)>`  ` <foo: 100>`
 
Yeah, if that makes sense, so my current data is based on current version, which seems work ok. 

I think I can implement a binary-level density and compare the two, we can take a look together and discuss on it. 


 

https://github.com/llvm/llvm-project/pull/92144


More information about the llvm-commits mailing list