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

[Cong Hou via llvm-commits]

congh added inline comments.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:1312
@@ -1311,3 +1311,3 @@
     UnwindDest.first->setIsEHPad();
-    addSuccessorWithWeight(FuncInfo.MBB, UnwindDest.first, UnwindDest.second);
+    addSuccessorWithProb(FuncInfo.MBB, UnwindDest.first, UnwindDest.second);
   }
----------------
davidxl wrote:
> How can we make sure it is not a bug in the producer? With Probability based interface, we have a chance to enforce the producer to behave.
Agree.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:1518
@@ -1510,4 +1517,3 @@
   else {
-    if (!Weight)
-      Weight = getEdgeWeight(Src, Dst);
-    Src->addSuccessor(Dst, Weight);
+    if (Prob.isZero())
+      Prob = getEdgeProbability(Src, Dst);
----------------
davidxl wrote:
> Why? Zero probability should be allowed.
OK, I think I forget to update this part.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:2048
@@ +2047,3 @@
+  addSuccessorWithProb(SwitchBB, B.Default, B.DefaultProb);
+  addSuccessorWithProb(SwitchBB, MBB, B.Prob);
+  SwitchBB->normalizeSuccProbs();
----------------
davidxl wrote:
> Why can't the producer guarantee the sum is one ?-- in theory, the caller should do the normalization based on the update logic there . Blindly do normalization here may hide bugs. It is not possible for the consumer to decide if normalization is expected or there is a bug in producer.
I think bug checking is a point here. In some cases probabilities calculation is complex, and may be done in several places, and that is when validating if the sum of successors' probabilities is one is difficult. But I agree with you. Let's find a way to validate the probabilities in the future.


http://reviews.llvm.org/D14361