<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jan 24, 2017 at 11:02 AM, Xinliang David Li <span dir="ltr"><<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Tue, Jan 24, 2017 at 10:57 AM, Rong Xu <span dir="ltr"><<a href="mailto:xur@google.com" target="_blank">xur@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Yes. that's another way to do this. I considered that when I implemented the code. I choose not do because:<div>(1) it makes it the instrumentation code complex and increase the code size as basically you inline the logic to every instrumentation site.</div></div></blockquote><div><br></div></span><div>We can filter a lot of memcpy sites with constant sizes, so the number of sites remaining should not be large (compared to virtual calls).</div></div></div></div></blockquote><div><br></div><div><br></div><div>To avoid code size bloat in instrumentation,  it is possible to introduce a wrapper API in compiler_rt which takes more parameters (defininig interval boundaries). The wrapper API does this</div><div>1) collapse the target value according to the interval definitition parameters</div><div>2) call into __llvm_profile_instrument_target directly.</div><div><br></div><div>The instrumentation lowerer can call the wrapper API directly.</div><div><br></div><div>David</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>(2) it affects the static counter allocation. This range profile potentially uses a lot more counters that current indirectly calls. </div></div></blockquote><div><br></div></span><div>Why is that?</div><span class="HOEnZb"><font color="#888888"><div><br></div><div>David</div></font></span><span class=""><div> </div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div></div><div>The advantage of not change compiler_rt is that less information is needed to pass b/w runtime and instrumentation. This especially true is we want to have profile client to control it's own ranges. </div><div><br></div><div>But I still think the change in compiler_rt is straight-forward way.</div></div><div class="m_2742961221791297637HOEnZb"><div class="m_2742961221791297637h5"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jan 24, 2017 at 10:33 AM, David Li via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">davidxl added a comment.<br>
<br>
In fact, I don't there is a need for compiler_rt change at all for the interval/range profiling. The compiler_rt API should be kept simple.  In other words, interval profiling should already be 'lowered' into regular value profiling in the instrumentation lowering phase. The instrumentation lowering will general inline sequence that collapse ranges into single value depending on the value profile kind.  Only the __llvm_profile_instrument_targ<wbr>et low level interface is needed.<br>
<br>
The only downside of this approach that we can not override the range definitions at runtime -- but I don't think that is worth the complexity introduced.<br>
<div class="m_2742961221791297637m_-5533819942857421105HOEnZb"><div class="m_2742961221791297637m_-5533819942857421105h5"><br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D28964" rel="noreferrer" target="_blank">https://reviews.llvm.org/D2896<wbr>4</a><br>
<br>
<br>
<br>
</div></div></blockquote></div><br></div>
</div></div></blockquote></span></div><br></div></div>
</blockquote></div><br></div></div>