On 25.06.2024 21:07, Andrew Cooper wrote:
> The prior version (renamed to bitmap_for_each()) was inefficeint when used
> over a scalar, but this is the more common usage even before accounting for
> the many opencoded forms.
>
> Introduce a new version which operates on scalars only and does so without
> spilling them to memory. This in turn requires the addition of a type-generic
> form of ffs().
>
> Add testing for the new construct alongside the ffs/fls testing.
>
> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
Reviewed-by: Jan Beulich <jbeul...@suse.com>
with two remarks:
> The naming of ffs_g() is taken from the new compiler builtins which are using
> a g suffix to mean type-generic.
As you say, a g suffix, not a _g one; gcc14 documents __builtin_ffsg().
(Seeing it exists I wonder whether we wouldn't want to use it when
available, and only fall back to the macro for older versions.) Any
specific reason you use _g?
> --- a/xen/include/xen/bitops.h
> +++ b/xen/include/xen/bitops.h
> @@ -56,6 +56,16 @@ static always_inline __pure unsigned int ffs64(uint64_t x)
> return !x || (uint32_t)x ? ffs(x) : ffs(x >> 32) + 32;
> }
>
> +/*
> + * A type-generic ffs() which picks the appropriate ffs{,l,64}() based on
> it's
> + * argument.
> + */
> +#define ffs_g(x) \
> + sizeof(x) <= sizeof(int) ? ffs(x) : \
> + sizeof(x) <= sizeof(long) ? ffsl(x) : \
> + sizeof(x) <= sizeof(uint64_t) ? ffs64(x) : \
> + ({ BUILD_ERROR("ffs_g() Bad input type"); 0; })
Related to the patch introducing it: I can see how BUILD_ERROR_ON() may
be deemed sub-optimal here. Nevertheless I think we should be able to
find some common ground there.
Jan
