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

[Xinliang David Li via llvm-commits]

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?


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
>