<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
<div class="moz-cite-prefix">On 04/12/2015 09:51 PM, Bob Wilson
wrote:<br>
</div>
<blockquote
cite="mid:FE1F9B02-FD2D-484C-8E98-044B91D6F041@apple.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<br class="">
<div>
<blockquote type="cite" class="">
<div class="">On Apr 10, 2015, at 9:25 AM, <a
moz-do-not-send="true" href="mailto:betulb@codeaurora.org"
class="">betulb@codeaurora.org</a> wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<div style="" class="">
<blockquote type="cite" style="font-family: Helvetica;
font-size: 12px; font-style: normal; font-variant:
normal; font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class=""><br class="">
On 04/09/2015 11:06 AM, Betul Buyukkurt wrote:<br
class="">
<blockquote type="cite" class=""><br class="">
In <a moz-do-not-send="true"
href="http://reviews.llvm.org/D8908#153838" class="">http://reviews.llvm.org/D8908#153838</a>,
@reames wrote:<br class="">
<br class="">
<blockquote type="cite" class="">Have the IR level
construct patches made it up for review? If so, can<br
class="">
</blockquote>
<br class="">
So far I've posted two patches. These two patches
should apply cleanly<br class="">
to the tip, working with the present profile
infrastructure. The next<br class="">
set of patches will be the enabler ones: i.e. three
more patches one for<br class="">
each of clang, llvm and compiler-rt. Clang patch will
be up for review<br class="">
later today.<br class="">
<br class="">
<blockquote type="cite" class="">you send me a link?
I managed to miss them.<br class="">
</blockquote>
<br class="">
So far there is this patch and the instrinsic
instruction definitions:<br class="">
<a moz-do-not-send="true"
href="http://reviews.llvm.org/D8877" class="">http://reviews.llvm.org/D8877</a>.
All patches are necessary for getting the<br class="">
IC targets and having them displayed by the
llvm-profdata.<br class="">
</blockquote>
Ok, I'm really not convinced that the instrumentation
code needs to be<br class="">
or should be an intrinsic. This seems like something
which should be<br class="">
emitted by the frontend and optimized like any other
code. To say this<br class="">
a different way, my instrumentation is going to be
entirely different<br class="">
than your instrumentation.<br class="">
<br class="">
Having said that, I really don't care about this part of
the proposed<br class="">
changes since they aren't going to impact me at all.
I'm am<br class="">
specifically not objecting to the changes, just
commenting. :)<br class="">
<blockquote type="cite" class=""><br class="">
<blockquote type="cite" class="">I'm assuming this
will be some type of per call site metadata?<br
class="">
</blockquote>
<br class="">
We do assign metadata at the indirect call sites.
Format looks like as<br class="">
follows:<br class="">
<br class="">
!33 = metadata !{metadata !"indirect_call_targets",
i64<br class="">
<total_exec_count>, metadata !"target_fn1â,
i64 <target_fn1_count>,<br class="">
metadata !"target_fn2â, i64
<target_fn2_count>, â¦.}<br class="">
<br class="">
Currently, we're recording only the top most called
five function names<br class="">
at each indirect call site. Following the string
literal<br class="">
âindirect_call_targetsâ are the fields
<total_exec_count> i.e. a 64<br class="">
bit value for the total number of times the indirect
call is executed<br class="">
followed by the function names and execution counts of
each target.<br class="">
</blockquote>
This was the part I was trying to ask about. I really
want to see where<br class="">
you're going with this optimization wise. My naive
guess is that this<br class="">
is going to be slightly off for what you actually want.<br
class="">
<br class="">
Assuming you're going for profile guided
devirtualization (and thus<br class="">
inlining), being able to check the type of the receiver
(as opposed to<br class="">
the result of the virtual lookup) might be advantageous.
(Or, to say it<br class="">
differently, that's what I'm used to seeing. Your
approach might be<br class="">
completely reasonable, it's just not what I'm used to
seeing.) Have you<br class="">
thought about the tradeoffs here?<br class="">
</blockquote>
<br style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;"
class="">
<span style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
float: none; display: inline !important;" class="">Not
sure if I understood the problem here, however, we're
recording both</span><br style="font-family: Helvetica;
font-size: 12px; font-style: normal; font-variant:
normal; font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
float: none; display: inline !important;" class="">the
target address and the addresses/names of the
instrumented functions</span><br style="font-family:
Helvetica; font-size: 12px; font-style: normal;
font-variant: normal; font-weight: normal;
letter-spacing: normal; line-height: normal; orphans:
auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;"
class="">
<span style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
float: none; display: inline !important;" class="">during
the execution of the instrumented binary. During profile
reading</span><br style="font-family: Helvetica;
font-size: 12px; font-style: normal; font-variant:
normal; font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
float: none; display: inline !important;" class="">these
addresses are used to match the target addresses to
corresponding</span><br style="font-family: Helvetica;
font-size: 12px; font-style: normal; font-variant:
normal; font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
float: none; display: inline !important;" class="">functions.
During optimization, target function name should be
adequate to</span><br style="font-family: Helvetica;
font-size: 12px; font-style: normal; font-variant:
normal; font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
float: none; display: inline !important;" class="">retrieve
the function type information which then can be used to
compare</span><br style="font-family: Helvetica;
font-size: 12px; font-style: normal; font-variant:
normal; font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
float: none; display: inline !important;" class="">the
types of the arguments as well as the return type w/ the
call site to</span><br style="font-family: Helvetica;
font-size: 12px; font-style: normal; font-variant:
normal; font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
float: none; display: inline !important;" class="">ensure
the match.</span><br style="font-family: Helvetica;
font-size: 12px; font-style: normal; font-variant:
normal; font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<br style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;"
class="">
<span style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
float: none; display: inline !important;" class="">To
clarify, our interest at this time is to upstream all
the</span><br style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant: normal;
font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
float: none; display: inline !important;" class="">infrastructure
related changes. The optimizations using the profile
data</span><br style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant: normal;
font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
float: none; display: inline !important;" class="">will
not be part of our initial set of patches. On the other
hand,</span><br style="font-family: Helvetica;
font-size: 12px; font-style: normal; font-variant:
normal; font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
float: none; display: inline !important;" class="">agreeing
on the metadata format at the call sites would be
advantageous in</span><br style="font-family: Helvetica;
font-size: 12px; font-style: normal; font-variant:
normal; font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
float: none; display: inline !important;" class="">terms
of supporting multiple profile methods.</span><br
style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;"
class="">
<br style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;"
class="">
<span style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
float: none; display: inline !important;" class="">Thanks,</span><br
style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;"
class="">
<span style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
float: none; display: inline !important;" class="">-Betul</span><br
style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: auto;
word-spacing: 0px; -webkit-text-stroke-width: 0px;"
class="">
</div>
</div>
</blockquote>
<div><br class="">
</div>
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.</div>
<div><br class="">
</div>
<div>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.</div>
</blockquote>
I strongly agree with this comment. You're optimization patches
don't need to be polished, but we do need to see something to
justify the approach you're taking with the instrumentation. To be
blunt, the instrumentation is *pointless* unless it leads to some
useful result. That result doesn't necessarily be optimization, but
that's the default one I'm assuming you're going for. If your
objective is something else, please feel free to say so.<br>
<br>
Philip<br>
</body>
</html>