[PATCH] bitmap: simplify GENMASK(size

* [PATCH] bitmap: simplify GENMASK(size - 1, 0) lines
@ 2021-10-26 14:41 Qian Cai
  2021-10-26 15:10 ` Andy Shevchenko
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Qian Cai @ 2021-10-26 14:41 UTC (permalink / raw)
  To: Yury Norov; +Cc: Andy Shevchenko, Rasmus Villemoes, linux-kernel, Qian Cai

Since "size" is an "unsigned int", the rvalue "size - 1" will still be
"unsigned int" according to the C standard (3.2.1.5 Usual arithmetic
conversions). Therefore, GENMASK(size - 1, 0) will always return 0UL. Those
are also caught by GCC (W=2):

./include/linux/find.h: In function 'find_first_bit':
./include/linux/bits.h:25:22: warning: comparison of unsigned expression in '< 0' is always false [-Wtype-limits]
   25 |   __is_constexpr((l) > (h)), (l) > (h), 0)))
      |                      ^
./include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
   16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
      |                                                              ^
./include/linux/bits.h:25:3: note: in expansion of macro '__is_constexpr'
   25 |   __is_constexpr((l) > (h)), (l) > (h), 0)))
      |   ^~~~~~~~~~~~~~
./include/linux/bits.h:38:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
   38 |  (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
      |   ^~~~~~~~~~~~~~~~~~~
./include/linux/find.h:119:31: note: in expansion of macro 'GENMASK'
  119 |   unsigned long val = *addr & GENMASK(size - 1, 0);
      |                               ^~~~~~~
./include/linux/bits.h:25:34: warning: comparison of unsigned expression in '< 0' is always false [-Wtype-limits]
   25 |   __is_constexpr((l) > (h)), (l) > (h), 0)))
      |                                  ^
./include/linux/build_bug.h:16:62: note: in definition of macro 'BUILD_BUG_ON_ZERO'
   16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
      |                                                              ^
./include/linux/bits.h:38:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
   38 |  (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
      |   ^~~~~~~~~~~~~~~~~~~
./include/linux/find.h:119:31: note: in expansion of macro 'GENMASK'
  119 |   unsigned long val = *addr & GENMASK(size - 1, 0);
      |                               ^~~~~~~

Signed-off-by: Qian Cai <quic_qiancai@quicinc.com>
---
 include/linux/find.h | 28 ++++++++--------------------
 1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/include/linux/find.h b/include/linux/find.h
index 5bb6db213bcb..5ce2b17aea42 100644
--- a/include/linux/find.h
+++ b/include/linux/find.h
@@ -115,11 +115,8 @@ unsigned long find_next_zero_bit(const unsigned long *addr, unsigned long size,
 static inline
 unsigned long find_first_bit(const unsigned long *addr, unsigned long size)
 {
-	if (small_const_nbits(size)) {
-		unsigned long val = *addr & GENMASK(size - 1, 0);
-
-		return val ? __ffs(val) : size;
-	}
+	if (small_const_nbits(size))
+		return size;
 
 	return _find_first_bit(addr, size);
 }
@@ -140,11 +137,8 @@ unsigned long find_first_and_bit(const unsigned long *addr1,
 				 const unsigned long *addr2,
 				 unsigned long size)
 {
-	if (small_const_nbits(size)) {
-		unsigned long val = *addr1 & *addr2 & GENMASK(size - 1, 0);
-
-		return val ? __ffs(val) : size;
-	}
+	if (small_const_nbits(size))
+		return size;
 
 	return _find_first_and_bit(addr1, addr2, size);
 }
@@ -162,11 +156,8 @@ unsigned long find_first_and_bit(const unsigned long *addr1,
 static inline
 unsigned long find_first_zero_bit(const unsigned long *addr, unsigned long size)
 {
-	if (small_const_nbits(size)) {
-		unsigned long val = *addr | ~GENMASK(size - 1, 0);
-
-		return val == ~0UL ? size : ffz(val);
-	}
+	if (small_const_nbits(size))
+		return size;
 
 	return _find_first_zero_bit(addr, size);
 }
@@ -183,11 +174,8 @@ unsigned long find_first_zero_bit(const unsigned long *addr, unsigned long size)
 static inline
 unsigned long find_last_bit(const unsigned long *addr, unsigned long size)
 {
-	if (small_const_nbits(size)) {
-		unsigned long val = *addr & GENMASK(size - 1, 0);
-
-		return val ? __fls(val) : size;
-	}
+	if (small_const_nbits(size))
+		return size;
 
 	return _find_last_bit(addr, size);
 }
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread