[llvm] r223795 - Revert "Move function to obtain branch weights into the BranchInst class. NFC."

Chandler Carruth chandlerc at google.com
Tue Dec 9 09:46:47 PST 2014


Thanks for moving the interface around, however it seems to highlight why
this is not a good interface even isolated in a single file:

On Tue, Dec 9, 2014 at 6:32 PM, Juergen Ributzka <juergen at apple.com> wrote:

> -      if (Br1->getBranchWeights(TrueWeight, FalseWeight)) {
> +      if (extractBranchMetadata(Br1, TrueWeight, FalseWeight)) {
>          uint64_t NewTrueWeight = TrueWeight;
>          uint64_t NewFalseWeight = TrueWeight + 2 * FalseWeight;
>

What happens if 2 * FalseWeight wraps the uint64_t?

I'm not sure why this pattern of code for updating branch weights is sound.
Maybe it is,  but it isn't at all clear.why. Do we have some guarantee that
these are run through scaleWeights first and so only have 32-bits of active
data in it?

Also, is there some common interface for updating branch weight metadata
that both CGP and SimplifyCFG *could* share, ideally with more of the
scaling logic encapsulated and explained there?


>          scaleWeights(NewTrueWeight, NewFalseWeight);
> @@ -3975,7 +3996,7 @@ bool CodeGenPrepare::splitBranchConditio
>        // assumes that
>        //   FalseProb for BB1 == TrueProb for BB1 * FalseProb for TmpBB.
>        uint64_t TrueWeight, FalseWeight;
> -      if (Br1->getBranchWeights(TrueWeight, FalseWeight)) {
> +      if (extractBranchMetadata(Br1, TrueWeight, FalseWeight)) {
>          uint64_t NewTrueWeight = 2 * TrueWeight + FalseWeight;
>          uint64_t NewFalseWeight = FalseWeight;
>          scaleWeights(NewTrueWeight, NewFalseWeight);
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141209/09c609f2/attachment.html>


More information about the llvm-commits mailing list