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

[Frediano Ziglio]

ping

I think I sent a reply after the new path which could be confusing.

> 
> 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))
> #define write_uint32(ptr, val) ((uint32_unaligned_t *)(ptr))->v =
> SPICE_BYTESWAP32((uint32_t)val)
> #else
> #define read_uint32(ptr) (((uint32_unaligned_t *)(ptr))->v)
> #define write_uint32(ptr, val) (((uint32_unaligned_t *)(ptr))->v) = val
> #endif
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  python_modules/demarshal.py | 33 +++++++++++++++++++++++----------
>  1 file changed, 23 insertions(+), 10 deletions(-)
> 
> Changes since v1:
> - improved commit message;
> - added SPICE_ATTR_PACKED to structure definition for compatibility;
> - use intX_unaligned_t instead of intX_unaligned_p.
> 
> diff --git a/python_modules/demarshal.py b/python_modules/demarshal.py
> index 1ea131d..da87d44 100644
> --- a/python_modules/demarshal.py
> +++ b/python_modules/demarshal.py
> @@ -40,24 +40,37 @@ def write_parser_helpers(writer):
>  
>      writer = writer.function_helper()
>  
> +    writer.writeln("#include <spice/start-packed.h>")
> +    for size in [16, 32, 64]:
> +        for sign in ["", "u"]:
> +            type = "%sint%d" % (sign, size)
> +            writer.begin_block("typedef struct SPICE_ATTR_PACKED")
> +            writer.variable_def("%s_t" % type, "v")
> +            writer.end_block(newline=False)
> +            writer.writeln(" %s_unaligned_t;" % type)
> +    writer.writeln("#include <spice/end-packed.h>")
> +    writer.newline()
> +
> +    for sign in ["", "u"]:
> +        type = "%sint8" % sign
> +        writer.macro("read_%s" % type, "ptr", "(*((%s_t *)(ptr)))" % type)
> +        writer.macro("write_%s" % type, "ptr, val", "*(%s_t *)(ptr) = val" %
> (type))
> +    writer.newline()
> +
>      writer.writeln("#ifdef WORDS_BIGENDIAN")
> -    for size in [8, 16, 32, 64]:
> +    for size in [16, 32, 64]:
>          for sign in ["", "u"]:
>              utype = "uint%d" % (size)
>              type = "%sint%d" % (sign, size)
>              swap = "SPICE_BYTESWAP%d" % size
> -            if size == 8:
> -                writer.macro("read_%s" % type, "ptr", "(*((%s_t *)(ptr)))" %
> type)
> -                writer.macro("write_%s" % type, "ptr, val", "*(%s_t *)(ptr)
> = val" % (type))
> -            else:
> -                writer.macro("read_%s" % type, "ptr", "((%s_t)%s(*((%s_t
> *)(ptr))))" % (type, swap, utype))
> -                writer.macro("write_%s" % type, "ptr, val", "*(%s_t *)(ptr)
> = %s((%s_t)val)" % (utype, swap, utype))
> +            writer.macro("read_%s" % type, "ptr",
> "((%s_t)%s(((%s_unaligned_t *)(ptr))->v))" % (type, swap, utype))
> +            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 [8, 16, 32, 64]:
> +    for size in [16, 32, 64]:
>          for sign in ["", "u"]:
>              type = "%sint%d" % (sign, size)
> -            writer.macro("read_%s" % type, "ptr", "(*((%s_t *)(ptr)))" %
> type)
> -            writer.macro("write_%s" % type, "ptr, val", "(*((%s_t *)(ptr)))
> = val" % type)
> +            writer.macro("read_%s" % type, "ptr", "(((%s_unaligned_t
> *)(ptr))->v)" % type)
> +            writer.macro("write_%s" % type, "ptr, val", "(((%s_unaligned_t
> *)(ptr))->v) = val" % type)
>      writer.writeln("#endif")
>  
>      for size in [8, 16, 32, 64]: