[PATCH v2 04/11] xen/bitops: Introduce a multiple_bits_set() helper

Andrew Cooper posted 11 patches 2 months, 3 weeks ago
There is a newer version of this series
[PATCH v2 04/11] xen/bitops: Introduce a multiple_bits_set() helper
Posted by Andrew Cooper 2 months, 3 weeks ago
This will be used to simplify real logic in the following patch.  Add compile
and boot time testing as with other bitops.

Because the expression is so simple, implement it as a function-like macro
which is generic on the type of it's argument, rather than having multiple
variants.

Testing function-like macros needs a minor adjustments to the infrastructure
in xen/self-tests.h to avoid bracketing the fn parameter.  The utility of this
outweighs the associated risks.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Bertrand Marquis <bertrand.marquis@arm.com>
CC: Michal Orzel <michal.orzel@amd.com>
CC: Oleksii Kurochko <oleksii.kurochko@gmail.com>
CC: Shawn Anastasio <sanastasio@raptorengineering.com>

Name inevitably subject to nitpicking.  I'd prefer it to be shorter but I
can't think of anything suitable.

v2:
 * Drop redundant CHECK() in the BITS_PER_LONG > 32 case.
---
 xen/common/bitops.c          | 23 +++++++++++++++++++++++
 xen/include/xen/bitops.h     | 10 ++++++++++
 xen/include/xen/self-tests.h | 10 ++++++++--
 3 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/xen/common/bitops.c b/xen/common/bitops.c
index 9e532f0d87aa..b504dd1308b8 100644
--- a/xen/common/bitops.c
+++ b/xen/common/bitops.c
@@ -112,9 +112,32 @@ static void __init test_for_each_set_bit(void)
         panic("for_each_set_bit(uint64) expected %#"PRIx64", got %#"PRIx64"\n", ull, ull_res);
 }
 
+static void __init test_multiple_bits_set(void)
+{
+    /*
+     * multiple_bits_set() is generic on the type of it's parameter, as the
+     * internal expression is so simple.
+     */
+
+    CHECK(multiple_bits_set, 0, false);
+    CHECK(multiple_bits_set, 1, false);
+    CHECK(multiple_bits_set, 2, false);
+    CHECK(multiple_bits_set, 3, true);
+
+    CHECK(multiple_bits_set, 1 | (1UL << (BITS_PER_LONG - 1)), true);
+#if BITS_PER_LONG > 32
+    CHECK(multiple_bits_set, 1 | (1UL << 32), true);
+#endif
+
+    CHECK(multiple_bits_set, 0x8000000000000001ULL, true);
+    CHECK(multiple_bits_set, 0xc000000000000000ULL, true);
+}
+
 static void __init __constructor test_bitops(void)
 {
     test_ffs();
     test_fls();
     test_for_each_set_bit();
+
+    test_multiple_bits_set();
 }
diff --git a/xen/include/xen/bitops.h b/xen/include/xen/bitops.h
index 9f0a0ce4a73b..e7c2c5598275 100644
--- a/xen/include/xen/bitops.h
+++ b/xen/include/xen/bitops.h
@@ -298,6 +298,16 @@ static always_inline attr_const unsigned int fls64(uint64_t x)
               __v && ((iter) = ffs_g(__v) - 1, true);   \
               __v &= __v - 1 )
 
+/*
+ * Calculate if a value has two or more bits set.  Always use this in
+ * preference to an expression of the form 'hweight(x) > 1'.
+ */
+#define multiple_bits_set(x)                    \
+    ({                                          \
+        typeof(x) _v = (x);                     \
+        (_v & (_v - 1)) != 0;                   \
+    })
+
 /* --------------------- Please tidy below here --------------------- */
 
 #ifndef find_next_bit
diff --git a/xen/include/xen/self-tests.h b/xen/include/xen/self-tests.h
index e9a879489363..bd8a4867aa40 100644
--- a/xen/include/xen/self-tests.h
+++ b/xen/include/xen/self-tests.h
@@ -15,11 +15,14 @@
  *
  * Clang < 8 can't fold constants through static inlines, causing this to
  * fail.  Simply skip it for incredibly old compilers.
+ *
+ * N.B. fn is intentionally not bracketed to allow us to test function-like
+ * macros too.
  */
 #if !defined(CONFIG_CC_IS_CLANG) || CONFIG_CLANG_VERSION >= 80000
 #define COMPILE_CHECK(fn, val, res)                                     \
     do {                                                                \
-        typeof((fn)(val)) real = (fn)(val);                             \
+        typeof(fn(val)) real = fn(val);                                 \
                                                                         \
         if ( !__builtin_constant_p(real) )                              \
             BUILD_ERROR("'" STR(fn(val)) "' not compile-time constant"); \
@@ -34,10 +37,13 @@
  * Check that Xen's runtime logic for fn(val) gives the expected answer.  This
  * requires using HIDE() to prevent the optimiser from collapsing the logic
  * into a constant.
+ *
+ * N.B. fn is intentionally not bracketed to allow us to test function-like
+ * macros too.
  */
 #define RUNTIME_CHECK(fn, val, res)                     \
     do {                                                \
-        typeof((fn)(val)) real = (fn)(HIDE(val));       \
+        typeof(fn(val)) real = fn(HIDE(val));           \
                                                         \
         if ( real != (res) )                            \
             panic("%s: %s(%s) expected %u, got %u\n",   \
-- 
2.39.2