[PATCH] D13908: Add new interfaces to MBB for manipulating successors with probabilities instead of weights.

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: lib/CodeGen/MachineBasicBlock.cpp:527
@@ +526,3 @@
+  if (!Prob.isUnknown() && Probs.empty()) {
+    Probs.assign(Successors.size(), BranchProbability::getUnknown());
+  }
----------------
I think it is better to assert Successors.size() is 0 here. Under what situation can have mixed 'known' and 'known' probabilities?

================
Comment at: lib/CodeGen/MachineBasicBlock.cpp:1164
@@ +1163,3 @@
+  if (Probs.empty())
+    return BranchProbability(1, succ_size());
+
----------------
Is it up to the user to decide whether a default value should be returned? 

================
Comment at: lib/CodeGen/MachineBasicBlock.cpp:1167
@@ +1166,3 @@
+  auto Prob = *getProbabilityIterator(Succ);
+  if (Prob.isUnknown()) {
+    uint32_t UnknownProbNum = 0;
----------------
How is this possible? Should be it asserted that Prob is Known?

================
Comment at: lib/CodeGen/MachineBasicBlock.cpp:1191
@@ +1190,3 @@
+                                           BranchProbability Prob) {
+  assert(!Prob.isUnknown());
+  if (Probs.empty())
----------------
my question is addSuccessor interface will allocate Prob vector if Prob is known, but the behavior here is different.


http://reviews.llvm.org/D13908