[PATCH] D14361: Let SelectionDAG start to use probability-based interface to add successors.

[Manman Ren via llvm-commits]

On Mon, Nov 9, 2015 at 3:11 PM, Cong Hou <congh at google.com> wrote:

> congh added a comment.
>
> In http://reviews.llvm.org/D14361#285639, @manmanren wrote:
>
> > In http://reviews.llvm.org/D14361#285380, @congh wrote:
> >
> > > In http://reviews.llvm.org/D14361#284529, @manmanren wrote:
> > >
> > > > It seems that we can submit the changes other than
> SelectionDAGBuilder.h, SelectionDAGBuilder.cpp and SelectionDAGISel.cpp,
> separately. Correct me if I am wrong.
> > > >
> > > > Thanks for working on this!
> > > > Manman
> > >
> > >
> > > 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.
> >
> >
> > 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.
>
>
> SelectionDAGBuilder won't use probability but only assigns correct
> weight/probability to new CFG.
>
> > 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?
>
>
> 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.
>

Does it mean you can commit the change in
lib/Target/X86/X86FrameLowering.cpp separately then?

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

Cheers,
Manman


> > I may have missed some obvious things :)
>
>
> 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:)
>
> > Cheers,
>
> > Manman
>
>
>
> http://reviews.llvm.org/D14361
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151109/e29371ba/attachment.html>