<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 4, 2015 at 1:16 PM, Xinliang David Li <span dir="ltr"><<a href="mailto:xinliangli@gmail.com" target="_blank">xinliangli@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Wed, Mar 4, 2015 at 11:58 AM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@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 dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On Wed, Mar 4, 2015 at 10:46 AM, Xinliang David Li <span dir="ltr"><<a href="mailto:xinliangli@gmail.com" target="_blank">xinliangli@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 dir="ltr">I am not sure the new layout strategy will work well consistently.</div></blockquote><div><br></div></span><div>There is at least one bug in the new strategy that I know of, and generally its behind a flag because it isn't as well tested or understood yet... Anyways.</div><span><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>1) if 'if (a1())' and 'if (a2())' branches are equally likely, the new layout does not actually improve icache utilization -- it just trades one for another</div></div></blockquote><div><br></div></span><div>? Before, we would *interleave* the basic blocks for the two unlikely branches. If we end up taking one of them, this would force another discontinuity.</div><span><div> </div></span></div></div></div></blockquote><div><br></div></span><div>The new heuristic is basically another way to enforce topological order which may not work. 'd1'  or 'd2' in this case can be large and cold, and it is better to put them far way in hat condition.</div></blockquote><div><br></div><div>Yes, but I think that too is orthogonal. We currently will do this when they are cold but not when they are large. The outlining structural hint is perhaps the start of a way to consider when they are also large. We don't currently use size in the layout heuristics and we clearly should, but no one has prototyped a set of heuristics on that yet. I think Daniel has some thoughts there, but hasn't yet gotten to it because the structural hints don't yet seem correct and it seems better to get those right first.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><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 dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>2) if 'if(a2()' is less likely, i.e. 'b2,c2,d2,f2' are much colder, the new strategy will reduce reuse.</div></div></blockquote><div><br></div></span><div>I don't understand this comment? Can you give an example?</div></div></div></div></blockquote><div><br></div><div><br></div></span><div>This is the side effect of the new placement strategy -- basically sequence 'b2,c2, ..' is always laid out before 'b1,c1, ....'. 'b2,c2..' can be colder, and placing them closer to a1,a2,done can reduce locality. </div></blockquote></div><br>Ah, I see. Yes, this is the core bug that I mentioned. I had hoped there would be some simple way to address it, but it looks like those have other problems. =[</div><div class="gmail_extra"><br></div><div class="gmail_extra">After a bunch of profiling and testing of the flag with Daniel, I'm going to try a different way to achieve the same results without fully reversing the order here. Sadly, it's a *ton* more work and looks completely different. I'll probably revert this patch and try the other approach.</div></div>