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

[Tijl Coosemans]

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.
-------------- 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-head/attachments/20120321/df9b1f46/attachment.pgp