svn commit: r232266 - in head/sys: amd64/include i386/include pc98/include x86/include

[Tijl Coosemans]

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():
> 
> A minor problem with only having a macro version for __bswap64() turned
> up:
> 
>> % -#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)))
> 
> When x a non-volatile variable, gcc and clang produce the good code
> "rolw $8,x" for "x = __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.
> 
> So any macro version must use gcc features to be safe.  The following
> seems to work:
> 
> #define	__bswap16(x)	__extension__ ({ __uint16_t __x = x;
>  	(__uint16_t)(__x << 8 | __x >> 8); })
> 
> 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 228 bytes
Desc: This is a digitally signed message part.
Url : http://lists.freebsd.org/pipermail/svn-src-all/attachments/20120301/588a4957/attachment.pgp