[PATCH] D12603: Use fixed-point representation for BranchProbability

[Xinliang David Li via llvm-commits]

On Thu, Sep 10, 2015 at 2:15 PM, Xinliang David Li <davidxl at google.com> wrote:
> Cong,  It is more common to use power of 2 as the scaling factor in
> fixed point representation for computation efficiency.  For 32bit
> width, 0x80000000 is common (noted as Q31).  Here are the benefits
>
> 1) it is much faster to do the scale operation with Q31 -- instead of
> using the current slow version of scale, it can call a faster version
> to be introduced.
>
> This will probably help resolve bug
> https://llvm.org/bugs/show_bug.cgi?id=24620 where 20% of the time is
> spent in the slow version of the scale method.
>
> 2) it allows negative fractional number to be represented
>
> 3) it is more precise for lots of common cases. For instance,
> representation of 0.5, 0.25, 0.75 (i.e. 1/2, 1/4, 3/4) etc are
> actually lossless with Q31 while the current choice of using
> uint32_max can not represent them precisely.
>
> (if needed in the future, we can also extend this to be a template
> class where number of bits in the scaling factor is a template
> parameter to support values > 1
>    template <int bits> class FixedPoint {
>      static const int ScalingFactor = 1 << bits;
>  };
> )
>
> In the IR dump, I find it is easier to read
> 1) percentage,
> 2) M/N form when N is less than 10.
>
> Current dump of 0x10ffff/0xffffffff form is hard to read -- can then
> be simplified to make the denominator < 10?

More precisely, if there is no precision loss, make the denominator a
power of 2 value <= 16. Otherwise, make the max denominator to be
limited to <=64 or 128 with some precision loss.

David


>
>
> Duncan, what is your opinion?
>
> thanks,
>
> David
>
>
>
>
>
> On Thu, Sep 10, 2015 at 12:21 PM, Cong Hou <congh at google.com> wrote:
>> congh updated this revision to Diff 34474.
>> congh added a comment.
>>
>> Update the patch by fixing block frequency / branch probability unit tests failures.
>>
>> I am not sure if this is the right way to update the unit tests. The fixed-point representation has precision issues comparing to the previous rational representation and that is why we need to update the comparison right answers in unit tests.
>>
>>
>> http://reviews.llvm.org/D12603
>>
>> Files:
>>   include/llvm/Analysis/BlockFrequencyInfoImpl.h
>>   include/llvm/Support/BranchProbability.h
>>   lib/Support/BranchProbability.cpp
>>   lib/Target/ARM/ARMBaseInstrInfo.cpp
>>   test/Analysis/BlockFrequencyInfo/basic.ll
>>   test/Analysis/BlockFrequencyInfo/loops_with_profile_info.ll
>>   test/Analysis/BranchProbabilityInfo/basic.ll
>>   test/Analysis/BranchProbabilityInfo/loop.ll
>>   test/Analysis/BranchProbabilityInfo/noreturn.ll
>>   test/Analysis/BranchProbabilityInfo/pr18705.ll
>>   test/Analysis/BranchProbabilityInfo/pr22718.ll
>>   test/CodeGen/AArch64/fast-isel-branch-cond-split.ll
>>   test/CodeGen/ARM/2013-10-11-select-stalls.ll
>>   test/CodeGen/ARM/ifcvt4.ll
>>   test/CodeGen/ARM/sjlj-prepare-critical-edge.ll
>>   test/CodeGen/ARM/tail-merge-branch-weight.ll
>>   test/CodeGen/ARM/test-sharedidx.ll
>>   test/CodeGen/Thumb2/thumb2-ifcvt1.ll
>>   test/Transforms/SampleProfile/branch.ll
>>   test/Transforms/SampleProfile/calls.ll
>>   test/Transforms/SampleProfile/discriminator.ll
>>   test/Transforms/SampleProfile/fnptr.ll
>>   test/Transforms/SampleProfile/propagate.ll
>>   unittests/Support/BlockFrequencyTest.cpp
>>   unittests/Support/BranchProbabilityTest.cpp
>>