This patch removes all weight-related interfaces from BPI and replace them by probability versions. With this patch, we won't use edge weight anymore in either IR or MC passes. Edge probabilitiy is a better representation in terms of CFG update and validation.
Details
Diff Detail
Event Timeline
lib/Analysis/BranchProbabilityInfo.cpp | ||
---|---|---|
143–146 | 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 ... | |
265–268 | Similarly, the code can be simplified without special case.. | |
349 | No need to guard. | |
352 | No need to guard. | |
355 | No need to guard. | |
363 | The original code will adjust the weight if it is too low. Do we need to check Prob here too? | |
588–591 | Is it better just call getEdgeProbability(Src, *I) |
lib/Analysis/BranchProbabilityInfo.cpp | ||
---|---|---|
143–146 | OK. We can set UnreachableProb to 100% when ReachableEdges is empty. | |
265–268 | Done. | |
349 | This is needed to keep the original behavior: when BackEdges is empty, we don't want to normalize probabilities including the BP in this branch. Part of the reason is that we don't have a function to normalize all BB's successors' probabilities (although we have one for MBB). | |
352 | Ditto. | |
355 | Ditto. | |
363 | I think this is unnecessary: the weight can become too small (like zero) after being divided but we don't have this issue in probabilities. | |
588–591 | We need to take care of the case MapI == Probs.end() so we cannot call getEdgeProbability(Src, *I). |
lib/Analysis/BranchProbabilityInfo.cpp | ||
---|---|---|
143–146 | I later found that I have to take care of the non-zero divider case here. To make the code more clear, I think the early exit solution might be better. |
lib/Analysis/BranchProbabilityInfo.cpp | ||
---|---|---|
143–147 | 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. | |
349 | 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 |
lib/Analysis/BranchProbabilityInfo.cpp | ||
---|---|---|
143–147 | OK. | |
349 | 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. |
lib/Analysis/BranchProbabilityInfo.cpp | ||
---|---|---|
349 | Ok, I now see the normalization is done before prob assignment. However I think we can get rid of the normalization completely by computing the right denorm value in the first place. Say the number of back edges is B, number of In edges is I, and number of exiting edges are E, the denorm is D = (B? TAKEN_WEIGHT: 0) + (I ? TAKEN_WEIGHT : 0) + (E ? NON_TAKEN_WEIGHT : 0) So a backedge prob (if exists) is: Prob(backedge) = TAKEN_WEIGHT/(D*B) |
lib/Analysis/BranchProbabilityInfo.cpp | ||
---|---|---|
349 | OK. I have updated the patch accordingly. |
Can we unify the code a little more:
BranchProbability UnreachableProb =
BranchProbability ReachableProb = (UR_NOTAKEN_WEIGHT, ReachableEdges.size() * ...);
for (...: UnreachableEdges)
for (...)