Re: [Xen-devel] [PATCH 05/12] xen/arm64: bitops: Match the register size with the value size in flsl

[Julien Grall <Julien.Grall@xxxxxxx>]

Hi,

On 27/03/2019 19:03, Andrew Cooper wrote:
> On 27/03/2019 18:45, Julien Grall wrote:
>> Clang is pickier than GCC for the register size in asm statement. It expects
>> the register size to match the value size.
>>
>> The instruction clz is expecting the two operands to be the same size
>> (i.e 32-bit or 64-bit). As the flsl function is dealing with 64-bit
>> value, we need to make the destination variable 64-bit as well.
>>
>> While at it, add a newline before the return statement.
>>
>> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
>> ---
>>   xen/include/asm-arm/arm64/bitops.h | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/include/asm-arm/arm64/bitops.h 
>> b/xen/include/asm-arm/arm64/bitops.h
>> index 6bf1922680..05045f1109 100644
>> --- a/xen/include/asm-arm/arm64/bitops.h
>> +++ b/xen/include/asm-arm/arm64/bitops.h
>> @@ -34,12 +34,13 @@ static /*__*/always_inline unsigned long __ffs(unsigned 
>> long word)
>>   
>>   static inline int flsl(unsigned long x)
>>   {
>> -        int ret;
>> +        uint64_t ret;
>>   
>>           if (__builtin_constant_p(x))
>>                  return generic_flsl(x);
>>   
>>           asm("clz\t%0, %1" : "=r" (ret) : "r" (x));
> 
> As x is fixed by the ABI, ret should be unsigned long to match, surely?

No need for it. The result of the instruction clz will always be smaller 
than 64.

I suspect they require a 64-bit register just for simplicity as they 
encode the size for the 2 registers in only a single bit (i.e sf).

> 
> This will compile as it is arm64 specific, but it looks just as wrong
> (per the commit message) as using int in the first place.
See above. I can clarify it in the commit message.

Cheers,

-- 
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel