Re: [Minios-devel] [UNIKRAFT PATCH v4 6/7] arch/arm64: Implement bitops for arm64

[Santiago Pagani <Santiago.Pagani@xxxxxxxxx>]

Hi Justin,

Thanks for the patch. Please have a look at my comments inline.

Best,
Santiago

On 09.10.19, 15:30, "Jia He" <justin.he@xxxxxxx> wrote:

    Without this patch, compiler will report warning as follows:
      In file included from plat/common/arm/time.c:41:0:
    include/uk/bitops.h: In function 'uk_get_count_order':
    include/uk/bitops.h:89:10: warning: implicit declaration of function
    'ukarch_fls'; did you mean 'ukarch_ffsl'? [-Wimplicit-function-declaration]
      order = ukarch_fls(count);
              ^~~~~~~~~~
              ukarch_ffsl
    include/uk/bitops.h: In function 'uk_find_last_bit':
    include/uk/bitops.h:154:18: warning: implicit declaration of function
    'ukarch_flsl'; did you mean 'ukarch_ffsl'? [-Wimplicit-function-declaration]
        return (bit + ukarch_flsl(mask));
                      ^~~~~~~~~~~
                      ukarch_ffsl
    
    Signed-off-by: Jia He <justin.he@xxxxxxx>
    ---
     arch/arm/arm64/include/uk/asm/atomic.h | 34 ++++++++++++++++++++++++++
     1 file changed, 34 insertions(+)
    
    diff --git a/arch/arm/arm64/include/uk/asm/atomic.h 
b/arch/arm/arm64/include/uk/asm/atomic.h
    index 7ee6667..fb7d3bc 100644
    --- a/arch/arm/arm64/include/uk/asm/atomic.h
    +++ b/arch/arm/arm64/include/uk/asm/atomic.h
    @@ -37,6 +37,29 @@
     #error Do not include this header directly
     #endif
     
    +/**
    + * ukarch_ffs - find first (lowest) set bit in word.
    + * @word: The word to search
    + *
    + * Returns one plus the index of the least significant 1-bit of x, or
    + * if x is zero, returns zero.
    + */
    +static inline unsigned int ukarch_ffs(unsigned int x)
    +{
    +   return __builtin_ffs(x);
    +}
    +
    +/**
    + * ukarch_fls - find last (highest) set bit in word.
    + * @word: The word to search
    + *
    + * Undefined if no bit exists, so code should check against 0 first.
    + */
    +static inline unsigned int ukarch_fls(unsigned int x)
    +{
    +   return sizeof(x) * 8 - __builtin_clz(x);
For this function and ukarch_flsl, have you tested this? It sounds to me that 
you are missing a "-1". That is, it should be " return sizeof(x) * 8 - 1 - 
__builtin_clz(x)"
Otherwise, in case __builtin_clz(x) returns 0, then the function would return 
32 instead of 31.

    +}
    +
     /**
      * ukarch_ffsl - find first (lowest) set bit in word.
      * @word: The word to search
    @@ -68,3 +91,14 @@ static inline unsigned long ukarch_ffsl(unsigned long 
word)
     
        return 63 - clz;
     }
    +
    +/**
    + * ukarch_flsl - find last (highest) set bit in word.
    + * @word: The word to search
    + *
    + * Undefined if no bit exists, so code should check against 0 first.
    + */
    +static inline unsigned long ukarch_flsl(unsigned long x)
    +{
    +   return sizeof(x) * 8 - __builtin_clzl(x);
    +}
    -- 
    2.17.1
    
    

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel