Re: [Qemu-devel] [PATCH v4 08/16] bitops: Add ONES macro

["Alex Bennée" <alex.bennee@linaro.org>]

Alistair Francis <alistair.francis@xilinx.com> writes:

> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>
> Little macro that just gives you N ones (justified to LSB).
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
>
>  include/qemu/bitops.h | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
> index 8164225..27bf98d 100644
> --- a/include/qemu/bitops.h
> +++ b/include/qemu/bitops.h
> @@ -430,4 +430,6 @@ static inline uint64_t deposit64(uint64_t value, int start, int length,
>      return (value & ~mask) | ((fieldval << start) & mask);
>  }
>
> +#define ONES(num) ((num) == 64 ? ~0ull : (1ull << (num)) - 1)
> +

I'm a little ambivalent about this one. The definition should probably
be up at the top of the file with the other #defines. I don't like the
name as it is a little too generic. I also notice in all the actual uses
you immediately invert the result because you want to mask the top bits.

I suspect what's needed here is a more generic helper:

#define MAKE_64BIT_MASK (shift, length) \
...

And then the:

        .ro = ~ONES(27) },

Becomes:

        .ro = MAKE_64BIT_MASK(27, 64 - 27);


>  #endif


--
Alex Bennée