[PATCH] D28965: [PGO] Value profile for size of memory intrinsic calls

Rong Xu via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 8 12:39:22 PST 2017


On Wed, Mar 8, 2017 at 11:28 AM, David Li via Phabricator <
reviews at reviews.llvm.org> wrote:

> davidxl added inline comments.
>
>
> ================
> Comment at: lib/Transforms/Instrumentation/InstrProfiling.cpp:546
> +      if (Kind == IPVK_MemOPSize)
> +        TotalNS += I * (MemOPSizeRangeLast + 1);
> +    }
> ----------------
> xur wrote:
> > davidxl wrote:
> > > Larger range does not mean on average the number of values tracked
> will be proportionally larger. Perhaps using a different heuristic for
> allocation size? Besides, TotalNS tracks the number of value sites,
> overloading it for number of values does not make sense
> > I removed this change. Now MemOPSize profile uses the same heuristic as
> indirect-call profiling.
> With this default heuristics, do you see any dropped value profiling at
> runtime?
>
>
I need to rerun the tests. Will report back.


>
> ================
> Comment at: tools/llvm-profdata/llvm-profdata.cpp:449
>
> +typedef struct ValueSitesStats {
> +  ValueSitesStats()
> ----------------
> Can you split out this part as a refactoring patch?
>
>
Split patch submitted here:
https://reviews.llvm.org/D30752


>
> https://reviews.llvm.org/D28965
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170308/4a4f99a2/attachment.html>


More information about the llvm-commits mailing list