[llvm] r197272 - [block-freq] Add the method APInt::nearestLogBase2().

[Stephen Canon]

On Dec 16, 2013, at 1:39 AM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:

> On 2013 Dec 15, at 20:41, Michael Gottesman <mgottesman at apple.com> wrote:
> 
>> On Dec 15, 2013, at 2:44 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>> 
>>> I thought of another degenerate case, and unfortunately I think the
>>> current code gets the wrong answer:
>>> 
>>> - APInt(1, 1).nearestLogBase2() == 0 // lg(1) should give 0, not 1.
>> 
>> This is incorrect since log2 is not a well defined operation in Z/Z_1. If one defines log2(x) as the largest N such that 2^N < x then since in Z/Z_1, 2^N = 0^N, N could be any number. In reality we should probably add in an assertion to make sure that the routines are only called if the BitWidth is > 1.
> 
> I’m not convinced that lg(1) is undefined; whether 2 itself is
> representable is irrelevant.  Even if I’m wrong, the following
> would be surprising:
> 
>    x.logBase2()        => 0
>    x.ceilLogBase2()    => 0
>    x.nearestLogBase2() => 1 ???
> 
> AFAICT, countLeadingZeros() hits at least two branches before
> calling __builtin_clz, so the micro-optimization won't give
> much benefit in practice anyway.

I’m inclined to agree with Duncan here.  If you really want to get all mathematical about it, we can define exp2 as the unique exponential map on monoids from (Z/Z_N,+) to (Z/Z_2^N,•) that maps 1 —> 2 if it exists, and treat N=1 as a degenerate corner case since there is no 1 in Z/Z_1.  Then let log2 be a step-function extension of the inverse, and call it a day.

Alternatively (or TL;DR mathematical gobbledygook): It is more useful to have log2(1) = 0 regardless of how many bits are used to represent 1.

– Steve