|
[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.
At 16:30 +0000 on 27 Feb (1393515036), Jan Beulich wrote:
> Using == 0 here, ...
>
> > + 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__ )
> > \
>
> ... but using ! here. I'd prefer the latter everywhere, but I'd also
> be fine with you choosing the former consistently. (Same of course
> again further down.)
Good point. v3 is below
Tim.
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>
---
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
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |