Re: [PATCH 1/1] bitfield.h: Ensure FIELD_PREP_CONST() is constant

Yury Norov
2026-04-10T16:55:33Z

+ Matt Coster

On Fri, Apr 10, 2026 at 10:09:27AM +0100, david.laight.linux@gmail.com wrote:
> From: David Laight <david.laight.linux@gmail.com>
>
> Some versions of gcc report that some expressions involving
> __builtin_ffsll(1ull << 63) are not integer constant expressions.
>
> Rework FIELD_PREP_CONST() to avoid the issue.
>
> Signed-off-by: David Laight <david.laight.linux@gmail.com>

Thanks, David.

So, the original patch that spotted the problem is:

https://lore.kernel.org/all/20260409-field-prep-fix-v1-1-f0e9ae64f63c@imgtec.com/

Let's keep Matt in the loop, at least.

Please keep the details from the original patch. "Some versions of
some expressions" style is too uncertain.

> ---
>
> Note that when 'val' is a variable 'val << constant' is likely
> to execute faster than 'val * (1 << constant)'.
> So the normal FIELD_PREP() is best left alone.

Do you have any numbers? I'd prefer to have the codebase consistent
when possible.

Thanks,
Yury

> include/linux/bitfield.h | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
> index 54aeeef1f0ec..f21765851d5c 100644
> --- a/include/linux/bitfield.h
> +++ b/include/linux/bitfield.h
> @@ -45,6 +45,7 @@
> */
>
> #define __bf_shf(x) (__builtin_ffsll(x) - 1)
> +#define __bf_low_bit(mask) ((mask) & (~(mask) + 1))
>
> #define __scalar_type_to_unsigned_cases(type) \
> unsigned type: (unsigned type)0, \
> @@ -138,8 +139,6 @@
> __FIELD_PREP(_mask, _val, "FIELD_PREP: "); \
> })
>
> -#define __BF_CHECK_POW2(n) BUILD_BUG_ON_ZERO(((n) & ((n) - 1)) != 0)
> -
> /**
> * FIELD_PREP_CONST() - prepare a constant bitfield element
> * @_mask: shifted mask defining the field's length and position
> @@ -157,11 +156,11 @@
> /* mask must be non-zero */ \
> BUILD_BUG_ON_ZERO((_mask) == 0) + \
> /* check if value fits */ \
> - BUILD_BUG_ON_ZERO(~((_mask) >> __bf_shf(_mask)) & (_val)) + \
> + BUILD_BUG_ON_ZERO(~((_mask) / __bf_low_bit(_mask)) & (_val)) + \
> /* check if mask is contiguous */ \
> - __BF_CHECK_POW2((_mask) + (1ULL << __bf_shf(_mask))) + \
> + BUILD_BUG_ON_ZERO((_mask) & ((_mask) + __bf_low_bit(_mask))) + \
> /* and create the value */ \
> - (((typeof(_mask))(_val) << __bf_shf(_mask)) & (_mask)) \
> + (((_val) * __bf_low_bit(_mask)) & (_mask)) \
> )
>
> /**
> --
> 2.39.5