<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Feb 5, 2014 at 4:34 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@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"><div class="gmail_extra"><br><div class="gmail_quote"><div class="im">On Wed, Feb 5, 2014 at 3:58 PM, Manman Ren <span dir="ltr"><<a href="mailto:manman.ren@gmail.com" target="_blank">manman.ren@gmail.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>I thought it is straight-forward to use a lower threshold for cold functions and didn't think this patch is going to have a wide & dramatic impact.</div>


<div><br></div><div>I am not aware of any benchmark in lnt that uses cold attribute. If there is one, please let me know so I can collect some performance number.</div></blockquote><div><br></div></div><div>Surely there are benchmarks with more representative code than SPEC? Maybe not. There is also no cold attribute in the test suite.</div>
<div class="im">
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><br></div><div>When Diego first introduced the Cold attribute and used it to adjust block weights, there was no request on benchmark numbers. So I assumed negligible performance impact for this patch.</div>

</blockquote><div><br></div></div><div>Sure, but there wasn't any use of cold in LLVM before. Also, changing inliner heuristics has an *incredibly* broader impact. It literally has the largest impact in the entire compiler.</div>
</div></div></div></blockquote><div><br></div><div>Agree. </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">

<div><br></div><div>Notably, his change didn't introduce any particularly interesting thresholds. It reused existing thresholds for __builtin_expect IIRC.</div><div><br></div><div>There at least needs to be *some* science behind the use of 75 for the magical threshold here. When setting the other thresholds both Jakob and I would run large benchmark suites across a wide range of values and try to dig out the shape of the graph.</div>
</div></div></div></blockquote><div><br></div><div>When setting threshold for inlinehint, do you recall what benchmark suites we collected data on?</div><div><br></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"><div class="im">
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br></div><div>Sorry if this caused regressions. The main purpose of this is performance of instrumentation based PGO.</div></blockquote></div></div><br>I understand that this is the intent, but the whole point of PGO just feeding the existing ways to represent things in the IR is because we already use those when we *don't* have a profile, and any optimizations will apply to both.</div>

<div class="gmail_extra"><br></div><div class="gmail_extra"><br></div><div class="gmail_extra">I'd like to see some numbers behind the use of "75" in the inliner.</div></div></blockquote><div><br></div><div>
75 is the threshold we use when optimizing for size (OptSizeThreshold). I used the same number for cold functions.</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"> Until then, I'd ask to keep the actual threshold unchanged and just introduce the flag so that folks can conduct experiments to collect this data. Then you can send a request to the mailing list that folks conduct such experiments and collect feedback from the community. This pattern has worked extremely well recently as Arnold has added new features and tuned thresholds in the vectorizer. I think the inliner deserves the same or more scrutiny.</div>
</div></blockquote><div> </div><div>Sure. I will set the default inlinecold-threshold to be the same as inline-threshold (225). About conducting experiments, without PGO setting cold attribute, we will need benchmarks that actually use "cold" attribute.</div>
</div><br></div><div class="gmail_extra">Thanks,</div><div class="gmail_extra">Manman</div></div>