<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Nov 19, 2020 at 4:24 PM David Li via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">davidxl added inline comments.<br>
<br>
<br>
================<br>
Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:1657<br>
+  uint64_t FuncEntryCount = Func.getBBInfo(&*F.begin()).CountValue;<br>
+  uint64_t NewEntryCount = 0.5 + FuncEntryCount * Scale;<br>
+  if (NewEntryCount == 0)<br>
----------------<br>
xur wrote:<br>
> davidxl wrote:<br>
> > Perhaps take the max of the original func entry count and the new entry count?<br>
> Not sure if want a max here. Here a value of 1 makes the sum of raw counters and sum of BFI counters close to each other, better than original func entry count.<br>
> <br>
> Using max will never reduce the entry count -- I don't think that is what we want.<br>
I thought the intention of the patch is to correct the 'guessed' entry count which is usually '1'. If the original entry count is not 1, it is usually a value we can trust. Sometimes BFI can create insanely large counts which makes the scale really small. It ends up leading to new entry entry count to become 1 (from non-1 value). Is that the intended behavior?<br>
<br></blockquote><div>No. that was not the intention. I had this patch for a long time. There are other factors that make BFI counters deviate from raw profile values. Those facts include bugs in BFI, round-off error, inconsistent profile. I have seen all of them.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
<br>
CHANGES SINCE LAST ACTION<br>
  <a href="https://reviews.llvm.org/D61540/new/" rel="noreferrer" target="_blank">https://reviews.llvm.org/D61540/new/</a><br>
<br>
<a href="https://reviews.llvm.org/D61540" rel="noreferrer" target="_blank">https://reviews.llvm.org/D61540</a><br>
<br>
</blockquote></div></div>