On 31/01/2025 at 22:46, Geert Uytterhoeven wrote:
> The existing FIELD_{GET,PREP}() macros are limited to compile-time
> constants. However, it is very common to prepare or extract bitfield
> elements where the bitfield mask is not a compile-time constant.
Why is it that the existing FIELD_{GET,PREP}() macros must be limited to
compile time constants? Instead of creating another variant for
non-constant bitfields, wouldn't it be better to make the existing macro
accept both?
As far as I can see, only __BUILD_BUG_ON_NOT_POWER_OF_2() and
__BF_FIELD_CHECK() need to be adjusted. I am thinking of this:
diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index 63928f173223..c6bedab862d1 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h
@@ -8,6 +8,7 @@
#define _LINUX_BITFIELD_H
#include <linux/build_bug.h>
+#include <linux/compiler.h>
#include <asm/byteorder.h>
/*
@@ -62,15 +63,13 @@
#define __BF_FIELD_CHECK(_mask, _reg, _val, _pfx) \
({ \
- BUILD_BUG_ON_MSG(!__builtin_constant_p(_mask), \
- _pfx "mask is not constant"); \
- BUILD_BUG_ON_MSG((_mask) == 0, _pfx "mask is zero"); \
- BUILD_BUG_ON_MSG(__builtin_constant_p(_val) ? \
- ~((_mask) >> __bf_shf(_mask)) & \
- (0 + (_val)) : 0, \
+ BUILD_BUG_ON_MSG(statically_true((_mask) == 0), \
+ _pfx "mask is zero"); \
+ BUILD_BUG_ON_MSG(statically_true(~((_mask) >>
__bf_shf(_mask)) & \
+ (0 + (_val))), \
_pfx "value too large for the field"); \
- BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask) > \
- __bf_cast_unsigned(_reg, ~0ull), \
+
BUILD_BUG_ON_MSG(statically_true(__bf_cast_unsigned(_mask, _mask) > \
+
__bf_cast_unsigned(_reg, ~0ull)), \
_pfx "type of reg too small for mask"); \
__BUILD_BUG_ON_NOT_POWER_OF_2((_mask) + \
(1ULL << __bf_shf(_mask))); \
diff --git a/include/linux/build_bug.h b/include/linux/build_bug.h
index 3aa3640f8c18..3b8055ebb55f 100644
--- a/include/linux/build_bug.h
+++ b/include/linux/build_bug.h
@@ -18,9 +18,9 @@
/* Force a compilation error if a constant expression is not a power of
2 */
#define __BUILD_BUG_ON_NOT_POWER_OF_2(n) \
- BUILD_BUG_ON(((n) & ((n) - 1)) != 0)
+ BUILD_BUG_ON(statically_true((n) & ((n) - 1)))
#define BUILD_BUG_ON_NOT_POWER_OF_2(n) \
- BUILD_BUG_ON((n) == 0 || (((n) & ((n) - 1)) != 0))
+ BUILD_BUG_ON(statically_true(!(n) || ((n) & ((n) - 1))))
/*
* BUILD_BUG_ON_INVALID() permits the compiler to check the validity of the
> To avoid this limitation, the AT91 clock driver and several other
> drivers already have their own non-const field_{prep,get}() macros.
> Make them available for general use by consolidating them in
> <linux/bitfield.h>, and improve them slightly:
> 1. Avoid evaluating macro parameters more than once,
> 2. Replace "ffs() - 1" by "__ffs()",
> 3. Support 64-bit use on 32-bit architectures.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
(...)
Yours sincerely,
Vincent Mailhol
