[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.
>
> 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.

Does the sum of the weights need to fit in 32 bits, or just the weights 
themselves?

Nick