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

Cong Hou via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 21 16:57:58 PST 2015

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.

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.


More information about the llvm-commits mailing list