<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Dec 9, 2014 at 7:18 PM, Juergen Ributzka <span dir="ltr"><<a href="mailto:juergen@apple.com" target="_blank">juergen@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>Branch weights are only 32bit (based on createBranchWeights), so the interface of the extract function is misleading by suggesting they are 64bit.</div></blockquote><div><br></div><div>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.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><br></div><div>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.</div></blockquote></div><br>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.</div><div class="gmail_extra"><br></div><div class="gmail_extra">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).</div></div>