From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:40048)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <alistair23@gmail.com>) id 1abyQp-0002ex-2c
	for qemu-devel@nongnu.org; Fri, 04 Mar 2016 17:42:47 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <alistair23@gmail.com>) id 1abyQo-0003cI-2S
	for qemu-devel@nongnu.org; Fri, 04 Mar 2016 17:42:47 -0500
Received: from mail-oi0-x242.google.com ([2607:f8b0:4003:c06::242]:34151)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <alistair23@gmail.com>) id 1abyQn-0003cE-Tu
	for qemu-devel@nongnu.org; Fri, 04 Mar 2016 17:42:45 -0500
Received: by mail-oi0-x242.google.com with SMTP id m82so3714706oif.1
	for <qemu-devel@nongnu.org>; Fri, 04 Mar 2016 14:42:45 -0800 (PST)
MIME-Version: 1.0
Sender: alistair23@gmail.com
In-Reply-To: <87h9grka4e.fsf@linaro.org>
References: <cover.1455055858.git.alistair.francis@xilinx.com>
	<889f6c7641bb04403de2a03406177dcf24ca8a53.1455055858.git.alistair.francis@xilinx.com>
	<87h9grka4e.fsf@linaro.org>
From: Alistair Francis <alistair.francis@xilinx.com>
Date: Fri, 4 Mar 2016 14:42:15 -0800
Message-ID: <CAKmqyKNtyRgapPJnoCqAk9sbbbX1gjUWbCLUGmrtqJDe6Lc3MA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [Qemu-devel] [PATCH v4 08/16] bitops: Add ONES macro
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: =?UTF-8?B?QWxleCBCZW5uw6ll?= <alex.bennee@linaro.org>
Cc: Edgar Iglesias <edgar.iglesias@xilinx.com>, Peter Maydell <peter.maydell@linaro.org>, "qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>, Alistair Francis <alistair.francis@xilinx.com>, Peter Crosthwaite <crosthwaitepeter@gmail.com>, Edgar Iglesias <edgar.iglesias@gmail.com>, =?UTF-8?Q?Andreas_F=C3=A4rber?= <afaerber@suse.de>, =?UTF-8?B?S09OUkFEIEZyw6lkw6lyaWM=?= <fred.konrad@greensocs.com>

On Mon, Feb 29, 2016 at 3:51 AM, Alex Benn=C3=A9e <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) =3D=3D 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 =3D ~ONES(27) },
>
> Becomes:
>
>         .ro =3D 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=C3=A9e
>