<div dir="ltr">As we discussed offline, there are cases that the taildup can hurt make things worse (especially when there is no profile data). We should probably revert this patch for now and continue discussion in the original patch.<div><br></div><div>David</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jan 11, 2017 at 11:43 AM, Kyle Butt <span dir="ltr"><<a href="mailto:iteratee@google.com" target="_blank">iteratee@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 Wed, Jan 11, 2017 at 11:07 AM, Kyle Butt <span dir="ltr"><<a href="mailto:iteratee@google.com" class="m_-4575271657910435869gmail-cremed m_-4575271657910435869cremed" target="_blank">iteratee@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"><div>I'm not sure I understand. I looked at his test case and I get the same layout on X86 as I do on amdgcn.</div><div><br></div><div class="gmail_extra">Matt: There are 4 cases:</div><div class="gmail_extra"><br></div><div class="gmail_extra"><font face="monospace, monospace">bb2 bb6 Probability (As guessed by heuristics) Taken branch count original Taken branch count new</font></div><div class="gmail_extra"><font face="monospace, monospace"> N N 23.4% 2 1 (unconditional)</font></div><div class="gmail_extra"><font face="monospace, monospace"> N Y 39.1% 1 1</font></div><div class="gmail_extra"><font face="monospace, monospace"> Y N 14.1% 1 1</font></div></div></blockquote></span><div>^^^^^^ This line is wrong</div><div> <span style="font-family:monospace,monospace"> Y N 14.1% 1 2</span></div><span class=""><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"><font face="monospace, monospace"> Y Y 23.4% 0 1</font></div><div class="gmail_extra"><font face="monospace, monospace"><br></font></div><div class="gmail_extra"><font face="arial, helvetica, sans-serif">By my count, I see the same score for both, and with <a href="https://reviews.llvm.org/D28522" class="m_-4575271657910435869gmail-cremed m_-4575271657910435869cremed" target="_blank">https://reviews.llvm.org/<wbr>D28522</a>, the branch to the exit block will disappear in your example, making the new layout strictly better.</font></div></div></blockquote><div><br></div></span><div>You're right. This is worse. <br></div><div><br></div><div>Sorry, I can't count.</div><span class=""><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 class="m_-4575271657910435869gmail-h5"><div class="gmail_extra"><font face="monospace, monospace"><br></font></div><div class="gmail_extra"><div class="gmail_quote">On Wed, Jan 11, 2017 at 9:36 AM, Xinliang David Li <span dir="ltr"><<a href="mailto:davidxl@google.com" class="m_-4575271657910435869gmail-m_1952705816574652369gmail-cremed m_-4575271657910435869gmail-m_1952705816574652369cremed m_-4575271657910435869gmail-cremed m_-4575271657910435869cremed" 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">Kyle, I looked at Matt's case in more details. For this target, bb2 in is selected as fallthrough of bb in the final layout, so bb4 should not be a taildup'ed. Can you take a look what went wrong?<div><br></div><div>By comparison, on x86, the cloned bb4 is the layout successor of bb which is expected. </div><span class="m_-4575271657910435869gmail-m_1952705816574652369gmail-HOEnZb"><font color="#888888"><div><br></div><div>David</div></font></span></div><div class="m_-4575271657910435869gmail-m_1952705816574652369gmail-HOEnZb"><div class="m_-4575271657910435869gmail-m_1952705816574652369gmail-h5"><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jan 11, 2017 at 12:06 AM, Matt Arsenault <span dir="ltr"><<a href="mailto:arsenm2@gmail.com" class="m_-4575271657910435869gmail-m_1952705816574652369gmail-cremed m_-4575271657910435869gmail-m_1952705816574652369cremed m_-4575271657910435869gmail-cremed m_-4575271657910435869cremed" target="_blank">arsenm2@gmail.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 style="word-wrap:break-word"><span><br><div><blockquote type="cite"><div>On Jan 10, 2017, at 19:13, Kyle Butt <<a href="mailto:iteratee@google.com" class="m_-4575271657910435869gmail-m_1952705816574652369gmail-cremed m_-4575271657910435869gmail-m_1952705816574652369cremed m_-4575271657910435869gmail-cremed m_-4575271657910435869cremed" target="_blank">iteratee@google.com</a>> wrote:</div><div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="m_-4575271657910435869gmail-m_1952705816574652369gmail-m_1497048604110866542m_7537539302510827896HOEnZb"><font color="#888888"><br></font></span></blockquote><div><br></div><div>I looked at the code in question. There are more compare instructions, but no codepath should execute more of them. Which codepath are you concerned about?</div><div><br></div><div>For the compare, and 1 of the branches, it occurs due to tail duplication, and so for those, this is not a regression, it is WAI.</div><div><br></div><div>Are you worried about the code size, or did this actually cause a performance regression?</div><div>If it did cause a regression, can you tell me which path is the hot path? </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="m_-4575271657910435869gmail-m_1952705816574652369gmail-m_1497048604110866542m_7537539302510827896HOEnZb"><font color="#888888">
-Matt<br>
<br>
</font></span></blockquote></div><br></div><div class="gmail_extra">Thanks,</div><div class="gmail_extra">Kyle.</div></div>
</div></blockquote></div><br></span><div>This changes from having a path where no branch occurs, to ensuring that a branch will occur, and branches are expensive. I noticed this from the code size changes, but I’m mostly surprised by replacing a fall through with a branch.</div><div><br></div><div>Looking at the expected cycle counts on all paths in the artificial testcase, the loads + waits are always skipped, which is good. I think if the waitcnts were inserted smarter, the original code CFG would be slightly better. I need to look more at the full testcase.</div><span class="m_-4575271657910435869gmail-m_1952705816574652369gmail-m_1497048604110866542HOEnZb"><font color="#888888"><div><br></div><div>-Matt</div></font></span></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div></div></div></div>
</blockquote></span></div><br></div></div>
</blockquote></div><br></div>