[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <8423d75207f64e4081f0019601b4a016@AcuMS.aculab.com>
Date: Sun, 15 Dec 2024 20:01:46 +0000
From: David Laight <David.Laight@...LAB.COM>
To: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Yury Norov
<yury.norov@...il.com>, Rasmus Villemoes <linux@...musvillemoes.dk>
CC: Andrew Morton <akpm@...ux-foundation.org>, Masahiro Yamada
<masahiroy@...nel.org>, 'Vincent Mailhol' <mailhol.vincent@...adoo.fr>,
"Linus Torvalds" <torvalds@...ux-foundation.org>
Subject: [PATCH next] linux/bits.h: Simplify GENMASK()
Change 95b980d62d52c replaced ~0ul and ~0ull with ~UL(0) and ~ULL(0)
in the GENMASK() defines as part of a change to allow the bitops
definitions be used from assembly.
The definitions have since been moved to a uapi header which
probably shouldn't require some other header be included first.
The definition of __GENMASK() is also overcomplicated partially
due to avoiding overflow warnings from shifting ~0u left.
Implement GENMASK() using the simpler (1u << hi) * 2 - (1u << lo) formula.
This doesn't rely on right shifts and doesn't need to know the number
of bits in the integral type.
It can be used for different types by just changing the type of the 1u.
__GENMASK() __GENMASK_ULL() and __GENMASK_U128() can now implemeted
using a single ___GENMASK(one, hi, lo).
Overflow warnings (from shifting out the MSB) are avoided by subtracting 1
before the multiply and adding two back in later.
The complers see straight through the subterfuge when generating code.
Since there are already conditionals for ASSEMBLY in bits.h, for ASSEMBLY
directly expand GENMASK() and GENMASK_ULL() as ___GENMASK(1, hi, lo)
rather than through the __GENMASK() and __GENMASK_ULL() uapi defines.
Remove the UL(x) and ULL(x) from the uapi file.
GENMASK() and GENMASK_ULL() now generate the correct values when
ASSEMBLY is defined.
Fortunately they've never been used.
Rename 'h' to 'hi' and 'l' to 'lo' because 'l' looks like '1' in many fonts.
Signed-off-by: David Laight <david.laight@...lab.com>
---
include/linux/bits.h | 43 ++++++++++++++++-----------------------
include/uapi/linux/bits.h | 15 +++++++-------
2 files changed, 24 insertions(+), 34 deletions(-)
diff --git a/include/linux/bits.h b/include/linux/bits.h
index 60044b608817..d5cf0ec22e43 100644
--- a/include/linux/bits.h
+++ b/include/linux/bits.h
@@ -14,41 +14,32 @@
#define BITS_PER_BYTE 8
/*
- * Create a contiguous bitmask starting at bit position @l and ending at
- * position @h. For example
+ * Create a contiguous bitmask starting at bit position @lo and ending at
+ * position @hi. For example
* GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.
*/
#if !defined(__ASSEMBLY__)
#include <linux/build_bug.h>
-#define GENMASK_INPUT_CHECK(h, l) \
+#define GENMASK_INPUT_CHECK(hi, lo) \
(BUILD_BUG_ON_ZERO(__builtin_choose_expr( \
- __is_constexpr((l) > (h)), (l) > (h), 0)))
-#else
-/*
- * BUILD_BUG_ON_ZERO is not available in h files included from asm files,
- * disable the input check if that is the case.
- */
-#define GENMASK_INPUT_CHECK(h, l) 0
-#endif
+ __is_constexpr((lo) > (hi)), (lo) > (hi), 0)))
-#define GENMASK(h, l) \
- (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
-#define GENMASK_ULL(h, l) \
- (GENMASK_INPUT_CHECK(h, l) + __GENMASK_ULL(h, l))
+#define GENMASK(hi, lo) \
+ (GENMASK_INPUT_CHECK(hi, lo) + __GENMASK(hi, lo))
+#define GENMASK_ULL(hi, lo) \
+ (GENMASK_INPUT_CHECK(hi, lo) + __GENMASK_ULL(hi, lo))
-#if !defined(__ASSEMBLY__)
+#define GENMASK_U128(hi, lo) \
+ (GENMASK_INPUT_CHECK(hi, lo) + __GENMASK_U128(hi, lo))
+#else
/*
- * Missing asm support
- *
- * __GENMASK_U128() depends on _BIT128() which would not work
- * in the asm code, as it shifts an 'unsigned __init128' data
- * type instead of direct representation of 128 bit constants
- * such as long and unsigned long. The fundamental problem is
- * that a 128 bit constant will get silently truncated by the
- * gcc compiler.
+ * BUILD_BUG_ON_ZERO is not available in h files included from asm files,
+ * 128bit exprssions don't work, neither can C 0UL (etc) constants be used.
+ * These definitions only have to work for constants and don't require
+ * that ~0 have any specific number of set bits.
*/
-#define GENMASK_U128(h, l) \
- (GENMASK_INPUT_CHECK(h, l) + __GENMASK_U128(h, l))
+#define GENMASK(hi, lo) ___GENMASK(1, hi, lo)
+#define GENMASK_ULL(hi, lo) ___GENMASK(1, hi, lo)
#endif
#endif /* __LINUX_BITS_H */
diff --git a/include/uapi/linux/bits.h b/include/uapi/linux/bits.h
index 5ee30f882736..a25d9dfb7072 100644
--- a/include/uapi/linux/bits.h
+++ b/include/uapi/linux/bits.h
@@ -4,15 +4,14 @@
#ifndef _UAPI_LINUX_BITS_H
#define _UAPI_LINUX_BITS_H
-#define __GENMASK(h, l) \
- (((~_UL(0)) - (_UL(1) << (l)) + 1) & \
- (~_UL(0) >> (__BITS_PER_LONG - 1 - (h))))
+/* Result is '(1u << (hi + 1)) - (1u << lo)' coded to avoid overflow. */
+#define ___GENMASK(one, hi, lo) \
+ ((((one) << (hi)) - 1) * 2 + 1 - (((one) << (lo)) - 1))
-#define __GENMASK_ULL(h, l) \
- (((~_ULL(0)) - (_ULL(1) << (l)) + 1) & \
- (~_ULL(0) >> (__BITS_PER_LONG_LONG - 1 - (h))))
+#define __GENMASK(hi, lo) ___GENMASK(1UL, hi, lo)
-#define __GENMASK_U128(h, l) \
- ((_BIT128((h)) << 1) - (_BIT128(l)))
+#define __GENMASK_ULL(hi, lo) ___GENMASK(1ULL, hi, lo)
+
+#define __GENMASK_U128(hi, lo) ___GENMASK((__uint128)1, hi, lo)
#endif /* _UAPI_LINUX_BITS_H */
--
2.17.1
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Powered by blists - more mailing lists