[Spice-devel] [PATCH spice-common v2 1/3] Make the compiler work out better way to write unaligned memory

[Christophe Fergeau]

On Fri, Sep 22, 2017 at 11:10:02AM +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.
> A packed structure tells the compiler to not leave padding inside it
> and that the structure can be unaligned so any field can be unaligned
> having to generate proper access code based on architecture.
> For instance ARM7 can use unaligned access but not for 64 bit
> numbers (currently these accesses are emulated by Linux kernel
> with obvious performance consequences).
> 
> This changes the current 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_t *)(ptr)))
> #define write_uint32(ptr, val) (*((uint32_t *)(ptr))) = val
> #endif
> 
> to:
> 
> #include <spice/start-packed.h>
> typedef struct SPICE_ATTR_PACKED {
>     uint32_t v;
> } uint32_unaligned_t;
> #include <spice/end-packed.h>
> 
> #ifdef WORDS_BIGENDIAN
> #define read_uint32(ptr) ((uint32_t)SPICE_BYTESWAP32(((uint32_unaligned_t *)(ptr))->v))

For int32, this generates
#define read_uint32(ptr) ((int32_t)SPICE_BYTESWAP32(((uint32_unaligned_t *)(ptr))->v))
rather than
#define read_uint32(ptr) ((int32_t)SPICE_BYTESWAP32(((int32_unaligned_t *)(ptr))->v))
I don't know if this is intentional?
(the change would be:

diff --git a/python_modules/demarshal.py b/python_modules/demarshal.py
index da87d44..241e620 100644
--- a/python_modules/demarshal.py
+++ b/python_modules/demarshal.py
@@ -63,7 +63,7 @@ def write_parser_helpers(writer):
             utype = "uint%d" % (size)
             type = "%sint%d" % (sign, size)
             swap = "SPICE_BYTESWAP%d" % size
-            writer.macro("read_%s" % type, "ptr", "((%s_t)%s(((%s_unaligned_t *)(ptr))->v))" % (type, swap, utype))
+            writer.macro("read_%s" % type, "ptr", "((%s_t)%s(((%s_unaligned_t *)(ptr))->v))" % (type, swap, type))
             writer.macro("write_%s" % type, "ptr, val", "((%s_unaligned_t *)(ptr))->v = %s((%s_t)val)" % (utype, swap, utype))
     writer.writeln("#else")
     for size in [16, 32, 64]:

Apart from this, looks good.

Christophe