llvm dev - Apr 2015 - [LLVMdev] Loss of precision with very large branch weights

On Fri, Apr 24, 2015 at 12:29 PM, Diego Novillo <dnovillo at google.com>
wrote:
>
>
> On Fri, Apr 24, 2015 at 3:28 PM, Xinliang David Li <davidxl at
google.com>
> wrote:
>>
>> yes -- for count representation, 64 bit is needed. The branch weight
>> here is different and does not needs to be 64bit to represent branch
>> probability precisely.
>
>
> Actually, the branch weights are really counts.
No -- I think that is our original proposal (including changing the
meaning of MD_prof meta data) :).  After many rounds of discussions, I
think what we eventually settled to is to
1) use 64 bit value to represent function entry count
2) keep branch weight representation and meaning as it is

Changing weights to 64bit for can slightly increase memory usage. In
fact, what we want longer term is get rid of 'weights' and just use a
fixed point representation for branch probability. For blocks with 2
targets, such info can be attached at Block (source) level, thus
further saving memory.
>They get converted to
> frequencies.  For frequencies, we don't really need 64bits, as
they're just
> comparative values that can be squished into 32bits.  It's the branch
> weights being 32 bit quantities that are throwing off the calculations.
Do you still see the issue after fixing bhe bug (limit without scaling) in
 BranchProbabilityInfo::calcMetadataWeights ?

David
>
>
> Diego.

On Fri, Apr 24, 2015 at 3:44 PM, Xinliang David Li <davidxl at google.com>
wrote:
> On Fri, Apr 24, 2015 at 12:29 PM, Diego Novillo <dnovillo at
google.com>
> wrote:
> >
> >
> > On Fri, Apr 24, 2015 at 3:28 PM, Xinliang David Li <davidxl at
google.com>
> > wrote:
> >>
> >> yes -- for count representation, 64 bit is needed. The branch
weight
> >> here is different and does not needs to be 64bit to represent
branch
> >> probability precisely.
> >
> >
> > Actually, the branch weights are really counts.
>
> No -- I think that is our original proposal (including changing the
> meaning of MD_prof meta data) :).

Sure.  Though they kind of are. They get massaged and smoothed when
branch_weights are placed from the raw counts, but for sufficiently small
values they are very close to counts.

>
> >They get converted to
> > frequencies.  For frequencies, we don't really need 64bits, as
they're
> just
> > comparative values that can be squished into 32bits.  It's the
branch
> > weights being 32 bit quantities that are throwing off the
calculations.
>
> Do you still see the issue after fixing bhe bug (limit without scaling) in
>  BranchProbabilityInfo::calcMetadataWeights ?
>
That's the fix I was contemplating initially. I was curious at whether
moving to 64bit would make this easier.


Diego.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150424/91c73695/attachment.html>

On Fri, Apr 24, 2015 at 1:10 PM, Diego Novillo <dnovillo at google.com>
wrote:
>
>
> On Fri, Apr 24, 2015 at 3:44 PM, Xinliang David Li <davidxl at
google.com>
> wrote:
>>
>> On Fri, Apr 24, 2015 at 12:29 PM, Diego Novillo <dnovillo at
google.com>
>> wrote:
>> >
>> >
>> > On Fri, Apr 24, 2015 at 3:28 PM, Xinliang David Li <davidxl at
google.com>
>> > wrote:
>> >>
>> >> yes -- for count representation, 64 bit is needed. The branch
weight
>> >> here is different and does not needs to be 64bit to represent
branch
>> >> probability precisely.
>> >
>> >
>> > Actually, the branch weights are really counts.
>>
>> No -- I think that is our original proposal (including changing the
>> meaning of MD_prof meta data) :).
>
>
> Sure.  Though they kind of are. They get massaged and smoothed when
> branch_weights are placed from the raw counts, but for sufficiently small
> values they are very close to counts.
right.
>
>>
>>
>> >They get converted to
>> > frequencies.  For frequencies, we don't really need 64bits, as
they're
>> > just
>> > comparative values that can be squished into 32bits.  It's the
branch
>> > weights being 32 bit quantities that are throwing off the
calculations.
>>
>> Do you still see the issue after fixing bhe bug (limit without scaling)
in
>>  BranchProbabilityInfo::calcMetadataWeights ?
>
>
> That's the fix I was contemplating initially. I was curious at whether
> moving to 64bit would make this easier.

It is certainly easier but with a cost :)

david
>
>
> Diego.