[PATCH 1/6] make byte swapping macros less opaque to the optimizer

[Jeremy Huddleston]

For 1,2,4,5:
Reviewed-by: Jeremy Huddleston <jeremyhu at apple.com>
Tested-by: Jeremy Huddleston <jeremyhu at apple.com>

For 3,6:
Reviewed-by: Jeremy Huddleston <jeremyhu at apple.com>


On Apr 27, 2011, at 16:33, Matt Turner wrote:

> Text size reduction of ~33k (1.8%):
>   text	   data	    bss	    dec	    hex	filename
> 1879739	  72212	  55328	2007279	 1ea0ef	hw/xfree86/Xorg.before
> 1845291	  72212	  55328	1972831	 1e1a5f	hw/xfree86/Xorg
> 
> On x86[-64], the number of bswap instructions goes up from 5 to 1006.
> 
> By defining lswapl as gcc's builtin bswap32, the number of bswap
> instructions used goes up to 1025, but the text and binary sizes stay
> the same. This tells me that either gcc is
> 	- dumb, and unable to optimize away __builtin_bswap32 when it should
> 	- smart, and able to optimize away the open-coded bswap
> 
> Either way, it doesn't matter much.
> 
> Signed-off-by: Matt Turner <mattst88 at gmail.com>
> ---
> 
> The follow-up patch to this which removes the temporary variables used
> by the swap macros is too big for the mailing list, so please review it
> here:
> http://cgit.freedesktop.org/~mattst88/xserver/commit/?id=11ff6813cf2f2a04243f4eecb60078d77fc973ec
> 
> I have compile tested ephyr and dmx, but I wasn't able to compile test
> XQuartz, so please give it a test.
> 
> include/misc.h |   31 ++++++++-----------------------
> 1 files changed, 8 insertions(+), 23 deletions(-)
> 
> diff --git a/include/misc.h b/include/misc.h
> index 803f5ba..b7a3fd2 100644
> --- a/include/misc.h
> +++ b/include/misc.h
> @@ -240,32 +240,17 @@ xstrtokenize(const char *str, const char* separators);
> #define SwapRestL(stuff) \
>     SwapLongs((CARD32 *)(stuff + 1), LengthRestL(stuff))
> 
> -/* byte swap a 32-bit value */
> -#define swapl(x, n) { \
> -		 n = ((char *) (x))[0];\
> -		 ((char *) (x))[0] = ((char *) (x))[3];\
> -		 ((char *) (x))[3] = n;\
> -		 n = ((char *) (x))[1];\
> -		 ((char *) (x))[1] = ((char *) (x))[2];\
> -		 ((char *) (x))[2] = n; }
> -
> -/* byte swap a short */
> -#define swaps(x, n) { \
> -		 n = ((char *) (x))[0];\
> -		 ((char *) (x))[0] = ((char *) (x))[1];\
> -		 ((char *) (x))[1] = n; }
> -
> /* copy 32-bit value from src to dst byteswapping on the way */
> -#define cpswapl(src, dst) { \
> -                 ((char *)&(dst))[0] = ((char *) &(src))[3];\
> -                 ((char *)&(dst))[1] = ((char *) &(src))[2];\
> -                 ((char *)&(dst))[2] = ((char *) &(src))[1];\
> -                 ((char *)&(dst))[3] = ((char *) &(src))[0]; }
> +#define cpswapl(src, dst) (dst) = lswapl((src))
> 
> /* copy short from src to dst byteswapping on the way */
> -#define cpswaps(src, dst) { \
> -		 ((char *) &(dst))[0] = ((char *) &(src))[1];\
> -		 ((char *) &(dst))[1] = ((char *) &(src))[0]; }
> +#define cpswaps(src, dst) (dst) = lswaps((src))
> +
> +/* byte swap a 32-bit value */
> +#define swapl(x, n) (*(x)) = lswapl(*(x))
> +
> +/* byte swap a short */
> +#define swaps(x, n) (*(x)) = lswaps(*(x))
> 
> extern _X_EXPORT void SwapLongs(
>     CARD32 *list,
> -- 
> 1.7.3.4
> 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>