| Message ID | 889f6c7641bb04403de2a03406177dcf24ca8a53.1455055858.git.alistair.francis@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
On Mon, Feb 29, 2016 at 3:51 AM, Alex Bennée <alex.bennee@linaro.org> wrote: > > 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 I like that idea. I have updated the implementation to use this style instead of the ONES() style. Thanks, Alistair > > > -- > Alex Bennée >
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) + #endif