Hi,

On 4/17/19 9:24 PM, Stefano Stabellini wrote:
On Wed, 27 Mar 2019, Julien Grall wrote:
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.gr...@arm.com>
---
   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.

I agree with Andrew: I can see that the code is correct, but it really
looks wrong at first sight. Why not use unsigned long? This is an arm64
header anyway.

While this is implemented in arm64, this is a function used in common code... So the prototype would have to be changed everywhere.

However, as I pointed out this is a design decision from Arm64 and I think it would be wrong to return unsigned long.

I offered the suggestion to update the commit message. I can also add a comment in the code.

Cheers,

--
Julien Grall

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

Reply via email to