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

[Xinliang David Li via llvm-commits]

Serguei, thanks for the explanation. I have no issue with your patch.

David

On Wed, May 17, 2017 at 2:07 AM, Serguei Katkov via Phabricator <
reviews at reviews.llvm.org> wrote:

> 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
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170517/2b8c1c2a/attachment.html>