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

Chandler Carruth chandlerc at google.com
Tue Dec 9 10:23:14 PST 2014


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

> Branch weights are only 32bit (based on createBranchWeights), so the
> interface of the extract function is misleading by suggesting they are
> 64bit.
>

OK, this is the kind of thing that really needs to be clarified in the
interface of any function here. And/or in the comments doing the math.


>
> The code for updating the branch weight is more or less verbatim from
> SelectionDAGBuilder. We will never be able to recover the original branch
> weights, because there is not enough information to do so. This is mostly a
> guestimate what the original weights could have been.
>

Ok, I don't recall the code going in, but just because we have poorly
commented or factored code already in the tree shouldn't stop us from doing
better when we're moving it around and/or enhancing it.

I think it would make a lot of sense to pull this into some utility (maybe
there is even a good interface for putting it onto BranchInst) for updating
metadata when splitting branches in this way. And then you can fix both the
code duplication and have potentially a better place to make these kinds of
comments explaining why the 64-bit integer math is safe (we start with
32-bits, do some math, and then scale back down).
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141209/4472a49a/attachment.html>


More information about the llvm-commits mailing list