Re: [PATCH 1/2] include: bitops: Add mask extract64/deposit64

[Alistair Francis <alistair23@gmail.com>]

On Tue, Aug 6, 2024 at 5:32 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 8/5/24 14:33, Alistair Francis wrote:
> > Based on the RISC-V get_field() and set_field() macros add
> > mask_extract64() and mask_deposit64() bitop functions. These can extrac
> > and deposit values into fields using a bit field mask directly instead
> > of a length and shift.
> >
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >   include/qemu/bitops.h | 35 +++++++++++++++++++++++++++++++++++
> >   1 file changed, 35 insertions(+)
> >
> > diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
> > index 2c0a2fe751..dd26f4a6b5 100644
> > --- a/include/qemu/bitops.h
> > +++ b/include/qemu/bitops.h
> > @@ -409,6 +409,22 @@ static inline uint64_t extract64(uint64_t value, int start, int length)
> >       return (value >> start) & (~0ULL >> (64 - length));
> >   }
> >
> > +/**
> > + * mask_extract64:
> > + * @value: the value to extract the bit field from
> > + * @mask: the mask bit field to extract
> > + *
> > + * Extract from the 64 bit input @value the bit mask specified by the
> > + * @mask parameter, and return it. The value returned is shifted
> > + * so that only the bit field is returned.
> > + *
> > + * Returns: the value of the bit field extracted from the input value.
> > + */
> > +static inline uint64_t mask_extract64(uint64_t value, uint64_t mask)
> > +{
> > +    return (value & mask) / (mask & ~(mask << 1));
> > +}
>
> Adding these miss the point of using "standard" qemu operations.

My thinking is that if they are added then they become standard :)

At least then they are included in core code and easier for people to
understand.

>
> But if we were going to add this, avoid the division.
>
>   (value & mask) >> ctz64(mask)

Good point

Alistair

>
> I presume the original formulation is so that the macro can be used in the context of a
> compile-time constant.
>
>
> r~