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
[-- Attachment #1 --] 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: >>> >>> Index: x86/include/endian.h >>> =================================================================== >>> --- endian.h (revision 233184) >>> +++ endian.h (working copy) >>> @@ -65,9 +65,9 @@ >>> >>> #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)) >>> >>> #ifdef __GNUCLIKE_BUILTIN_CONSTANT_P >>> #define __bswap16(x) \ >>> >>> I ran into this while porting the __builtin_constant_p() functionality >>> over to ia64. >> >> 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. > > Oh, I now parse the comment in __bswap64_var() correctly. That's fugly. > > 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. [-- Attachment #2 --] -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.18 (FreeBSD) iF4EABEIAAYFAk9pkcIACgkQfoCS2CCgtivQ6AD/dv8UZ/JL9KIGvhpFlUnBAy3l BgOIVlAC0I/ol+MolkUA/0gZiQJAHpPcCl2k7/sRx2b0Fq0qVr8Ng1pWX7dHmuGS =dgsC -----END PGP SIGNATURE-----
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201203210930.58389.tijl>