Re: [Qemu-devel] [PATCH] bitops: unify bitops_ffsl with the one in host-utils.h

[Peter Maydell]

On 30 January 2013 16:53, Paolo Bonzini <address@hidden> wrote:
> +    if (!word) {
> +        return 0;
> +    }

If we're specifically defining the behaviour for zero input we
need to fix the API documentation comment which currently says
this case is undefined and the caller should be checking.

> -#if LONG_MAX > 0x7FFFFFFF
> -       if ((word & 0xffffffff) == 0) {
> -               num += 32;
> -               word >>= 32;
> -       }
> +#if QEMU_GNUC_PREREQ(3, 4)
> +    return __builtin_ctzl(word) + 1;
> +#else
> +    if (sizeof(long) == 4) {
> +        return ctz32(word) + 1;
> +    } else if (sizeof(long) == 8) {
> +        return ctz64(word) + 1;
> +    } else {
> +        abort();
> +    }
>  #endif
> -       if ((word & 0xffff) == 0) {
> -               num += 16;
> -               word >>= 16;
> -       }
> -       if ((word & 0xff) == 0) {
> -               num += 8;
> -               word >>= 8;
> -       }
> -       if ((word & 0xf) == 0) {
> -               num += 4;
> -               word >>= 4;
> -       }
> -       if ((word & 0x3) == 0) {
> -               num += 2;
> -               word >>= 2;
> -       }
> -       if ((word & 0x1) == 0) {
> -               num += 1;
> -        }
> -       return num;
>  }

This reimplementation appears to have an off by one error.
For example, on an input of 4, the old algorithm returns 2
and this one returns 3.

(this is what was causing my test case on MacOS to hang.)

-- PMM