On 02/23/2015 10:40 AM, Markus Armbruster wrote:
>>> int64_t pow2floor(int64_t value)
>>> {
>>> assert(value > 0);
>>> return 0x8000000000000000u >> clz64(value);
>>> }
>>
>> Needs to be 0x8000000000000000ull for 32-bit machines to compile correctly.
>
> Why?
Because 0x8000000000000000u is only required to be a 'long', and on
32-bit machines, your constant would overflow long. See, for example,
commit 5cb6be2ca. You NEED the 'll' suffix to ensure that the compiler
doesn't reject the constant as an overflow.
>
>> Why is the parameter int64_t? Wouldn't it be more useful to have:
>>
>> uint64_t pow2floor(uint64_t value)
>
> Crossed my mind, too. However, the existing callers pass *signed*
> arguments.
I guess it means auditing callers either way.
>
>>> int64_t pow2ceil(int64_t value)
>>> {
>>
>> Again, why allow signed inputs?
>>
>>> assert(value <= 0x4000000000000000)
>>> if (value <= 1)
>>> return 1;
>>
>> In particular, this slams all negative values to a result of 1, which
>> doesn't necessarily make sense.
>
> It implements a straightforward contract: return the smallest power of
> two greater or equal to the argument. The function's domain is the set
> of int64_t arguments where this value can be represented in int64_t,
> i.e. [-2^63..2^62].
>
> Feel free to suggest a more sensible contract.
But why should I claim that the nearest power of 2 greater than -3 is
positive 1, when I could argue that it should instead be -2 (nearest
positive or negative power of 2 rounding towards +inf) or -4 (nearest
positive or negative power of 2 rounding away from 0)? Since there are
multiple potential contracts once negative values are involved, I find
it easier to just make the contract require a positive input to begin with.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
