[PATCH] D30633: [BPI] Reduce the probability of unreachable edge to minimal value greater than 0

[Serguei Katkov via Phabricator via llvm-commits]

skatkov added a comment.

David, thank you very much for pointing me to place where the weight is bumped in BFI. Yes, it is really bumped for branch frequency computation but branch probability is not updated that is why weight 0 helps me for block placement but 1 is not (1 - helps to reduce the probability but does not help in common case).

Let me just explain one more time where the problem is.
Consider the test basing on https://bugs.llvm.org/show_bug.cgi?id=32214.
We have a loop
LoopHeader -> (UnreachableExit, Middleblock)
Middleblock -> (ColdPath, Backedge)
ColdPath ->Backedge
Backedge -> (NormalExit, LoopHeader)

MiddleBlock and ColdPath are only needed for triggering loop rotation.
The probability of the edge LoopHeader -> UnreachableExit is set to 1/2^20 without this patch and 1/2^31 within this patch.
Let's probability of edge Backedge -> NormalExit is obtained by real profile date and equal to 1/2^31.

MachineBlockPlacement pass tries to do a loop rotation. It will build a loop chain: LoopHeader->MiddleBlock->BackEdge->ColdPath.
To do a loop rotation it needs to find the best exit. It does it using the following (it reality it is a bit more complex but for explanation it is enough):
BlockFrequency ExitEdgeFreq = MBFI->getBlockFreq(MBB) * SuccProb;
There are two exits:
LoopHeader -> UnreachableExit
Backedge -> NormalExit

The frequency of LoopHeader is greater than frequency of Backedge due to mass comes out to UnreachableBlock (even f it's probability is zero due to bump of the weight in BFI).
So without this patch SuccProb for UnreachableExit is equal to 1/2^20 and chances the ExitEdgeFreq will be greater for UnreachableExit than for NormalExit more.
Within this patch the worst case is when probability of edge Backedge -> NormalExit is the lowest one (1/2^31).
In this case SuccProb will be the same for both exits. But taking into account that frequency of LoopHeader is greater than for Backedge the ExitEdgeFreq will be greater for UnreachableExit anyway.
However when SuccProb for UnreachableExit is equal to zero the NormalExit becomes the best exit.
That is why for corner case the zero unreachable probability helps to fix the loop rotation issue.

So, this patch makes sense anyway is due to it reduces the number of cases where loop rotation works incorrectly while for the corner cases Zero probability is required.

Any opinions? May be I missed anything?


https://reviews.llvm.org/D30633