Has this been tested on actual problematic hardware, and improved
performance? I remember last time this was discussed, the outcome was
not always what was expected.
Christophe
On Mon, Sep 18, 2017 at 02:52:57AM -0400, Frediano Ziglio wrote:
> 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@xxxxxxxxxx>
> > ---
> > 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]:
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel