Patchwork [v2,1/5] bitops: Add ONES macro

login
register
mail settings
Submitter Peter Crosthwaite
Date April 5, 2013, 8:43 a.m.
Message ID <b956c896f5581a9ee1c2cf97f7fb9ac84bb67455.1365151096.git.peter.crosthwaite@xilinx.com>
Download mbox | patch
Permalink /patch/234069/
State New
Headers show

Comments

Peter Crosthwaite - April 5, 2013, 8:43 a.m.
Little macro that just gives you N ones (justified to LSB).

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 include/qemu/bitops.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
Peter Crosthwaite - April 5, 2013, 9:11 a.m.
Hi Peter,

On Fri, Apr 5, 2013 at 6:53 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 5 April 2013 09:43, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>> Little macro that just gives you N ones (justified to LSB).
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> ---
>>
>>  include/qemu/bitops.h |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
>> index affcc96..da47fc8 100644
>> --- a/include/qemu/bitops.h
>> +++ b/include/qemu/bitops.h
>> @@ -273,4 +273,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)
>
> You can avoid the ?: here (assuming you're happy to say that
> ONES(0) is a silly thing to ask for):
>

Not that silly. ONES(0) should just stay true to the contract and
return 0. Otherwise prospective clients may have to guard against 0
cases in loops which is ugly:

for (i = 0; i < N; i++) {
    ....
    mask = ONES(i); /* rather than ones = i ? ones(i) : 0 */
    ....
}

But  I thought about this more, and perhaps just ditch the ? anyway,
and rely on the shift by 64 ending up as 0. Then the -1 should return
return 64 (all) ones anyway.

> #define ONES(num) (~0ULL >> (64 - (num)))
>
> Needs a documentation comment, anyway.
>

OK

Regards,
Peter

> thanks
> -- PMM
>
Peter Maydell - April 5, 2013, 9:24 a.m.
On 5 April 2013 10:11, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
> But  I thought about this more, and perhaps just ditch the ? anyway,
> and rely on the shift by 64 ending up as 0.

That's undefined behaviour, please don't.

-- PMM

Patch

diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
index affcc96..da47fc8 100644
--- a/include/qemu/bitops.h
+++ b/include/qemu/bitops.h
@@ -273,4 +273,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