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

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: lib/Analysis/BranchProbabilityInfo.cpp:143
@@ -149,9 +142,3 @@
 
-  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());
----------------
Can we unify the code a little more:

BranchProbability UnreachableProb =
   ( ReachableEdges.empty() ?
           BranchProbability(1, UnreachableEdges.size()) :
           BranchProbability(UR_TAKEN_WEIGHT, (....) * UnreachableEdges.size()));
BranchProbability ReachableProb = (UR_NOTAKEN_WEIGHT, ReachableEdges.size() * ...);

for (...: UnreachableEdges) 
    set...
for (...)
    set ...


================
Comment at: lib/Analysis/BranchProbabilityInfo.cpp:264
@@ -267,9 +263,3 @@
 
-  uint32_t ColdWeight =
-      std::max(CC_TAKEN_WEIGHT / (unsigned) ColdEdges.size(), MIN_WEIGHT);
-  for (SmallVectorImpl<unsigned>::iterator I = ColdEdges.begin(),
-                                           E = ColdEdges.end();
-       I != E; ++I)
-    setEdgeWeight(BB, *I, ColdWeight);
-
-  if (NormalEdges.empty())
+  if (NormalEdges.empty()) {
+    BranchProbability Prob(1, ColdEdges.size());
----------------
Similarly, the code can be simplified without special case..

================
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,
----------------
No need to guard.

================
Comment at: lib/Analysis/BranchProbabilityInfo.cpp:350
@@ +349,3 @@
+                                 LBH_TAKEN_WEIGHT + LBH_NONTAKEN_WEIGHT);
+  if (!InEdges.empty())
+    Probs[1] = BranchProbability(LBH_TAKEN_WEIGHT,
----------------
No need to guard.

================
Comment at: lib/Analysis/BranchProbabilityInfo.cpp:353
@@ +352,3 @@
+                                 LBH_TAKEN_WEIGHT + LBH_NONTAKEN_WEIGHT);
+  if (!ExitingEdges.empty())
+    Probs[2] = BranchProbability(LBH_NONTAKEN_WEIGHT,
----------------
No need to guard.

================
Comment at: lib/Analysis/BranchProbabilityInfo.cpp:361
@@ -353,1 +360,3 @@
+    for (unsigned SuccIdx : BackEdges)
+      setEdgeProbability(BB, SuccIdx, Prob);
   }
----------------
The original code will adjust the weight if it is too low. Do we need to check Prob here too?

================
Comment at: lib/Analysis/BranchProbabilityInfo.cpp:586
@@ -599,5 +585,3 @@
     if (*I == Dst) {
-      MapI = Weights.find(std::make_pair(Src, I.getSuccessorIndex()));
-      if (MapI != Weights.end()) {
-        FoundWeight = true;
-        Weight += MapI->second;
+      auto MapI = Probs.find(std::make_pair(Src, I.getSuccessorIndex()));
+      if (MapI != Probs.end()) {
----------------
Is it better just call getEdgeProbability(Src, *I)


http://reviews.llvm.org/D15519