[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 Dec 26, 2011, at 8:31 PM, Nick Lewycky wrote:
>
> Hi Nick, sorry for the late comments. I missed this the first time.
>
>> +      APInt GCD = APIntOps::GreatestCommonDivisor(ProbTrue, ProbFalse);
>> +      ProbTrue = ProbTrue.udiv(GCD);
>> +      ProbFalse = ProbFalse.udiv(GCD);
>
> This causes pretty weird behavior if you think about it.
> Future weight computations are more likely to overflow if weights are co-prime?
>
>> +
>> +      if (Overflow1 || Overflow2 || Overflow3 || Overflow4 || Overflow5 ||
>> +          Overflow6) {
>> +        DEBUG(dbgs()<<  "Overflow recomputing branch weight on: "<<  *PBI
>> +<<  "when merging with: "<<  *BI);
>> +        PBI->setMetadata(LLVMContext::MD_prof, NULL);
>
> Chicken. ;-)
>
> Please deal with overflow correctly, and scale the new weights to fit in 32 bits. Then you don't need the GCD stuff either.

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.

So, I've tried to implement this and ended up with the patch attached. 
Any suggestions for improvement would be appreciated.

Nick

> Branch weights could be actual execution counts, and we don't require them to be scaled down to small integers. The full 32-bit range should be well supported.
>
> I was sure Jakub had written code just like this, but I can't find it now.
>
> /jakob
>
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: x.patch
Type: text/x-patch
Size: 4008 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120122/b65f4597/attachment.bin>