diff mbox

[v4,08/16] bitops: Add ONES macro

Message ID 889f6c7641bb04403de2a03406177dcf24ca8a53.1455055858.git.alistair.francis@xilinx.com
State New
Headers show

Commit Message

Alistair Francis Feb. 9, 2016, 10:14 p.m. UTC 
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(+)

Comments

Alex Bennée Feb. 29, 2016, 11:51 a.m. UTC | #1 
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
Alistair Francis March 4, 2016, 10:42 p.m. UTC | #2 
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 mbox

Patch

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