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

[Cong Hou via llvm-commits]

On Mon, Nov 9, 2015 at 3:29 PM, Manman Ren <manman.ren at gmail.com> wrote:

>
>
> 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?
>

What I am worrying about is that it is possible that the changes in
X86FrameLowering.cpp may relate in those in SelectionDAGBuilder.cpp, and
that is why I would like to include both in this patch. Or do you mean if I
can split this patch into two parts: the first just update the interfaces
and the second uses them?



> 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
>

Yes, you are right. I will try to make both review and transition easier.
Thank you!


>
> 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/185e22ce/attachment.html>