FYI, I finally got back around to this and committed it in r161409. I implemented the heuristic you suggested of never aligning blocks once they are cold relative to the function entry, that makes perfect sense.<div><br></div>
<div>I've kept the LoopInfo-based logic as I think it's a nice simple way to reason about what the right strategy is, and we already are using LoopInfo if we do block placement at all, so we might as well re-use that data....</div>
<div><br></div><div>Let me know if you have any other thoughts on this, I'm happy to keep tweaking or poking it until its right. Also let me know if you see and perf regressions that seem likely related.</div><div class="gmail_extra">
<br><br><div class="gmail_quote">On Mon, Jul 23, 2012 at 8:34 PM, Andrew Trick <span dir="ltr"><<a href="mailto:atrick@apple.com" target="_blank">atrick@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word"><div class="im"><div><div>On Jul 16, 2012, at 6:24 AM, Chandler Carruth <<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.com</a>> wrote:</div><br><blockquote type="cite">
<div style="font-family:Helvetica;font-size:medium;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:-webkit-auto;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<span> </span>Attached is a patch that instead walks the blocks and considers various aspects of the layout and the probabilities to decide whether aligning the block is likely to be profitable. It uses completely arbitrary heuristics which happen to produce results similar to the previous results, but with specific fixes to obviously ridiculous alignment such as reported in the PR.</div>
<div style="font-family:Helvetica;font-size:medium;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:-webkit-auto;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<br></div><div style="font-family:Helvetica;font-size:medium;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:-webkit-auto;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
It errs slightly in the direction of aligning fewer branch targets. To my mind this is the correct slant because alignment introduces significant code bloat and Agners tells me to be very judicious in aligning things as it rarely makes a large difference. If anything, I'd like to make this *more* conservative, but I'd rather take baby steps.</div>
<div style="font-family:Helvetica;font-size:medium;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:-webkit-auto;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<br></div><div style="font-family:Helvetica;font-size:medium;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:-webkit-auto;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
Does this initial step look good?</div><div style="font-family:Helvetica;font-size:medium;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:-webkit-auto;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<br></div><div style="font-family:Helvetica;font-size:medium;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:-webkit-auto;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
Thoughts about how far to go here? I have a follow-up patch that makes the whole thing tunable with a commandline flag. I can apply that and run some in depth benchmarks, but I suspect that the patch as-is already represents a strict improvement over the status quo...</div>
<div style="font-family:Helvetica;font-size:medium;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:-webkit-auto;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
-Chandler</div></blockquote></div><br></div><div><div>This looks like a good improvement. I'm not used to determining alignment without knowing instruction sizes and the actual alignment cost... But your approach makes sense.</div>
<div><br></div><div>+    // If the block is cold relative to its loop header, don't align it</div><div>+    // regardless of what edges into the block exist.</div><div><br></div><div>What if it's cold relative to its outer loop, or parent function? Cold code can have nested loops. If you can get away with frequency relative to function entry and layout predecessor, then you don't need LoopInfo here.</div>
</div><div><br></div><div>-Andy</div></div><br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div>