Date: Wed, 21 Mar 2012 09:30:53 +0100 From: Tijl Coosemans <tijl@freebsd.org> To: John Baldwin <jhb@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r232721 - head/sys/x86/include Message-ID: <201203210930.58389.tijl@freebsd.org> In-Reply-To: <201203201456.14375.jhb@freebsd.org> References: <201203091148.q29BmuIp005151@svn.freebsd.org> <201203201519.12926.tijl@freebsd.org> <201203201456.14375.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--nextPart2167658.op2pGTnxf8 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: quoted-printable On Tuesday 20 March 2012 19:56:14 John Baldwin wrote: > On Tuesday, March 20, 2012 10:19:07 am Tijl Coosemans wrote: >> On Tuesday 20 March 2012 13:34:10 John Baldwin wrote: >>> BTW, I think I found an old "bug" in this file. The _gen() variants >>> should only use the _gen() variants of smaller types rather than using >>> the version that re-checks __build_constant_p() I think. That is: >>>=20 >>> Index: x86/include/endian.h >>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >>> --- endian.h (revision 233184) >>> +++ endian.h (working copy) >>> @@ -65,9 +65,9 @@ >>> =20 >>> #define __bswap16_gen(x) (__uint16_t)((x) << 8 | (x) >> 8) >>> #define __bswap32_gen(x) \ >>> - (((__uint32_t)__bswap16(x) << 16) | __bswap16((x) >> 16)) >>> + (((__uint32_t)__bswap16_gen(x) << 16) | __bswap16_gen((x) >> 16)) >>> #define __bswap64_gen(x) \ >>> - (((__uint64_t)__bswap32(x) << 32) | __bswap32((x) >> 32)) >>> + (((__uint64_t)__bswap32_gen(x) << 32) | __bswap32_gen((x) >> 32)) >>> =20 >>> #ifdef __GNUCLIKE_BUILTIN_CONSTANT_P >>> #define __bswap16(x) \ >>>=20 >>> I ran into this while porting the __builtin_constant_p() functionality >>> over to ia64. >>=20 >> No, on i386 bswap64 with a variable argument currently expands to two >> bswap instructions. With your change it would be many shifts and logical >> operations. The _gen variants are more like fallback implementations. >> If bswapNN cannot be implemented directly it is split up. If those >> smaller problems can be implemented directly, good, if not split it up >> again and so on. >=20 > Oh, I now parse the comment in __bswap64_var() correctly. That's fugly. >=20 > Still, it seems that if I keep the patch to port this to ia64 (so it > can do constants as constants), then it will need to use this approach > since it won't have the i386 problem (in its case the _gen variants > are only used for constants). Maybe name them _const then as on other architectures. --nextPart2167658.op2pGTnxf8 Content-Type: application/pgp-signature; name=signature.asc Content-Description: This is a digitally signed message part. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.18 (FreeBSD) iF4EABEIAAYFAk9pkcIACgkQfoCS2CCgtivQ6AD/dv8UZ/JL9KIGvhpFlUnBAy3l BgOIVlAC0I/ol+MolkUA/0gZiQJAHpPcCl2k7/sRx2b0Fq0qVr8Ng1pWX7dHmuGS =dgsC -----END PGP SIGNATURE----- --nextPart2167658.op2pGTnxf8--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201203210930.58389.tijl>