<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Nov 9, 2015 at 3:11 PM, Cong Hou <span dir="ltr"><<a href="mailto:congh@google.com" target="_blank">congh@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">congh added a comment.<br>
<span class=""><br>
In <a href="http://reviews.llvm.org/D14361#285639" rel="noreferrer" target="_blank">http://reviews.llvm.org/D14361#285639</a>, @manmanren wrote:<br>
<br>
> In <a href="http://reviews.llvm.org/D14361#285380" rel="noreferrer" target="_blank">http://reviews.llvm.org/D14361#285380</a>, @congh wrote:<br>
><br>
> > In <a href="http://reviews.llvm.org/D14361#284529" rel="noreferrer" target="_blank">http://reviews.llvm.org/D14361#284529</a>, @manmanren wrote:<br>
> ><br>
> > > It seems that we can submit the changes other than SelectionDAGBuilder.h, SelectionDAGBuilder.cpp and SelectionDAGISel.cpp, separately. Correct me if I am wrong.<br>
> > ><br>
> > > Thanks for working on this!<br>
> > > Manman<br>
> ><br>
> ><br>
> > Changes under lib/Target/ should be checked in with SelectionDAG* as they are all doing instruction lowering. Other changes are for them so I think we need to put them together.<br>
><br>
><br>
> I am kind of confused. It seems to me that this change only sets up Probs array in MachineBasicBlock, but it does not start using it. When looking for getProbabilityIterator or getSuccProbability, I can't find any usage.<br>
<br>
<br>
</span>SelectionDAGBuilder won't use probability but only assigns correct weight/probability to new CFG.<br>
<span class=""><br>
> If we commit the change in lib/Target/X86/X86FrameLowering.cpp, we will be calling the new interface addSuccor(..., BranchProb), that will set up both Probs and Weights array in MachineBasicBlock, MBPI will still be using the Weights array, everything should just work, right?<br>
<br>
<br>
</span>Yes. BP can be reinterpreted as weight. As we still need weights to get probabilities, at this moment we have to always maintain weight list.<br></blockquote><div><br></div><div>Does it mean you can commit the change in lib/Target/X86/X86FrameLowering.cpp separately then?</div><div><br></div><div>We can commit patches incrementally this way to speed up the review process, thus shortening the transition period when we have both weights and probs; or we can review the rest all in one big patch if it needs a lot of extra work to have a transition period. Either way is fine with me</div><div><br></div><div>Cheers,</div><div>Manman</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<span class=""><br>
> I may have missed some obvious things :)<br>
<br>
<br>
</span>The basic idea of this patch is, we update interfaces in MBB so that SelectionDAG can use them to update probability/weight list (the former is disabled temporarily) using the new interface. Hopefully this can reduce your confusion:)<br>
<br>
> Cheers,<br>
<br>
> Manman<br>
<br>
<br>
<br>
<a href="http://reviews.llvm.org/D14361" rel="noreferrer" target="_blank">http://reviews.llvm.org/D14361</a><br>
<br>
<br>
<br>
</blockquote></div><br></div></div>