svn commit: r232721 - head/sys/x86/include

[Tijl Coosemans]

On Tuesday 20 March 2012 13:34:10 John Baldwin wrote:
> On Friday, March 09, 2012 6:48:56 am Tijl Coosemans wrote:
>> Author: tijl
>> Date: Fri Mar  9 11:48:56 2012
>> New Revision: 232721
>> URL: http://svn.freebsd.org/changeset/base/232721
>> 
>> Log:
>>   Clean up x86 endian.h:
>>   - Remove extern "C". There are no functions with external linkage here. [1]
>>   - Rename bswapNN_const(x) to bswapNN_gen(x) to indicate that these macros
>>     are generic implementations that can take non-constant arguments. [1]
>>   - Split up __GNUCLIKE_ASM && __GNUCLIKE_BUILTIN_CONSTANT_P and deal with
>>     each separately.
>>   - Replace _LP64 with __amd64__ because asm instructions are machine
>>     dependent, not ABI dependent.
>>   
>>   Submitted by:	bde [1]
>>   Reviewed by:	bde
> 
> 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.
-------------- 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/20120320/dec6f63e/attachment.pgp