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

David Li via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 17 18:28:22 PST 2015


davidxl 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);
   }
----------------
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.

================
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);
----------------
Why? Zero probability should be allowed.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:2048
@@ +2047,3 @@
+  addSuccessorWithProb(SwitchBB, B.Default, B.DefaultProb);
+  addSuccessorWithProb(SwitchBB, MBB, B.Prob);
+  SwitchBB->normalizeSuccProbs();
----------------
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.


http://reviews.llvm.org/D14361





More information about the llvm-commits mailing list