[PATCH] D9009: Value profiling compiler-rt changes

[Xinliang David Li via llvm-commits]

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

> > davidxl added inline comments.
> >
> > ================
> > Comment at: lib/profile/InstrProfiling.c:175
> > @@ +174,3 @@
> > +
> > +    for (uint32_t i = 0; i < NumVSites; ++i) {
> > +
> > ----------------
> > Looks like the structure is very close to ValueProfData structure used in
> > indexed format (except that the later is organized per-value kind, while
> > here it is combined).  It will get all the interfaces for reading etc.
>
> So, is this another file format change cycle? We've already changed the
> format three times during the life time of this CL.
>
>
The two formats are basically the same, so there is no much change
essentially. See my comment in another patch, it is more about code sharing
and reuse. Having said that, I am fine with changing this as a follow up.

> ================
> > Comment at: lib/profile/InstrProfilingBuffer.c:45
> > @@ -41,3 +44,3 @@
> >        PROFILE_RANGE_SIZE(Counters) * sizeof(uint64_t) +
> > -      NamesSize + Padding;
> > +      NamesSize + Padding2;
> >  }
> > ----------------
> > Value Profile Data size is not counted here.
>
> We do not make use of these API's, thus we first need input from the
> community (specifically the users of these API's) to change them.
> Currently, VP is just not supported w/ the Buffer API's. In case buffer
> API's are used to retrieve profile data during an instrumented executable
> continues it's execution (jit like scenarios?), then updating the
> ValueCounters member of __llvm_profile_data would possibly break the
> profiler.
>
>
This does not sound right -- buffer API and file API now behaves
differently.  The right way to do is:

1) introduce a lock for dumping. When the dumping lock is help, the
profiler will simply return without updating
2) when dumping is finished, the valueCounters can be cleared to be null
(and edge profile count needs to be cleared too) and the lock can be
released.

David


> -Betul
>
> >
> > ================
> > Comment at: lib/profile/InstrProfilingBuffer.c:110
> > @@ -101,2 +109,3 @@
> > +  UPDATE_memcpy(Zeroes,        Padding2      * sizeof(char));
> >  #undef UPDATE_memcpy
> >
> > ----------------
> > Value profile data is missing here.
> >
> >
> > http://reviews.llvm.org/D9009
> >
> >
> >
> >
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151111/b27150f7/attachment.html>