[llvm-commits] [llvm] r147286 - in /llvm/trunk: lib/Transforms/Utils/SimplifyCFG.cpp test/Transforms/SimplifyCFG/preserve-branchweights.ll

[Nick Lewycky]

Jakob Stoklund Olesen wrote:
>
> On Jan 22, 2012, at 8:25 PM, Nick Lewycky wrote:
>
>> So, I've tried to implement this and ended up with the patch attached. Any suggestions for improvement would be appreciated.
>
> Thanks, Nick. The patch looks good to me.

Uh... patch looks terrible to me, but sure okay, I'll commit it. :)

> BTW, are you actually benefiting from computing everything in APInt? We know the ranges of these values.

Ah, hrm. You just pointed out a bug in my patch, it's possible for me to 
produce two values whose sums don't fit in 32-bits.

As for APInt, it doesn't really matter.

>> You still need the GCD stuff, otherwise you could end up with branch weights 'i32 5', 'i32 5'. That might be legal, but it's certainly not optimal.
>
> I don't think it actually hurts, but you get some really weird rounding behavior based on number theoretic happenstance.
>
> These weights really should have been floats, but we had to approximate with ints because we need reproducible results across platforms. They are approximations of real numbers, they are not intended as 'mathematical integers'.
>
> Let me put it this way: If the weights were floats, would you be computing the GCD of the mantissas?

I don't care about taking the GCD for math reasons, I care about not 
storing different representations of the same data in the .bc files. 
Incidentally it'll also decrease the chance of the next user needing to 
deal with overflow in a slow-path, and that's good too, but not why I 
wrote it.

Nick