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

Manman Ren via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 6 20:30:38 PST 2015


manmanren added a comment.

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


================
Comment at: lib/CodeGen/MachineBasicBlock.cpp:534
@@ +533,3 @@
+    // weight. This will be removed once all weight-version interfaces in MBB
+    // are replaces with probability-version interfaces.
+    Weights.push_back(Prob.getNumerator());
----------------
typo: replaced
Thanks for doing extra to make code review easier!

================
Comment at: lib/CodeGen/MachineBasicBlock.cpp:565
@@ -556,1 +564,3 @@
+  // passes. Uncomment it once all edge weights are replaced with probabilities.
+#if 0
   // If probability list is empty it means we don't use it (disabled
----------------
I don't quite get why we need to comment out this. When we support both weights and branch probabilities, should we keep both? We can remove weights in the end, right?

================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:8425
@@ -8426,1 +8424,3 @@
+  thisMBB->addSuccessor(mainMBB, /* probability */ {1, 100});
+  thisMBB->addSuccessor(sinkMBB, /* probability */ {99, 100});
 
----------------
This change seems to be different from the weight distribution before. Can you add comments on why?


http://reviews.llvm.org/D14361





More information about the llvm-commits mailing list