[PATCH] Indirect call target profiling related profile reader/writer changes

[Xinliang David Li]

I have not looked at the patch yet, but just a related note: we are working
on adding  *general* value profiling support for LLVM. The profiler needs
to be really lightweight (for instrumentation runtime).  Indirect target
profiling is just one case that can be handled by the general framework.

I plan to send out the proposal soon.

thanks,

David




On Sun, Apr 12, 2015 at 9:51 PM, Bob Wilson <bob.wilson at apple.com> wrote:

>
> On Apr 10, 2015, at 9:25 AM, betulb at codeaurora.org wrote:
>
>
> On 04/09/2015 11:06 AM, Betul Buyukkurt wrote:
>
>
> In http://reviews.llvm.org/D8908#153838, @reames wrote:
>
> Have the IR level construct patches made it up for review?  If so, can
>
>
> So far I've posted two patches. These two patches should apply cleanly
> to the tip, working with the present profile infrastructure. The next
> set of patches will be the enabler ones: i.e. three more patches one for
> each of clang, llvm and compiler-rt. Clang patch will be up for review
> later today.
>
> you send me a link?  I managed to miss them.
>
>
> So far there is this patch and the instrinsic instruction definitions:
> http://reviews.llvm.org/D8877. All patches are necessary for getting the
> IC targets and having them displayed by the llvm-profdata.
>
> Ok, I'm really not convinced that the instrumentation code needs to be
> or should be an intrinsic.  This seems like something which should be
> emitted by the frontend and optimized like any other code.  To say this
> a different way, my instrumentation is going to be entirely different
> than your instrumentation.
>
> Having said that, I really don't care about this part of the proposed
> changes since they aren't going to impact me at all.  I'm am
> specifically not objecting to the changes, just commenting.  :)
>
>
> I'm assuming this will be some type of per call site metadata?
>
>
> We do assign metadata at the indirect call sites. Format looks like as
> follows:
>
> !33 = metadata !{metadata !"indirect_call_targets", i64
> <total_exec_count>, metadata !"target_fn1†, i64 <target_fn1_count>,
> metadata !"target_fn2†, i64 <target_fn2_count>, ….}
>
> Currently, we're recording only the top most called five function names
> at each indirect call site. Following the string literal
> “indirect_call_targets†are the fields  <total_exec_count> i.e. a 64
> bit value for the total number of times the indirect call is executed
> followed by the function names and execution counts of each target.
>
> This was the part I was trying to ask about.  I really want to see where
> you're going with this optimization wise.  My naive guess is that this
> is going to be slightly off for what you actually want.
>
> Assuming you're going for profile guided devirtualization (and thus
> inlining), being able to check the type of the receiver (as opposed to
> the result of the virtual lookup) might be advantageous.  (Or, to say it
> differently, that's what I'm used to seeing.  Your approach might be
> completely reasonable, it's just not what I'm used to seeing.)  Have you
> thought about the tradeoffs here?
>
>
> Not sure if I understood the problem here, however, we're recording both
> the target address and the addresses/names of the instrumented functions
> during the execution of the instrumented binary. During profile reading
> these addresses are used to match the target addresses to corresponding
> functions. During optimization, target function name should be adequate to
> retrieve the function type information which then can be used to compare
> the types of the arguments as well as the return type w/ the call site to
> ensure the match.
>
> To clarify, our interest at this time is to upstream all the
> infrastructure related changes. The optimizations using the profile data
> will not be part of our initial set of patches. On the other hand,
> agreeing on the metadata format at the call sites would be advantageous in
> terms of supporting multiple profile methods.
>
> Thanks,
> -Betul
>
>
> I have the same questions and concerns as Philip. If the goal of this is
> to support devirtualization, this is not how I would expect it to be done.
> If you really want something more general, can you provide data to show
> that it is worth the cost? Recording all that information when profiling
> will not be cheap, and it adds significant complexity as well.
>
> I also don’t think we should consider taking all of your infrastructure
> changes without first seeing at least preliminary versions of the
> optimization patches that demonstrate the benefits.
>
>
>
>
> Thanks.
> -Betul
>
>
> http://reviews.llvm.org/D8908
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150413/52853fc5/attachment.html>