Re: [Qemu-devel] [PATCH 1/5] BIT_RANGE convenience macro

[Dr. David Alan Gilbert]

* Peter Maydell (address@hidden) wrote:
> On 16 June 2016 at 18:12, Dr. David Alan Gilbert (git)
> <address@hidden> wrote:
> > From: "Dr. David Alan Gilbert" <address@hidden>
> >
> > e.g. BIT_RANGE(15, 0) gives 0xff00
> >
> > Suggested by: Paolo Bonzini <address@hidden>
> > Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> > ---
> >  include/qemu/bitops.h | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
> > index 755fdd1..e411688 100644
> > --- a/include/qemu/bitops.h
> > +++ b/include/qemu/bitops.h
> > @@ -23,6 +23,9 @@
> >  #define BIT_MASK(nr)            (1UL << ((nr) % BITS_PER_LONG))
> >  #define BIT_WORD(nr)            ((nr) / BITS_PER_LONG)
> >  #define BITS_TO_LONGS(nr)       DIV_ROUND_UP(nr, BITS_PER_BYTE * 
> > sizeof(long))
> > +/* e.g. BIT_RANGE(15, 0) -> 0xff00 */
> > +#define BIT_RANGE(hb, lb)     ((2ull << (hb)) - (1ull << (lb)))
> 
> Isn't this undefined behaviour if the hb is 63?

I've checked the C99 spec; it explicitly defines the unsigned
behaviour ('reduced modulo one more than the maximum value representable in the 
result type').

> Also there is semantic overlap with the MAKE_64BIT_MASK macro
> proposed in 
> https://lists.gnu.org/archive/html/qemu-devel/2016-05/msg02154.html
> (which also has ub, but see
> https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg02614.html
> for the version which doesn't).
> 
> I prefer a "start, length" macro to "position, position",
> because this matches what we already have for the deposit
> and extract functions in this header.

I think it depends on the use; I agree that makes sense
for things like extracting an n-bit integer; in this case
what we have is something which is fixed at bit 51 and
another bit - we dont ever think about the difference between
those two bits.

Dave

> 
> thanks
> -- PMM
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK