>
> 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?
Yes, intentional and consistent with previous code.
The usage of unsigned for BYTESWAP avoid sign extension during arithmetic.
> (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
>
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
