[PATCH] D79396: [BrachProbablityInfo] Set edge probabilities at once.

[Evgeniy via Phabricator via llvm-commits]

ebrevnov added inline comments.


================
Comment at: llvm/lib/Analysis/BranchProbabilityInfo.cpp:362
+    // unreachable branch fix and we have to distribute the whole difference
+    // from total probability 1.0. This keeps the difference limited.
+    auto ToDistribute = BranchProbability::getOne();
----------------
It's really hard to parse the comment since it heavily depends on old code which will be unavailable. Please try to rephrase. 


================
Comment at: llvm/lib/Analysis/BranchProbabilityInfo.cpp:369
     // the difference between reachable blocks.
-    if (ToDistribute > BranchProbability::getZero()) {
-      BranchProbability PerEdge = ToDistribute / ReachableIdxs.size();
+    BranchProbability PerEdge = ToDistribute / ReachableIdxs.size();
+    if (PerEdge > BranchProbability::getZero())
----------------
yrouban wrote:
> TODO: This spreads //ToDistribute// evenly upon the reachable edges. A better distribution would be proportional. So the relation between weights of the reachable edges would be kept unchanged:
> 
> For any reachable edges i and j:
> newBP[i] / newBP[j] == oldBP[i] / oldBP[j]
> newBP[i] / oldBP[i] == newBP[j] / oldBP[j] == Denominator / (Denominator - ToDistribute)
> newBP[i] = oldBP[i] * Denominator / (Denominator - ToDistribute)
I think it makes sense to put that comment into the code directly


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79396/new/

https://reviews.llvm.org/D79396