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.
signature.asc
Description: This is a digitally signed message part.
