[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 4/4] bitmaps/bitops: Clarify tests for small constant size.



>>> On 27.02.14 at 17:38, Tim Deegan <tim@xxxxxxx> wrote:
> From dba941064825d723b79f866d16bb0f07585de320 Mon Sep 17 00:00:00 2001
> From: Tim Deegan <tim@xxxxxxx>
> Date: Thu, 28 Nov 2013 15:40:48 +0000
> Subject: [PATCH] bitmaps/bitops: Clarify tests for small constant size.
> 
> No semantic changes, just makes the control flow a bit clearer.
> 
> I was looking at this bcause the (-!__builtin_constant_p(x) | x__)
> formula is too clever for Coverity, but in fact it always takes me a
> minute or two to understand it too. :)
> 
> Signed-off-by: Tim Deegan <tim@xxxxxxx>

A little reluctantly
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

> ---
> 
> v3: Consistenly use '!foo' rather than 'foo == 0'
> v2: fix find_next_bit macros to evaluate 'addr' exactly once.
> ---
>  xen/include/asm-x86/bitops.h | 62 
> ++++++++++++++++++++------------------------
>  xen/include/xen/bitmap.h     | 30 ++++++++++++---------
>  2 files changed, 46 insertions(+), 46 deletions(-)
> 
> diff --git a/xen/include/asm-x86/bitops.h b/xen/include/asm-x86/bitops.h
> index ab21d92..82a08ee 100644
> --- a/xen/include/asm-x86/bitops.h
> +++ b/xen/include/asm-x86/bitops.h
> @@ -335,23 +335,20 @@ static inline unsigned int __scanbit(unsigned long val, 
> unsigned long max)
>   * @offset: The bitnumber to start searching at
>   * @size: The maximum size to search
>   */
> -#define find_next_bit(addr, size, off) ({ \
> -    unsigned int r__ = (size); \
> -    unsigned int o__ = (off); \
> -    switch ( -!__builtin_constant_p(size) | r__ ) \
> -    { \
> -    case 0: (void)(addr); break; \
> -    case 1 ... BITS_PER_LONG: \
> -        r__ = o__ + __scanbit(*(const unsigned long *)(addr) >> o__, r__); \
> -        break; \
> -    default: \
> -        if ( __builtin_constant_p(off) && !o__ ) \
> -            r__ = __find_first_bit(addr, r__); \
> -        else \
> -            r__ = __find_next_bit(addr, r__, o__); \
> -        break; \
> -    } \
> -    r__; \
> +#define find_next_bit(addr, size, off) ({                                   
> \
> +    unsigned int r__;                                                       
> \
> +    const unsigned long *a__ = (addr);                                      
> \
> +    unsigned int s__ = (size);                                              
> \
> +    unsigned int o__ = (off);                                               
> \
> +    if ( __builtin_constant_p(size) && !s__ )                               \
> +        r__ = s__;                                                          
> \
> +    else if ( __builtin_constant_p(size) && s__ <= BITS_PER_LONG )          \
> +        r__ = o__ + __scanbit(*(const unsigned long *)(a__) >> o__, s__);   \
> +    else if ( __builtin_constant_p(off) && !o__ )                           \
> +        r__ = __find_first_bit(a__, s__);                                   
> \
> +    else                                                                    
> \
> +        r__ = __find_next_bit(a__, s__, o__);                               
> \
> +    r__;                                                                    
> \
>  })
>  
>  /**
> @@ -370,23 +367,20 @@ static inline unsigned int __scanbit(unsigned long val, 
> unsigned long max)
>   * @offset: The bitnumber to start searching at
>   * @size: The maximum size to search
>   */
> -#define find_next_zero_bit(addr, size, off) ({ \
> -    unsigned int r__ = (size); \
> -    unsigned int o__ = (off); \
> -    switch ( -!__builtin_constant_p(size) | r__ ) \
> -    { \
> -    case 0: (void)(addr); break; \
> -    case 1 ... BITS_PER_LONG: \
> -        r__ = o__ + __scanbit(~*(const unsigned long *)(addr) >> o__, r__); \
> -        break; \
> -    default: \
> -        if ( __builtin_constant_p(off) && !o__ ) \
> -            r__ = __find_first_zero_bit(addr, r__); \
> -        else \
> -            r__ = __find_next_zero_bit(addr, r__, o__); \
> -        break; \
> -    } \
> -    r__; \
> +#define find_next_zero_bit(addr, size, off) ({                              
> \
> +    unsigned int r__;                                                       
> \
> +    const unsigned long *a__ = (addr);                                      
> \
> +    unsigned int s__ = (size);                                              
> \
> +    unsigned int o__ = (off);                                               
> \
> +    if ( __builtin_constant_p(size) && !s__ )                               \
> +        r__ = s__;                                                          
> \
> +    else if ( __builtin_constant_p(size) && s__ <= BITS_PER_LONG )          \
> +        r__ = o__ + __scanbit(~*(const unsigned long *)(a__) >> o__, s__);  \
> +    else if ( __builtin_constant_p(off) && !o__ )                           \
> +        r__ = __find_first_zero_bit(a__, s__);                              
> \
> +    else                                                                    
> \
> +        r__ = __find_next_zero_bit(a__, s__, o__);                          
> \
> +    r__;                                                                    
> \
>  })
>  
>  /**
> diff --git a/xen/include/xen/bitmap.h b/xen/include/xen/bitmap.h
> index b5ec455..e2a3686 100644
> --- a/xen/include/xen/bitmap.h
> +++ b/xen/include/xen/bitmap.h
> @@ -110,13 +110,14 @@ extern int bitmap_allocate_region(unsigned long 
> *bitmap, int pos, int order);
>  
>  #define bitmap_bytes(nbits) (BITS_TO_LONGS(nbits) * sizeof(unsigned long))
>  
> -#define bitmap_switch(nbits, zero_ret, small, large)                 \
> -     switch (-!__builtin_constant_p(nbits) | (nbits)) {              \
> -     case 0: return zero_ret;                                        \
> -     case 1 ... BITS_PER_LONG:                                       \
> -             small; break;                                           \
> -     default:                                                        \
> -             large; break;                                           \
> +#define bitmap_switch(nbits, zero, small, large)                       \
> +     unsigned int n__ = (nbits);                                       \
> +     if (__builtin_constant_p(nbits) && !n__) {                        \
> +             zero;                                                     \
> +     } else if (__builtin_constant_p(nbits) && n__ <= BITS_PER_LONG) { \
> +             small;                                                    \
> +     } else {                                                          \
> +             large;                                                    \
>       }
>  
>  static inline void bitmap_zero(unsigned long *dst, int nbits)
> @@ -191,7 +192,8 @@ static inline void bitmap_complement(unsigned long *dst, 
> const unsigned long *sr
>  static inline int bitmap_equal(const unsigned long *src1,
>                       const unsigned long *src2, int nbits)
>  {
> -     bitmap_switch(nbits, -1,
> +     bitmap_switch(nbits,
> +             return -1,
>               return !((*src1 ^ *src2) & BITMAP_LAST_WORD_MASK(nbits)),
>               return __bitmap_equal(src1, src2, nbits));
>  }
> @@ -199,7 +201,8 @@ static inline int bitmap_equal(const unsigned long *src1,
>  static inline int bitmap_intersects(const unsigned long *src1,
>                       const unsigned long *src2, int nbits)
>  {
> -     bitmap_switch(nbits, -1,
> +     bitmap_switch(nbits,
> +             return -1,
>               return ((*src1 & *src2) & BITMAP_LAST_WORD_MASK(nbits)) != 0,
>               return __bitmap_intersects(src1, src2, nbits));
>  }
> @@ -207,21 +210,24 @@ static inline int bitmap_intersects(const unsigned long 
> *src1,
>  static inline int bitmap_subset(const unsigned long *src1,
>                       const unsigned long *src2, int nbits)
>  {
> -     bitmap_switch(nbits, -1,
> +     bitmap_switch(nbits,
> +             return -1,
>               return !((*src1 & ~*src2) & BITMAP_LAST_WORD_MASK(nbits)),
>               return __bitmap_subset(src1, src2, nbits));
>  }
>  
>  static inline int bitmap_empty(const unsigned long *src, int nbits)
>  {
> -     bitmap_switch(nbits, -1,
> +     bitmap_switch(nbits,
> +             return -1,
>               return !(*src & BITMAP_LAST_WORD_MASK(nbits)),
>               return __bitmap_empty(src, nbits));
>  }
>  
>  static inline int bitmap_full(const unsigned long *src, int nbits)
>  {
> -     bitmap_switch(nbits, -1,
> +     bitmap_switch(nbits,
> +             return -1,
>               return !(~*src & BITMAP_LAST_WORD_MASK(nbits)),
>               return __bitmap_full(src, nbits));
>  }
> -- 
> 1.8.5.2



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.