[PATCH] D96811: [CSSPGO][llvm-profgen] Change sample count of dangling probe in llvm-profgen

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:501
+    if (Probe->isDangling()) {
+      FunctionProfile.addBodySamples(Probe->Index, 0, UINT64_MAX);
+      continue;
----------------
wlei wrote:
> hoy wrote:
> > hoy wrote:
> > > Actually I `UINT64_MAX` may cause overflow to total samples. Even if it doesn't, profile merge may overflow too. That's one of the reasons we were using 0 as a special count. @wmi What do you think about keeping using 0?
> > I talked to other folks and they like using `UINT64_MAX` instead of 0 to be less confusing. @wlei we may need to fix the places that accumulate total samples and set entry count to not using `UINT64_MAX` as the sample count.
> Thanks for the sharing. So for profgen side, we can keep this patch, right? see line 501, it only adds the body sample and doesn't accumulate the total sample.
> 
> For compiler side, we need to avoid the addition when meeting the `UINT64_MAX`
May need to check dangling in `CSProfileGenerator::populateFunctionBoundarySamples` when updating the callsite target samples?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96811