<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Oct 28, 2016 at 10:39 AM, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div style="font-family:arial,helvetica,sans-serif;font-size:10pt;color:#000000"><br><hr id="m_8339029640217828513zwchr"><blockquote style="border-left:2px solid rgb(16,16,255);margin-left:5px;padding-left:5px;color:rgb(0,0,0);font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt"><span class=""><b>From: </b>"Xinliang David Li" <<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>><br></span><b>To: </b>"Hal Finkel" <<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>><br><b>Cc: </b>"陈德颢" <<a href="mailto:danielcdh@gmail.com" target="_blank">danielcdh@gmail.com</a>>, "Junbum Lim" <<a href="mailto:junbuml@codeaurora.org" target="_blank">junbuml@codeaurora.org</a>>, "llvm-commits" <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>>, <a href="mailto:anna@azul.com" target="_blank">anna@azul.com</a>, <a href="mailto:reviews%2BD26037%2Bpublic%2B89d2f1349c56fe36@reviews.llvm.org" target="_blank">reviews+D26037+public+<wbr>89d2f1349c56fe36@reviews.llvm.<wbr>org</a><br><b>Sent: </b>Friday, October 28, 2016 12:26:39 PM<span class=""><br><b>Subject: </b>Re: [PATCH] D26037: Add LoopSink pass for PGO.<br><br><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Oct 28, 2016 at 10:20 AM, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div style="font-family:arial,helvetica,sans-serif;font-size:10pt;color:rgb(0,0,0)"><br><hr id="m_8339029640217828513m_-617916386397224861zwchr"><blockquote style="border-left:2px solid rgb(16,16,255);margin-left:5px;padding-left:5px;color:rgb(0,0,0);font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt"><b>From: </b>"Xinliang David Li" <<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>><br><b>To: </b><a href="mailto:reviews%2BD26037%2Bpublic%2B89d2f1349c56fe36@reviews.llvm.org" target="_blank">reviews+D26037+public+<wbr>89d2f1349c56fe36@reviews.llvm.<wbr>org</a><br><b>Cc: </b>"陈德颢" <<a href="mailto:danielcdh@gmail.com" target="_blank">danielcdh@gmail.com</a>>, "Hal Finkel" <<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>>, "Junbum Lim" <<a href="mailto:junbuml@codeaurora.org" target="_blank">junbuml@codeaurora.org</a>>, "llvm-commits" <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>>, <a href="mailto:anna@azul.com" target="_blank">anna@azul.com</a><br><b>Sent: </b>Friday, October 28, 2016 12:11:04 PM<br><b>Subject: </b>Re: [PATCH] D26037: Add LoopSink pass for PGO.<span><br><br><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Oct 28, 2016 at 9:40 AM, Dehao Chen <span dir="ltr"><<a href="mailto:danielcdh@gmail.com" target="_blank">danielcdh@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">danielcdh added a comment.<br>
<br>
We need this pass to remove LCSSA nodes because this will be the last loop pass.<br>
<br>
Another question is: shall we pin this pass to PGO only? If not, we can simply add it to lib/Transforms/IPO/<wbr>PassManagerBuilder.cpp<br></blockquote><div><br></div><div id="m_8339029640217828513m_-617916386397224861DWT5029">Static branch prediction is conservative in predicting loop iterations (loop scale). This means it is likely some blocks (if wrapped under __builtin_expect) inside the loop will be wrongly marked as colder than preheader which can trigger loop sinking wrongly. </div></div></div></div></span></blockquote>Can you please elaborate? If we statically predict a modest number of iterations, which seems like a reasonable trade off, and we have a block which is expected to not execute (because of __builtin_expect), then is it obviously wrong that it should be colder than the preheader? If the opposite of __builtin_expect is "almost never", </div></div></blockquote><div><br></div><div><br></div><div id="m_8339029640217828513DWT5524">My observation is that the interpretation and use of '__builtin_expect' varies a lot from user to user. Some consider 80% or even as low as ~70% probability as expected, while some considers >99%. Even with 99% expected probability, it still can get wrong. For instance in a loop with 1 million iterations, the cold block will execute 10k times -- still much hotter than the preheader.</div></div></div></div></span></blockquote>Indeed; __builtin_expect would be better with a percentage ;)<br><br>However, I'm not sure that this is the best approach. Just because you have profiling data, doesn't mean you have complete profiling data. I think that it would be better if we indicated the veracity/confidence of the profiling data in the profiling metadata, and then used that.<span class="HOEnZb"><font color="#888888"><br><br></font></span></div></div></blockquote><div><br></div><div>Good point -- so checking option is not the best approach here. Better just make this into standard pipeline and let the pass to determine what to do depending on the actual availability of PGO data. We assume PGO users will use representative training data (otherwise he will shoot himself in the foot) so profile metadata from PGO has high confidence. Note that PGO meta data will overwrite meta data from user annotation, so there is no need to mark meta data with additional profile accuracy info.</div><div><br></div><div>David</div><div><br></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><div style="font-family:arial,helvetica,sans-serif;font-size:10pt;color:#000000"><span class="HOEnZb"><font color="#888888"> -Hal</font></span><span class=""><br><blockquote style="border-left:2px solid rgb(16,16,255);margin-left:5px;padding-left:5px;color:rgb(0,0,0);font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><div><br></div><div>David</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div style="font-family:arial,helvetica,sans-serif;font-size:10pt;color:rgb(0,0,0)">which is true for error paths and the like, then that seems not unreasonable. Why would loop sinking hurt there?<br><br></div></div></blockquote><div><br></div><div><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div style="font-family:arial,helvetica,sans-serif;font-size:10pt;color:rgb(0,0,0)">Thanks again,<br>Hal<span><br><blockquote style="border-left:2px solid rgb(16,16,255);margin-left:5px;padding-left:5px;color:rgb(0,0,0);font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> Making this pass depending on PGO is a safer thing to do.<br></div><div><br></div><div>David</div><div><br></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
<a href="https://reviews.llvm.org/D26037" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D26037</a><br>
<br>
<br>
<br>
</blockquote></div><br></div></div>
</blockquote><br><br><br></span><span class="m_8339029640217828513HOEnZb"><font color="#888888">-- <br><div><span></span>Hal Finkel<br>Lead, Compiler Technology and Programming Languages<br>Leadership Computing Facility<br>Argonne National Laboratory<span></span><br></div></font></span></div></div></blockquote></div><br></div></div>
</blockquote><br><br><br>-- <br><div><span name="x"></span>Hal Finkel<br>Lead, Compiler Technology and Programming Languages<br>Leadership Computing Facility<br>Argonne National Laboratory<span name="x"></span><br></div></span></div></div></blockquote></div><br></div></div>