<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jan 24, 2017 at 11:38 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:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="gmail-">On Tue, Jan 24, 2017 at 11:20 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:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Tue, Jan 24, 2017 at 11:15 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:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Tue, Jan 24, 2017 at 11:13 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:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>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></span><span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>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:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);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></span><div>in real applications, the static call site for memcpy and memset is not small. </div><span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);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></div></div></div></blockquote></span><div>we collect the precise size for small sized memcpy and memset. Most of the callsite has multiple size values. </div></div></div></div></blockquote><div><br></div></span><div>My question is how is this affected by the way it is implemented: 1) using API call ; 2) generating inline sequence of interval profiling.</div></div></div></div></blockquote><div><br></div></span><div>using API, I have the exact count of the range instrumentations (and the range) so I can adjust the counter allocation in lowering (different from indirect calls)</div><div>On the other hand, if we do the inline sequence in instrumentation, we treat them the same as current indirect calls instrumentation. </div><span><div> </div></span></div></div></div></blockquote><div><br></div></span><div>Did I miss anything? I do not see any special handling of static counter allocation in the patches. The default counters per-site is still unchanged?</div></div></div></div></blockquote><div><br></div><div>in InstrProfiling.cpp: function </div><div>void InstrProfiling::emitVNodes()</div><div>TotalNS is adjusted based on value profile kind.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="gmail-HOEnZb"><font color="#888888"><div><br></div><div>David</div></font></span><span class="gmail-"><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>Anyway, see my previous reply -- I think having a minimal wrapper API in compiler rt is fine.</div><span class="gmail-m_-7706026205100781058m_2295352276965750499HOEnZb"><font color="#888888"><div><br></div><div>David</div></font></span><span><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="gmail-m_-7706026205100781058m_2295352276965750499m_7333076623393369728m_-9037390898928238749HOEnZb"><font color="#888888"><div><br></div><div>David</div></font></span><span><div> </div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);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="gmail-m_-7706026205100781058m_2295352276965750499m_7333076623393369728m_-9037390898928238749m_2112626852295830731HOEnZb"><div class="gmail-m_-7706026205100781058m_2295352276965750499m_7333076623393369728m_-9037390898928238749m_2112626852295830731h5"><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:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);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="gmail-m_-7706026205100781058m_2295352276965750499m_7333076623393369728m_-9037390898928238749m_2112626852295830731m_-5533819942857421105HOEnZb"><div class="gmail-m_-7706026205100781058m_2295352276965750499m_7333076623393369728m_-9037390898928238749m_2112626852295830731m_-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></span></div><br></div></div>
</blockquote></span></div><br></div></div>
</blockquote></span></div><br></div></div>
</blockquote></span></div><br></div></div>
</blockquote></div><br></div></div>