[Spice-devel] [PATCH spice-common] codegen: Make the compiler work about better way to write unaligned memory

[Frediano Ziglio]

> 
> On Mon, Sep 11, 2017 at 11:13:04AM +0100, Frediano Ziglio wrote:
> > Instead of assuming that the system can safely do unaligned access
> > to memory use packed structures to allow the compiler generate
> > best code possible.
> 
> It will generate the best code possible, but assuming all accesses are
> unaligned I believe? Ie a perfectly fine 32 bit aligned field may still
> be read using byte-sized accesses if the arch/cpu does not support
> unaligned 32 bit reads?
> 

Yes. A packed structure tells the compiler that the structure can be
not aligned so any field can be unaligned and compiler will read/write
the fields byte by byte if needed.

> The big missing part in this commit log is what this commit is doing in
> human readable form...



> This changes the current read_intX methods from:
> 
> #ifdef WORDS_BIGENDIAN
> #define read_uint32(ptr) ((uint32_t)SPICE_BYTESWAP32(*((uint32_t *)(ptr))))
> #define write_uint32(ptr, val) *(uint32_t *)(ptr) =
> SPICE_BYTESWAP32((uint32_t)val)
> #else
> #define read_uint32(ptr) (((uint32_unaligned_p)(ptr))->v)
> #define read_uint32(ptr) (*((uint32_t *)(ptr)))
> #endif
> 
> to:
> #include <spice/start-packed.h>
> typedef struct {
>     uint32_t v;
> } *uint32_unaligned_p;
> include <spice/end-packed.h>
> 
> #ifdef WORDS_BIGENDIAN
> #define read_uint32(ptr)
> ((uint32_t)SPICE_BYTESWAP32(((uint32_unaligned_p)(ptr))->v))
> #define write_uint32(ptr, val) ((uint32_unaligned_p)(ptr))->v =
> SPICE_BYTESWAP32((uint32_t)val)
> #else
> #define read_uint32(ptr) (((uint32_unaligned_p)(ptr))->v)
> #define write_uint32(ptr, val) (((uint32_unaligned_p)(ptr))->v) = val
> #endif
> 

Should I copy this section entirely in the commit message?

> The struct is missing SPICE_ATTR_PACKED, and I think I prefer typedef
> struct {} uint32_unaligned_t;
> and then that the casts become ((uint32_unaligned_t *)(ptr))->v
> 

Changed, forgot the SPICE_ATTR_PACKED

> Christophe
> 

Frediano