[PATCH] D15519: Replace weights by probabilities in BPI.

[Cong Hou via llvm-commits]

congh added inline comments.

================
Comment at: lib/Analysis/BranchProbabilityInfo.cpp:143-146
@@ -149,10 +142,6 @@
 
-  uint32_t UnreachableWeight =
-    std::max(UR_TAKEN_WEIGHT / (unsigned)UnreachableEdges.size(), MIN_WEIGHT);
-  for (SmallVectorImpl<unsigned>::iterator I = UnreachableEdges.begin(),
-                                           E = UnreachableEdges.end();
-       I != E; ++I)
-    setEdgeWeight(BB, *I, UnreachableWeight);
-
-  if (ReachableEdges.empty())
+  if (ReachableEdges.empty()) {
+    BranchProbability Prob(1, UnreachableEdges.size());
+    for (unsigned SuccIdx : UnreachableEdges)
+      setEdgeProbability(BB, SuccIdx, Prob);
     return true;
----------------
davidxl wrote:
> Instead of using division later, why not use the multiplication suggestion in my previous comment? The weight is small enough that there is no risk of overflowing.
OK.

================
Comment at: lib/Analysis/BranchProbabilityInfo.cpp:347
@@ +346,3 @@
+  SmallVector<BranchProbability, 4> Probs(3, BranchProbability::getZero());
+  if (!BackEdges.empty())
+    Probs[0] = BranchProbability(LBH_TAKEN_WEIGHT,
----------------
davidxl wrote:
> Does it matter here? As you can see it is guarded later -- so there is no functional change here. Removing the guard makes code look cleaner
Here is an example: suppose BackEdges is empty and the other two are not. Then the current behavior is that we will have 0, 124/128 and 4/128 in Probs, which will not change after normalization. If we remove those branches , we will have 124/128, 124/128, and 4/128 in Probs, and after normalization they will become roughly 1/2, 1/2, and a very small prob. Note that we won't normalize probabilities that are assigned to successors anymore. Those two lists are quite different and will result in different behaviors.


http://reviews.llvm.org/D15519