[PATCH] D9009: Value profiling compiler-rt changes

[Xinliang David Li via llvm-commits]

On Thu, Nov 12, 2015 at 3:56 PM, Betul Buyukkurt <betulb at codeaurora.org>
wrote:

> >
> >
> > On Thu, Nov 12, 2015 at 10:12 AM, Betul Buyukkurt <betulb at codeaurora.org
> > <mailto:betulb at codeaurora.org> > wrote:
> >
> >
> >> davidxl added inline comments.
> >>
> >> ================
> >> Comment at: lib/profile/InstrProfiling.c:190
> >> @@ +189,3 @@
> >> +    }
> >> +    I->ValueCounters = (void *)PerSiteCountsHead;
> >> +    PerSiteCountsHead = (uint8_t *)VDataPtr;
> >> ----------------
> >> Release memory of ValueCounters ?
> >
> > This is a function that's running only atexit, I thought of adding it in
> > but then thought of it as unnecessary too due to this being one of the
> > very last actions of the run time during the lifetime of the program.
> >
> >
> >
> >
> > Ok to leave it as it is, but the assumption may not be true in the
> future.
> >
> > Consider the case when profile dump is not done at exit (e.g. for a
> server
> > program). APIs are provided for the user to periodically dump data for
> > selected program phases they care about. One low overhead way of doing
> > this
> > is let the program continuously do the profiling without the need to
> check
> > the state. APIs are provided so that user can do counter clearing and
> > dumping at the start and end of the phase ...
> >
> > Another thing we need to solve in the future is support raw profile data
> > on-line merging -- otherwise the raw profile data can get really big with
> > the current appending model. Without this, doing clang bootstrap with PGO
> > (4
> > stage with clang/llvm sources as training data) will be impossible.
> >
> > VP data certainly makes it trickier.
>
> The reason we're updating the ValueCounters pointer in the
> __llvm_profile_data is solely for code uniformity in accessing the members
> of the __llvm_profile_data. Yet it's not as conforming to the way the
> NamePtr or the CounterPtr members of the struct are processed. To support
> the cases you mention above, one needs to introduce also a lock to be able
> to write out the data and prevent the value profiling from proceeding
> during the data write out. I think all of this can be avoided by not
> overwriting the ValueCounters pointer.
>

The tricky part is not the ValueCounters pointer update, but the acutal VP
data merging -- because each site may have different number of targets and
order.   If in-memory data has the same size of in-disk file, the online
merge becomes trivial.   It is easily doable though requires more memory.

Your compiler-runtime change looks good to me.  Thanks for your great
patience! Once the base version is in, the community can also start
contribute to it ..

thanks,

David



> -Betul
>
> > David
> >
> >
> >
> >
> >>
> >> http://reviews.llvm.org/D9009
> >>
> >>
> >>
> >>
> >
> >
> >
> >
> >
> >
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151112/9abd17cc/attachment.html>