Re: [Qemu-devel] [PATCH v3] bitops: unify bitops_ffsl with the one in host-utils.h, call it bitops_ctzl

[Paolo Bonzini <pbonzini@redhat.com>]

Il 02/02/2013 16:11, Eric Blake ha scritto:
> On 02/02/2013 06:30 AM, Peter Maydell wrote:
> 
>>> - * Undefined if no bit exists, so code should check against 0 first.
>>> + * Returns -1 if no bit exists.  Note that compared to the C library
>>> + * routine ffsl, this one returns one less.
>>>   */
>>
>> Do any of our callers actually use the "-1 on 0 input" semantics?
>> (I guess that means "your new code you added" since the previous
>> callers all were happy with the undefined-on-zero semantics).
> 
> Yes, Paolo's code was replacing:
> 
> ffsl(var) - 1
> 
> with
> 
> bitops_ctzl(var)
> 
> where var==0 was a definite possibility, so we DO have code that depends
> on this semantic of returning -1.

Actually I'm pretty sure that var == 0 is not possible in hbitmap.  But
I still prefer having total functions, and also keeping the function
monotonic.

For example, I would use the number of bits in word for clz(0), since
clz(x) is monotonic decreasing.

Paolo

>> It seems an odd choice, since I can see a justification for:
>>  (a) "return number of bits in word" [every bit in the word is
>>      a trailing zero in some sense]
>>  (b) "undefined" [matches gcc builtin_ctz semantics]
> 
> For all non-zero values of var, ffsl(var) == bitops_ctzl(var)+1.
> Extending the equivalency for var==0 makes the function usable in the
> most places.
>