Date: Thu, 1 Mar 2012 23:46:20 +0100 From: Tijl Coosemans <tijl@freebsd.org> To: Bruce Evans <brde@optusnet.com.au> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Dimitry Andric <dim@freebsd.org> Subject: Re: svn commit: r232266 - in head/sys: amd64/include i386/include pc98/include x86/include Message-ID: <201203012346.20648.tijl@freebsd.org> In-Reply-To: <20120229181608.S2887@besplex.bde.org> References: <201202281939.q1SJdtYr084858@svn.freebsd.org> <20120229160522.W2514@besplex.bde.org> <20120229181608.S2887@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--nextPart1956764.VNK7ZdMx6j
Content-Type: Text/Plain;
charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
On Wednesday 29 February 2012 08:44:54 Bruce Evans wrote:
> On Wed, 29 Feb 2012, Bruce Evans wrote:
>> I cleaned this up a bit according to ideas in my previous mails, and
>> added a comment about the confusing use of __bswap64_const() (now
>> named __bswap64_gen()) in __bswap64_var():
>=20
> A minor problem with only having a macro version for __bswap64() turned
> up:
>=20
>> % -#define __bswap16_const(_x) (__uint16_t)((_x) << 8 | (_x) >> 8)
>> % -
>> % -#define __bswap16(_x) \
>> % - (__builtin_constant_p(_x) ? \
>> % - __bswap16_const((__uint16_t)(_x)) : __bswap16_var(_x))
>> ...
>> % +#define ___bswap16(x) (__uint16_t)((x) << 8 | (x) >> 8)
>> % +#define __bswap16(x) (___bswap16((__uint16_t)(x)))
>=20
> When x a non-volatile variable, gcc and clang produce the good code
> "rolw $8,x" for "x =3D __bswap16(x);" on short x. But when x a a volatile
> variable, gcc and clang produce fairly horrible code, with 2 loads of
> x corresponding to the 2 accesses to x. This is probably required by
> volatile semantics, and is a problem for all unsafe macros, especially
> when their name says that they are safe (oops). When __bswap16 is
> implemented as an inline function for the var case like it used to be,
> it only loads x once and there are no problems with volatile variables.
> Optimizing to "rolw $8,x" might still be possible iff x is not volatile,
> but load-modify-store is probably better anyway.
>=20
> So any macro version must use gcc features to be safe. The following
> seems to work:
>=20
> #define __bswap16(x) __extension__ ({ __uint16_t __x =3D x;
> (__uint16_t)(__x << 8 | __x >> 8); })
>=20
> clang now produces "rolw $8,x" when x is volatile. This seems to
> violate volatile semantics. gcc produces load-rolw-store then. Both
> produce "rolw $8,x" when x is not volatile.
I'll have a closer look at the patch tomorrow, but the Intel
documentation for the bswap instruction recommends to use xchg for
bswap16.
--nextPart1956764.VNK7ZdMx6j
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)
iF4EABEIAAYFAk9P/DwACgkQfoCS2CCgtisrzAD8CaKRjS8jxXFheUkmozl9Jftg
JcGmz5jhnUHDF8n+jwcA/2m5oLN7xxWOJUFlsK4bf4aiaa99HYRcs5ZUwnlkgzwA
=Zegu
-----END PGP SIGNATURE-----
--nextPart1956764.VNK7ZdMx6j--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201203012346.20648.tijl>