<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">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><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><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><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><div> </div><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><div>I don't understand this comment? Can you give an example?</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><br></div><div>The key problem is that why 'f1' is not picked as the successor for c1:</div><div> presumably it (f1, which is a merge point) is not picked because there is more likely predecessor, which is 'd1'. If 'd1' is more likely, 'd1' should be picked as successor of 'b1' -- picking 'c1' after 'b1' does not make sense.   If 'd1' is unlikely, then 'f1' should follow 'c1', and 'd1' should be outlined and placed far away -- which will improve cache reuse and reduce taken branches.</div></div></blockquote><div><br></div><div>But what if c1 and d1 are the *same probability*. That is the case here. If there were strong profile information, we would lay this out exactly as you describe, but in this case there isn't.</div><div><br></div><div>When there isn't any specific probability, the code intends to lay out diamonds in a cohesive chain: b1, c1, d1, f1 or b1, d1, c1, f1 would both be reasonable. What we got before this change was b1, c1, b2, c2, d1, f1, d2, f2, which seems really bad.</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>Unlike triangle shaped pattern if (a()) { b(); } c();, for diamond shape control flow, when real profile data is available (or branch annotation or prediction can be trusted), it is almost always better to break topological constraint using branch probability. <br></div></div></blockquote></div><br></div><div class="gmail_extra">We have specific logic to do this when we have strong profile information, but that logic isn't what this is changing. That logic may need improvements as well, but those should be orthogonal to this change.</div></div>