[PATCH] D81981: [PGO] Supplement PGO profile with Sample profile

Wed Jun 17 17:17:34 PDT 2020

hoyFB added a comment.

It's an interesting idea to improve the PGO profile quality with sample profiles. Thanks for working on this!

In D81981#2099580 <https://reviews.llvm.org/D81981#2099580>, @wmi wrote:

> In D81981#2099452 <https://reviews.llvm.org/D81981#2099452>, @davidxl wrote:
>
> > Why is the profile size increase? I expect the number of instrumented blocks remain mostly unchanged.
> >
> > The reason for the question is that if the overhead is low, I think we should make the default to be true.
>
>
> For function entry bb which have multiple successors, the existing algorithm in FuncPGOInstrumentation<Edge, BBInfo>::getInstrBB will insert the counter in all its successors. My current implementation simply adds a counter in entry block so in that case, it introduces redundent counter.
>
> I can improve it by selecting a successor to not insert counter for it since it can be inferred from the counters surrounding it. With that implemented, I expect the profile size will be unchanged.

The current PGO instrumentation is based on MST. Changing the instrumentation may require changes in how the counts of non-MST-edges are calculated (in `PGOUseFunc::setInstrumentedCounts`). So maybe adjust the MST to remove the sibling edge ?

================
Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:785

   auto canInstrument = [](BasicBlock *BB) -> BasicBlock * {
     // There are basic blocks (such as catchswitch) cannot be instrumented.
----------------
Remove the lambda and use the newly added static function instead?

Repository:
  rL LLVM

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

https://reviews.llvm.org/D81981