bitops: unify bitops_ffsl with the one in host-utils.h, call it bitops_ctzl
authorPaolo Bonzini <pbonzini@redhat.com>
Fri, 1 Feb 2013 22:03:16 +0000 (23:03 +0100)
committerBlue Swirl <blauwirbel@gmail.com>
Sat, 2 Feb 2013 20:16:00 +0000 (20:16 +0000)
We had two copies of a ffs function for longs with subtly different
semantics and, for the one in bitops.h, a confusing name: the result
was off-by-one compared to the library function ffsl.

Unify the functions into one, and solve the name problem by calling
the 0-based functions "bitops_ctzl" and "bitops_ctol" respectively.

This also fixes the build on platforms with ffsl, including Mac OS X
and Windows.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Tested-by: Andreas Färber <afaerber@suse.de>
Tested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
include/qemu/bitops.h
include/qemu/hbitmap.h
include/qemu/host-utils.h
memory.c
util/bitops.c
util/hbitmap.c

index 74e14e57247e1bef75a68227e25d4f9550ab313e..8b8879186218e008c5fa74fe7ce2224d80899e93 100644 (file)
@@ -13,6 +13,7 @@
 #define BITOPS_H
 
 #include "qemu-common.h"
+#include "host-utils.h"
 
 #define BITS_PER_BYTE           CHAR_BIT
 #define BITS_PER_LONG           (sizeof (unsigned long) * BITS_PER_BYTE)
 #define BITS_TO_LONGS(nr)      DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
 
 /**
- * bitops_ffs - find first bit in word.
+ * bitops_ctzl - count trailing zeroes in word.
  * @word: The word to search
  *
- * Undefined if no bit exists, so code should check against 0 first.
+ * Returns -1 if no bit exists.  Note that compared to the C library
+ * routine ffsl, this one returns one less.
  */
-static unsigned long bitops_ffsl(unsigned long word)
+static unsigned long bitops_ctzl(unsigned long word)
 {
-       int num = 0;
+#if QEMU_GNUC_PREREQ(3, 4)
+    return __builtin_ffsl(word) - 1;
+#else
+    if (!word) {
+        return -1;
+    }
 
-#if LONG_MAX > 0x7FFFFFFF
-       if ((word & 0xffffffff) == 0) {
-               num += 32;
-               word >>= 32;
-       }
+    if (sizeof(long) == 4) {
+        return ctz32(word);
+    } else if (sizeof(long) == 8) {
+        return ctz64(word);
+    } else {
+        abort();
+    }
 #endif
-       if ((word & 0xffff) == 0) {
-               num += 16;
-               word >>= 16;
-       }
-       if ((word & 0xff) == 0) {
-               num += 8;
-               word >>= 8;
-       }
-       if ((word & 0xf) == 0) {
-               num += 4;
-               word >>= 4;
-       }
-       if ((word & 0x3) == 0) {
-               num += 2;
-               word >>= 2;
-       }
-       if ((word & 0x1) == 0) {
-               num += 1;
-        }
-       return num;
 }
 
 /**
@@ -99,14 +88,14 @@ static inline unsigned long bitops_flsl(unsigned long word)
 }
 
 /**
- * ffz - find first zero in word.
+ * cto - count trailing ones in word.
  * @word: The word to search
  *
- * Undefined if no zero exists, so code should check against ~0UL first.
+ * Returns -1 if all bit are set.
  */
-static inline unsigned long ffz(unsigned long word)
+static inline unsigned long bitops_ctol(unsigned long word)
 {
-    return bitops_ffsl(~word);
+    return bitops_ctzl(~word);
 }
 
 /**
index 73f5d1d8d3e68386e14ead9411f635ee1127dfaf..250de03b03db3bd0970ad2bc6a2f695b0354ed39 100644 (file)
@@ -170,7 +170,7 @@ static inline int64_t hbitmap_iter_next(HBitmapIter *hbi)
 
     /* The next call will resume work from the next bit.  */
     hbi->cur[HBITMAP_LEVELS - 1] = cur & (cur - 1);
-    item = ((uint64_t)hbi->pos << BITS_PER_LEVEL) + ffsl(cur) - 1;
+    item = ((uint64_t)hbi->pos << BITS_PER_LEVEL) + bitops_ctzl(cur);
 
     return item << hbi->granularity;
 }
index 2a32be4cc06b03b32879c3edc056ce39ddc6f107..81c9a754ae2db2af6c8a70b4ebe08ff697b2dd4c 100644 (file)
@@ -26,7 +26,6 @@
 #define HOST_UTILS_H 1
 
 #include "qemu/compiler.h"   /* QEMU_GNUC_PREREQ */
-#include <string.h>     /* ffsl */
 
 #if defined(__x86_64__)
 #define __HAVE_FAST_MULU64__
@@ -238,29 +237,4 @@ static inline int ctpop64(uint64_t val)
 #endif
 }
 
-/* glibc does not provide an inline version of ffsl, so always define
- * ours.  We need to give it a different name, however.
- */
-#ifdef __GLIBC__
-#define ffsl qemu_ffsl
-#endif
-static inline int ffsl(long val)
-{
-    if (!val) {
-        return 0;
-    }
-
-#if QEMU_GNUC_PREREQ(3, 4)
-    return __builtin_ctzl(val) + 1;
-#else
-    if (sizeof(long) == 4) {
-        return ctz32(val) + 1;
-    } else if (sizeof(long) == 8) {
-        return ctz64(val) + 1;
-    } else {
-        abort();
-    }
-#endif
-}
-
 #endif
index 410c5f80b420f79c00910a462a6691d0dc62683a..cd7d5e0cf5f9486cc6f004363efc33617cd2add2 100644 (file)
--- a/memory.c
+++ b/memory.c
@@ -855,7 +855,7 @@ static uint64_t memory_region_dispatch_read1(MemoryRegion *mr,
     }
 
     if (!mr->ops->read) {
-        return mr->ops->old_mmio.read[bitops_ffsl(size)](mr->opaque, addr);
+        return mr->ops->old_mmio.read[bitops_ctzl(size)](mr->opaque, addr);
     }
 
     /* FIXME: support unaligned access */
@@ -908,7 +908,7 @@ static void memory_region_dispatch_write(MemoryRegion *mr,
     adjust_endianness(mr, &data, size);
 
     if (!mr->ops->write) {
-        mr->ops->old_mmio.write[bitops_ffsl(size)](mr->opaque, addr, data);
+        mr->ops->old_mmio.write[bitops_ctzl(size)](mr->opaque, addr, data);
         return;
     }
 
index 4c3a836a014a615a516253c4bbfde5bed51758f7..7b853cf944b327644b7803d935a5d32a75d6d24c 100644 (file)
@@ -60,7 +60,7 @@ found_first:
         return result + size;  /* Nope. */
     }
 found_middle:
-    return result + bitops_ffsl(tmp);
+    return result + bitops_ctzl(tmp);
 }
 
 /*
@@ -109,7 +109,7 @@ found_first:
         return result + size;  /* Nope. */
     }
 found_middle:
-    return result + ffz(tmp);
+    return result + bitops_ctol(tmp);
 }
 
 unsigned long find_last_bit(const unsigned long *addr, unsigned long size)
index 2aa487db74b0b817fd54911c31933c7ce3a4807e..a0df5d35915320c4fe8f0929b60f2a581a008f3d 100644 (file)
@@ -126,7 +126,7 @@ unsigned long hbitmap_iter_skip_words(HBitmapIter *hbi)
          * The index of this word's least significant set bit provides
          * the low-order bits.
          */
-        pos = (pos << BITS_PER_LEVEL) + ffsl(cur) - 1;
+        pos = (pos << BITS_PER_LEVEL) + bitops_ctzl(cur);
         hbi->cur[i] = cur & (cur - 1);
 
         /* Set up next level for iteration.  */