<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Apr 13, 2015 at 9:38 AM, Philip Reames <span dir="ltr"><<a href="mailto:listmail@philipreames.com" target="_blank">listmail@philipreames.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><br>
On 04/10/2015 09:25 AM, <a href="mailto:betulb@codeaurora.org" target="_blank">betulb@codeaurora.org</a> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On 04/09/2015 11:06 AM, Betul Buyukkurt wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
In <a href="http://reviews.llvm.org/D8908#153838" target="_blank">http://reviews.llvm.org/D8908#<u></u>153838</a>, @reames wrote:<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Have the IR level construct patches made it up for review? If so, can<br>
</blockquote>
So far I've posted two patches. These two patches should apply cleanly<br>
to the tip, working with the present profile infrastructure. The next<br>
set of patches will be the enabler ones: i.e. three more patches one for<br>
each of clang, llvm and compiler-rt. Clang patch will be up for review<br>
later today.<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
you send me a link? I managed to miss them.<br>
</blockquote>
So far there is this patch and the instrinsic instruction definitions:<br>
<a href="http://reviews.llvm.org/D8877" target="_blank">http://reviews.llvm.org/D8877</a>. All patches are necessary for getting the<br>
IC targets and having them displayed by the llvm-profdata.<br>
</blockquote>
Ok, I'm really not convinced that the instrumentation code needs to be<br>
or should be an intrinsic. This seems like something which should be<br>
emitted by the frontend and optimized like any other code. To say this<br>
a different way, my instrumentation is going to be entirely different<br>
than your instrumentation.<br>
<br>
Having said that, I really don't care about this part of the proposed<br>
changes since they aren't going to impact me at all. I'm am<br>
specifically not objecting to the changes, just commenting. :)<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I'm assuming this will be some type of per call site metadata?<br>
</blockquote>
We do assign metadata at the indirect call sites. Format looks like as<br>
follows:<br>
<br>
!33 = metadata !{metadata !"indirect_call_targets", i64<br>
<total_exec_count>, metadata !"target_fn1†, i64 <target_fn1_count>,<br>
metadata !"target_fn2†, i64 <target_fn2_count>, ….}<br>
<br>
Currently, we're recording only the top most called five function names<br>
at each indirect call site. Following the string literal<br>
“indirect_call_targets†are the fields <total_exec_count> i.e. a 64<br>
bit value for the total number of times the indirect call is executed<br>
followed by the function names and execution counts of each target.<br>
</blockquote>
This was the part I was trying to ask about. I really want to see where<br>
you're going with this optimization wise. My naive guess is that this<br>
is going to be slightly off for what you actually want.<br>
<br>
Assuming you're going for profile guided devirtualization (and thus<br>
inlining), being able to check the type of the receiver (as opposed to<br>
the result of the virtual lookup) might be advantageous. (Or, to say it<br>
differently, that's what I'm used to seeing. Your approach might be<br>
completely reasonable, it's just not what I'm used to seeing.) Have you<br>
thought about the tradeoffs here?<br>
</blockquote>
Not sure if I understood the problem here,<br>
</blockquote></div></div>
First, I am not trying to say there is a problem with your approach; I am only saying that it's not what I would have expected based on past experience. You may be entirely correct in your approach, you just need to convince me of that. :)<span class=""><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
however, we're recording both<br>
the target address and the addresses/names of the instrumented functions<br>
during the execution of the instrumented binary. During profile reading<br>
these addresses are used to match the target addresses to corresponding<br>
functions.<br>
</blockquote></span>
Ok, let's start from the basics. For profile guided devirtualization, you're constructing a cache from (something) to function pointer and using that cache lookup to enable inlining of the hot target. You have two standard choices on what to use as your cache key: the result of the virtual lookup and the inputs to the virtual lookup.<br>
<br>
Option 1 - Inputs to virtual lookup<br>
if ((receiver, vtable index) == what I predicted)<br>
tartget_I_predicted(); // inline me!!<br>
else {<br>
target = full virtual dispatch();<br>
target();<br>
}<br>
<br>
Option 2 - result of virtual lookup<br>
target = full virtual dispatch();<br>
if ('target' == what I predicted)<br>
tartget_I_predicted(); // inline me!!<br>
else {<br>
target();<br>
}<br>
<br>
You seem to be proposing option 2. I'm saying that I'm used to seeing option 1 used. Both approaches have their appeal, I'm just asking you to explain *why* you've chosen the one you apparently have.<span class=""><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
During optimization, target function name should be adequate to<br>
retrieve the function type information which then can be used to compare<br>
the types of the arguments as well as the return type w/ the call site to<br>
ensure the match.<br>
</blockquote></span>
Er, I'm now really confused. With a properly constructed check, there should be no runtime checking of types required. Can you explain why you think this is needed?</blockquote><div><br></div><div>This is compile time function signature matching. In rare cases, I have seen bad target functions get leaked into the value set (with another compiler) which requires compile time pruning -- but it is mostly a side effect of the implementation of the profiler (e.g., due to race condition etc).</div><div><br></div><div>David</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
To clarify, our interest at this time is to upstream all the<br>
infrastructure related changes. The optimizations using the profile data<br>
will not be part of our initial set of patches. On the other hand,<br>
agreeing on the metadata format at the call sites would be advantageous in<br>
terms of supporting multiple profile methods.<br>
</blockquote></span>
See my other response to this thread.<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Thanks,<br>
-Betul<br>
<br>
<br>
</blockquote>
<br>
<br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div></div>