[PATCH] D82123: [PGO] Add a functionality to always instrument the func entry BB

[Rong Xu via llvm-commits]

On Thu, Jun 18, 2020 at 2:15 PM David Li via Phabricator <
reviews at reviews.llvm.org> wrote:

> davidxl added inline comments.
>
>
> ================
> Comment at: llvm/lib/Transforms/Instrumentation/CFGMST.h:110
> +    // If we want to instrument the entry count, lower the weight to 0.
> +    if (PGOInstrumentEntry)
> +      EntryWeight = 0;
> ----------------
> Does this guarantee entry be instrumented?
>
>
> ================
> Comment at: llvm/lib/Transforms/Instrumentation/CFGMST.h:147
>              Weight = BPI->getEdgeProbability(&*BB,
> TargetBB).scale(scaleFactor);
> +          if (Weight == 0)
> +            Weight++;
> ----------------
> Is this the way to make sure entry is always instrumented?
>

This together with the line above, will guarantee that fake edge the entry
be the minimum. So it will not be the MST.
Yes. This is gonna guarantee that the entry will be instrumented.

>
>
> ================
> Comment at: llvm/lib/Transforms/Instrumentation/CFGMST.h:292
>      computeMinimumSpanningTree();
> +    if (PGOInstrumentEntry && (AllEdges.size() > 1))
> +      std::iter_swap(std::move(AllEdges.begin()),
> ----------------
> is this needed? Can somthing be done in CompputeMST without relying on
> weight trick?
>
> This is to make sure the index 0 be the entry BB. If we don't do this,
that will be the last index.

I think this is a pretty clean way to implement -- if one knows how
instrumentation is implemented.

>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D82123/new/
>
> https://reviews.llvm.org/D82123
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200618/bc70c889/attachment.html>