[PATCH] D9009: Value profiling compiler-rt changes

[Xinliang David Li via llvm-commits]

On Wed, Nov 11, 2015 at 6:04 PM, Betul Buyukkurt <betulb at codeaurora.org>
wrote:

> > davidxl added inline comments.
> >
> > ================
> > Comment at: lib/profile/InstrProfiling.c:81
> > @@ +80,3 @@
> > +void __llvm_profile_instrument_target(uint64_t TargetValue, void *Data_,
> > +  uint32_t CounterIndex) {
> > +
> > ----------------
> > Can we make the CounterIndex relative to the value kind? Why is value
> kind
> > parameter not passed in?
>
> It's not passed in, because it's already relative to the value kind.
> CounterIndex is computed inside the InstrProfiling.cpp of the llvm patch
> during lowering. It's a value that can be computed in compile time, so
> better to do it then.
>

Ok.

David

>
> -Betul
>
> >
> > ================
> > Comment at: lib/profile/InstrProfiling.c:87
> > @@ +86,3 @@
> > +
> > +  if (!Data->ValueCounters) {
> > +    uint64_t NumVSites = 0;
> > ----------------
> > This part of the code is cold -- move it out of line into another
> routine.
> >
> > ================
> > Comment at: lib/profile/InstrProfiling.c:119
> > @@ +118,3 @@
> > +
> > +  if (VDataCount >= UCHAR_MAX)
> > +    return;
> > ----------------
> > This may still be too large. If VDataCount is this large, the runtime
> > overhead will be substantial.
> >
> > In the future, we should enhance it with the following algoithm:
> > 1) reserve the first vdata node to do book keeping -- i.e. tracking
> number
> > of time values get dropped due to too long a list
> > 2) When the number of dropping times reaches a threshold, sort the
> > existing
> > linked list to make it in descending order, and zero out the last X
> > entries.
> >
> > Not needed for the first version.
> >
> > ================
> > Comment at: lib/profile/InstrProfiling.h:33
> > @@ +32,3 @@
> > +  VK_FIRST = VK_IndirectCallTarget,
> > +  VK_LAST = VK_IndirectCallTarget
> > +} __llvm_profile_value_kind;
> > ----------------
> > Make the enumerators name the same as the LLVM counter part -- this
> allows
> > future refactoring.
> >
> >
> > http://reviews.llvm.org/D9009
> >
> >
> >
> >
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151111/588aec1c/attachment.html>