Re: [PATCH v9 03/15] xen/bitops: implement fls{l}() in common logic

[Jan Beulich <jbeulich@xxxxxxxx>]

On 17.05.2024 11:06, Oleksii K. wrote:
> On Wed, 2024-05-15 at 11:09 +0200, Jan Beulich wrote:
>> But this then needs carrying through to ...
>>
>>> --- a/xen/arch/arm/include/asm/arm64/bitops.h
>>> +++ b/xen/arch/arm/include/asm/arm64/bitops.h
>>> @@ -22,17 +22,15 @@ static /*__*/always_inline unsigned long
>>> __ffs(unsigned long word)
>>>    */
>>>   #define ffz(x)  __ffs(~(x))
>>>   
>>> -static inline int flsl(unsigned long x)
>>> +static inline int arch_flsl(unsigned long x)
>>
>> ... e.g. here. You don't want to introduce signed/unsigned
>> mismatches.
> Is it critical for x86 to return int for flsl() and fls() or I can
> update the return type for x86 too?

The return types ought to be changed imo, for everything to end up
consistent.

>    static always_inline int arch_flsl(unsigned long x)
>    {
>        long r;
>    
>        asm ( "bsr %1,%0\n\t"
>              "jnz 1f\n\t"
>              "mov $-1,%0\n"
>              "1:" : "=r" (r) : "rm" (x));
>        return (int)r+1;
>    }
>    #define arch_flsl arch_flsl
>    
>    static always_inline int arch_fls(unsigned int x)
>    {
>        int r;
>    
>        asm ( "bsr %1,%0\n\t"
>              "jnz 1f\n\t"
>              "mov $-1,%0\n"
>              "1:" : "=r" (r) : "rm" (x));
>        return r + 1;
>    }
>    #define arch_fls arch_fls
> 
> Any specific reason why 'long' and 'int' types for r are used?

I don't think so. I expect it merely fits with no-one having cared about
signedness back at the time.

Jan