Re: [PATCH] x86/bitops: Replace __scanbit() with a simpler tzcnt()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 10.11.2026 16:31, Andrew Cooper wrote:
> There are 4 remaining users of __scanbit(), and they're capable of dealing
> with max being BITS_PER_LONG.  Switch to a simpler tzcnt() helper, which also
> removed some alternatives from behind the scenes.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
(with a suggestion below)

> Supercedes "x86/bitops: Improve code generation for __scanbit()"

Quite a bit neater indeed.

> --- a/xen/arch/x86/include/asm/bitops.h
> +++ b/xen/arch/x86/include/asm/bitops.h
> @@ -320,18 +320,23 @@ extern unsigned int __find_first_zero_bit(
>  extern unsigned int __find_next_zero_bit(
>      const unsigned long *addr, unsigned int size, unsigned int offset);
>  
> -static always_inline unsigned int __scanbit(unsigned long val, unsigned int 
> max)
> +/*
> + * Search for the first set bit in @x, returning BITS_PER_LONG if @x was 0.
> + * See arch_ffs() for safety discussions.
> + */
> +static always_inline unsigned int tzcnt(unsigned long x)
>  {
> -    if ( __builtin_constant_p(max) && max == BITS_PER_LONG )
> -        alternative_io("bsf %[in],%[out]; cmovz %[max],%k[out]",
> -                       "rep; bsf %[in],%[out]",
> -                       X86_FEATURE_BMI1,
> -                       [out] "=&r" (val),
> -                       [in] "r" (val), [max] "r" (max));
> -    else
> -        asm ( "bsf %1,%0 ; cmovz %2,%k0"
> -              : "=&r" (val) : "r" (val), "r" (max) );
> -    return (unsigned int)val;
> +    unsigned int r;
> +
> +    /*
> +     * The destination register still needs pre-loading for older CPUs which
> +     * will execute TZCNT as BSF.
> +     */
> +    asm ( "tzcnt %[x], %q[r]"
> +          : [r] "=r" (r)
> +          : [x] "rm" (x), "[r]" (BITS_PER_LONG) );
> +
> +    return r;
>  }

By having r and the function return type be unsigned int, when the actual
insn really is a qword operation, you force call sites to do the zero-
extension if they want an unsigned long quantity. When both are unsigned
long, no zero-extension needs performing by the compiler. Truncation to
unsigned int (or less) won't require any extra insn(s) anyway. While this
doesn't look as if it would make a difference for present uses, future
ones may benefit.

Jan