From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50724) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YQ0QZ-0007Al-Sh for qemu-devel@nongnu.org; Mon, 23 Feb 2015 16:20:33 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YQ0QW-0007s7-Lo for qemu-devel@nongnu.org; Mon, 23 Feb 2015 16:20:31 -0500 Received: from mx1.redhat.com ([209.132.183.28]:50649) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YQ0QW-0007rt-BF for qemu-devel@nongnu.org; Mon, 23 Feb 2015 16:20:28 -0500 Message-ID: <54EB9992.20601@redhat.com> Date: Mon, 23 Feb 2015 14:20:18 -0700 From: Eric Blake MIME-Version: 1.0 References: <1424694237-22786-1-git-send-email-aik@ozlabs.ru> <877fv8sp97.fsf@blackfin.pond.sub.org> <54EB52AE.4030002@redhat.com> <87sidwo7a7.fsf@blackfin.pond.sub.org> In-Reply-To: <87sidwo7a7.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="q2QOJQtqMA7phLCTmjEL6AvG76nABxqKb" Subject: Re: [Qemu-devel] [PATCH v2] utils: Add pow2ceil() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Alexey Kardashevskiy , Peter Maydell , qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --q2QOJQtqMA7phLCTmjEL6AvG76nABxqKb Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 02/23/2015 10:40 AM, Markus Armbruster wrote: >>> int64_t pow2floor(int64_t value) >>> { >>> assert(value > 0); >>> return 0x8000000000000000u >> clz64(value); >>> } >> >> Needs to be 0x8000000000000000ull for 32-bit machines to compile corre= ctly. >=20 > Why? Because 0x8000000000000000u is only required to be a 'long', and on 32-bit machines, your constant would overflow long. See, for example, commit 5cb6be2ca. You NEED the 'll' suffix to ensure that the compiler doesn't reject the constant as an overflow. >=20 >> Why is the parameter int64_t? Wouldn't it be more useful to have: >> >> uint64_t pow2floor(uint64_t value) >=20 > Crossed my mind, too. However, the existing callers pass *signed* > arguments. I guess it means auditing callers either way. >=20 >>> int64_t pow2ceil(int64_t value) >>> { >> >> Again, why allow signed inputs? >> >>> assert(value <=3D 0x4000000000000000) >>> if (value <=3D 1) >>> return 1; >> >> In particular, this slams all negative values to a result of 1, which >> doesn't necessarily make sense. >=20 > It implements a straightforward contract: return the smallest power of > two greater or equal to the argument. The function's domain is the set= > of int64_t arguments where this value can be represented in int64_t, > i.e. [-2^63..2^62]. >=20 > Feel free to suggest a more sensible contract. But why should I claim that the nearest power of 2 greater than -3 is positive 1, when I could argue that it should instead be -2 (nearest positive or negative power of 2 rounding towards +inf) or -4 (nearest positive or negative power of 2 rounding away from 0)? Since there are multiple potential contracts once negative values are involved, I find it easier to just make the contract require a positive input to begin wit= h. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --q2QOJQtqMA7phLCTmjEL6AvG76nABxqKb Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJU65mSAAoJEKeha0olJ0NqvgkH/08SzdHMqFV84ZU7l/rFDe0K 0uSJupLLZMcM9E1pWDpE90137LFyryx96S43SZuUulJiro9aSAou38b1dmHt3Job 2anYXzpWWGyA0HMGKf/U5SkAfTZHiajiYjg4/J43/MkSPZwbvR1r67zlOzNJLxVL yAkeAHK/3sqeHRG3+I0D8Qj6Ld2Ccq3oi4HDc/C4lw98C46CMEhD+YelXQh94/XQ HYCzKXxXrGbX6BUDtjHFCrBx6Py4aeSzlApM+CFo/NgbRqNt3rnAah2i4aBB2kQG bP2wnR7s6Txg467iEdJ5zu3dsendEiQwEi0nJKvEqAyMKLawP1dLU/K/yP2U61Q= =upnb -----END PGP SIGNATURE----- --q2QOJQtqMA7phLCTmjEL6AvG76nABxqKb--