<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jun 23, 2016 at 10:50 PM, Dehao Chen <span dir="ltr"><<a href="mailto:danielcdh@gmail.com">danielcdh@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">danielcdh added inline comments.<br>
<br>
================<br>
<span class="gmail-">Comment at: lib/CodeGen/MachineBlockPlacement.cpp:624<br>
@@ +623,3 @@<br>
+    //  order for BB->Succ to be selected, we must have F1 > F2 + F3. So the<br>
+    //  backward probability threshold is (backward checking igores other<br>
+    //  predecessors other than Pred2):<br>
----------------<br>
</span>davidxl wrote:<br>
<span class="gmail-">> davidxl wrote:<br>
> > danielcdh wrote:<br>
> > > This is a little confusing, I think the condition should be:<br>
> > ><br>
> > > F1 > max(2*F3, F2)<br>
> > ><br>
> > > So when checking Pred1, the backward threhold should be 0.66, when checking Pred2, the backward threshold should be 0.5?<br>
> > ><br>
> > > But for the current implementation, I tried all testcase, and some other testcases with different triangle-diamond combination, and it always gives me optimal solution.<br>
> > It is F1 > F2 + F3.<br>
> ><br>
> > Basically, we are comparing selecting BB->Succ as fall through vs selecting BB->Pred1 and Pred2->Succ as fall throughs. In order for BB->Succ to succeed, the minimal min(F1) = F2 + F3.<br>
> ><br>
> > When computing backward probability, only BB->Succ and Pred2->Succ edges are considered, so the probability threshold<br>
> >   T = min(F1) /(min(F1) + F2) = (F2 + F3 )/(2*F2 + F3).<br>
> ><br>
> > I will update the comments.<br>
</span>> Dehao's question:<br>
<span class="gmail-">><br>
> For case C, if F1=50, F2=40, F3=20, it seems it's most beneficial to choose BB->Succ as fall-through than Pred1->Succ or Pred2->Succ, or am I missing something?<br>
><br>
</span><span class="gmail-">> The answer is, in this case, it is better not to choose BB->Succ as the fall through.<br>
><br>
> Let's take a look at an example: suppose BB and Pred2 in example C has a predecessor 'S'. With your choice, the layout is<br>
><br>
> S, BB, Succ, Pred2, Pred1  with cost F2 + F3 + F2 + F3<br>
><br>
> The optimal layout is in fact<br>
><br>
> S, BB, P1, P2, Succ with cost  F1 + F2 + F3<br>
><br>
><br>
</span>I see. Thanks for explanation.<br>
<br>
It may worth mentioning in the comment that the threshold is F1>max(2*F3, F2+F3), because F2>F3, thus F1>F2+F3.<br></blockquote><div><br></div><div>It is actually F1 > max(Freq(BB->X), for all X except Succ) + max(Freq(Y->Succ), for all Y except BB), where the first term is F3, the second is F3.</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<br>
================<br>
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:664<br>
@@ +663,3 @@<br>
+    BlockFrequency F2, F3;<br>
+    if (MDT->properlyDominates(BB, PredWithMaxFreq)) {<br>
+      // Scenario (A)<br>
----------------<br>
Maybe combine the two scenarios to something like:<br>
<br>
F2 = std::max(MBFI->getBlockFreq(BB) * SuccProb.getCompl();, MaxPredEdgeFreq);<br>
F3 = std::min(MBFI->getBlockFreq(BB) * SuccProb.getCompl();, MaxPredEdgeFreq);<br></blockquote><div><br></div><div>This may not always be true for scenario C -- consider extending scenario A such that there is another Pred coming into Succ with the MaxPredEdgeFreq. This MaxPredEdgeFreq may not be larger than MBFI->getBlockFreq(BB) * SuccProb.getCompl().</div><div><br></div><div>David</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<br>
<br>
<a href="http://reviews.llvm.org/D21663" rel="noreferrer">http://reviews.llvm.org/D21663</a><br>
<br>
<br>
<br>
</blockquote></div><br></div></div>