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

[Yury Norov]

+ Matt Coster

On Fri, Apr 10, 2026 at 10:09:27AM +0100, david.laight.linux@xxxxxxxxx wrote:
> From: David Laight <david.laight.linux@xxxxxxxxx>
> 
> 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@xxxxxxxxx>

Thanks, David.

So, the original patch that spotted the problem is:

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

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