Re: [Xen-devel] [PATCH 03/11] Add endian, byteswap, and wordsize macros to mini-os

[George Dunlap <George.Dunlap@xxxxxxxxxxxxx>]

On Thu, Sep 27, 2012 at 6:09 PM, Matthew Fioravante
<matthew.fioravante@xxxxxxxxxx> wrote:
> This patch addes byte swapping macros and endian support to mini-os.
>
> Signed-off-by: Matthew Fioravante <matthew.fioravante@xxxxxxxxxx>
> Acked-by: Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>
>
[snip]
> diff --git a/extras/mini-os/include/byteswap.h 
> b/extras/mini-os/include/byteswap.h
> index 46821ae..992c8bd 100644
> --- a/extras/mini-os/include/byteswap.h
> +++ b/extras/mini-os/include/byteswap.h
> @@ -4,30 +4,36 @@
>  /* Unfortunately not provided by newlib.  */
>
>  #include <mini-os/types.h>
> -static inline uint16_t bswap_16(uint16_t x)
> -{
> -    return
> -    ((((x) & 0xff00) >> 8) | (((x) & 0xff) << 8));
> -}
> -
> -static inline uint32_t bswap_32(uint32_t x)
> -{
> -    return
> -    ((((x) & 0xff000000) >> 24) | (((x) & 0x00ff0000) >>  8) |
> -     (((x) & 0x0000ff00) <<  8) | (((x) & 0x000000ff) << 24));
> -}
> -
> -static inline uint64_t bswap_64(uint64_t x)
> -{
> -    return
> -    ((((x) & 0xff00000000000000ULL) >> 56) |
> -     (((x) & 0x00ff000000000000ULL) >> 40) |
> -     (((x) & 0x0000ff0000000000ULL) >> 24) |
> -     (((x) & 0x000000ff00000000ULL) >>  8) |
> -     (((x) & 0x00000000ff000000ULL) <<  8) |
> -     (((x) & 0x0000000000ff0000ULL) << 24) |
> -     (((x) & 0x000000000000ff00ULL) << 40) |
> -     (((x) & 0x00000000000000ffULL) << 56));
> -}
> +
> +#define bswap_16(x) ((uint16_t)(                         \
> +                (((uint16_t)(x) & (uint16_t)0x00ffU) << 8) |                 
>  \
> +                (((uint16_t)(x) & (uint16_t)0xff00U) >> 8)))
> +
> +/* Use gcc optimized versions if they exist */
> +#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 3)
> +#define bswap_32(v) __builtin_bswap32(v)
> +#define bswap_64(v) __builtin_bswap64(v)
> +#else
> +
> +#define bswap_32(x) ((uint32_t)(                         \
> +                (((uint32_t)(x) & (uint32_t)0x000000ffUL) << 24) |           
>  \
> +                (((uint32_t)(x) & (uint32_t)0x0000ff00UL) <<  8) |           
>  \
> +                (((uint32_t)(x) & (uint32_t)0x00ff0000UL) >>  8) |           
>  \
> +                (((uint32_t)(x) & (uint32_t)0xff000000UL) >> 24)))
> +
> +#define bswap_64(x) ((uint64_t)(                         \
> +                (((uint64_t)(x) & (uint64_t)0x00000000000000ffULL) << 56) |  
>  \
> +                (((uint64_t)(x) & (uint64_t)0x000000000000ff00ULL) << 40) |  
>  \
> +                (((uint64_t)(x) & (uint64_t)0x0000000000ff0000ULL) << 24) |  
>  \
> +                (((uint64_t)(x) & (uint64_t)0x00000000ff000000ULL) <<  8) |  
>  \
> +                (((uint64_t)(x) & (uint64_t)0x000000ff00000000ULL) >>  8) |  
>  \
> +                (((uint64_t)(x) & (uint64_t)0x0000ff0000000000ULL) >> 24) |  
>  \
> +                (((uint64_t)(x) & (uint64_t)0x00ff000000000000ULL) >> 40) |  
>  \
> +                (((uint64_t)(x) & (uint64_t)0xff00000000000000ULL) >> 56)))
> +
> +#endif

I think it would be worth adding to the commit message your rationale
for changing from static inline to #defines, which you gave in
response to Samuel in the last iteration of this patch.

[snip]

> diff --git a/extras/mini-os/include/ia64/arch_endian.h 
> b/extras/mini-os/include/ia64/arch_endian.h
> new file mode 100644
> index 0000000..0771683
> --- /dev/null
> +++ b/extras/mini-os/include/ia64/arch_endian.h
> @@ -0,0 +1,7 @@
> +#ifndef        ARCH_ENDIAN_H
> +#error "Do not include arch_endian by itself, include endian.h"
> +#else
> +
> +#define __BYTE_ORDER __LITTLE_ENDIAN
> +
> +#endif
> diff --git a/extras/mini-os/include/ia64/arch_wordsize.h 
> b/extras/mini-os/include/ia64/arch_wordsize.h
> new file mode 100644
> index 0000000..1b5a00f
> --- /dev/null
> +++ b/extras/mini-os/include/ia64/arch_wordsize.h
> @@ -0,0 +1 @@
> +#define __WORDSIZE 64

Again, is there a reason to include the ia64 files?

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel