<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Nov 11, 2015 at 6:04 PM, Betul Buyukkurt <span dir="ltr"><<a href="mailto:betulb@codeaurora.org" target="_blank">betulb@codeaurora.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">> davidxl added inline comments.<br>
><br>
> ================<br>
> Comment at: lib/profile/InstrProfiling.c:81<br>
> @@ +80,3 @@<br>
> +void __llvm_profile_instrument_target(uint64_t TargetValue, void *Data_,<br>
> +  uint32_t CounterIndex) {<br>
> +<br>
> ----------------<br>
> Can we make the CounterIndex relative to the value kind? Why is value kind<br>
> parameter not passed in?<br>
<br>
</span>It's not passed in, because it's already relative to the value kind.<br>
CounterIndex is computed inside the InstrProfiling.cpp of the llvm patch<br>
during lowering. It's a value that can be computed in compile time, so<br>
better to do it then.<br></blockquote><div><br></div><div>Ok.</div><div><br></div><div>David </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
-Betul<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
><br>
> ================<br>
> Comment at: lib/profile/InstrProfiling.c:87<br>
> @@ +86,3 @@<br>
> +<br>
> +  if (!Data->ValueCounters) {<br>
> +    uint64_t NumVSites = 0;<br>
> ----------------<br>
> This part of the code is cold -- move it out of line into another routine.<br>
><br>
> ================<br>
> Comment at: lib/profile/InstrProfiling.c:119<br>
> @@ +118,3 @@<br>
> +<br>
> +  if (VDataCount >= UCHAR_MAX)<br>
> +    return;<br>
> ----------------<br>
> This may still be too large. If VDataCount is this large, the runtime<br>
> overhead will be substantial.<br>
><br>
> In the future, we should enhance it with the following algoithm:<br>
> 1) reserve the first vdata node to do book keeping -- i.e. tracking number<br>
> of time values get dropped due to too long a list<br>
> 2) When the number of dropping times reaches a threshold, sort the<br>
> existing<br>
> linked list to make it in descending order, and zero out the last X<br>
> entries.<br>
><br>
> Not needed for the first version.<br>
><br>
> ================<br>
> Comment at: lib/profile/InstrProfiling.h:33<br>
> @@ +32,3 @@<br>
> +  VK_FIRST = VK_IndirectCallTarget,<br>
> +  VK_LAST = VK_IndirectCallTarget<br>
> +} __llvm_profile_value_kind;<br>
> ----------------<br>
> Make the enumerators name the same as the LLVM counter part -- this allows<br>
> future refactoring.<br>
><br>
><br>
> <a href="http://reviews.llvm.org/D9009" rel="noreferrer" target="_blank">http://reviews.llvm.org/D9009</a><br>
><br>
><br>
><br>
><br>
<br>
<br>
</div></div></blockquote></div><br></div></div>