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

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: lib/CodeGen/MachineBasicBlock.cpp:551
@@ -544,3 +550,3 @@
 
 void MachineBasicBlock::removeSuccessor(MachineBasicBlock *Succ) {
   Succ->removePredecessor(this);
----------------
Can you refactor this function let it call the iterator based removeSuccessor (in a different patch)?

================
Comment at: lib/CodeGen/MachineBasicBlock.cpp:587
@@ -575,1 +586,3 @@
 
+  // FIXME: Temporarily comment the following code as probabilities are now only
+  // used during instruction lowering, but this interface is called in later
----------------
Is it better to relax getProbabilityIterator for now to not assert and return end iterator instead?

Also can the opposite situation happen? The removed old BB does not have an corresponding weight? In that case, it is better to relax the getXXXiterator for now.

================
Comment at: lib/CodeGen/MachineBasicBlock.cpp:635
@@ -618,3 +634,3 @@
   // Update its weight instead of adding a duplicate edge.
   if (!Weights.empty()) {
     weight_iterator OldWI = getWeightIterator(OldI);
----------------
The refactored removeSuccessor can call an internal function

removeSuccessor(succ_iterator OldI, succ_iterator NewI) -- when NewI is end(), do not update NewI's weight/prob.

replaceSuccessor can then call this method as well.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:1632
@@ -1632,3 +1631,3 @@
     FindMergedConditions(BOp->getOperand(1), TBB, FBB, TmpBB, SwitchBB, Opc,
-                         NewTrueWeight, NewFalseWeight);
+                         NewTrueProb, NewFalseProb);
   } else {
----------------
I think it will be very useful if you can introduce an interface like this (a public static member of BranchProbability) f0r the most common case:

void getNormalizedProbabilities(BranchProbability& TrueProb, BranchProbability& FalseBB);

With this interface, the low level manipulation of numerator and denominator can be eliminated and the code becomes more readable.

Note that one of the benefits of using BP interface is it helps reasoning in updating -- so we should definitely avoid doing the low level synthesis above.




http://reviews.llvm.org/D14361