[PATCH] D99787: [CSSPGO] Fix incorrect probe distribution factor computation in top-down inliner

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:875
       // be prorated so that the it will reflect the real callsite counts.
-      setProbeDistributionFactor(CI, Candidate.CallsiteDistribution * Sum /
-                                         SumOrigin);
+      setProbeDistributionFactor(CI, 1.0 * Sum / SumOrigin);
       Candidate.CallInstr = DI;
----------------
Thanks for the fix. This seems losing the original distribution factor pre


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:1327
       auto CalleeSamples = findIndirectCallFunctionSamples(*I, Sum);
       uint64_t SumOrigin = Sum;
       Sum *= Candidate.CallsiteDistribution;
----------------
Thanks for the fix! I'm wondering if the fix should be made here by scaling down `SumOrigin` as well. `SumOrigin` is used to guide ICP and we are checking the scaled count against the original count below (line 1342). This could cause a promotable indirect callsite not promoted after the callsite is duplicated. E.g, an indirect callsite with only one target is duplicated in prelink with even distribution. And in postlink, each of the callsite has a target that counts 50% of the original counts. If the original callsite is supposed to be promoted, once duplicated, it may not get promoted in postlink because of the lower weight (50% vs original 100%) in either place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99787