* Re: [Qemu-devel] [PATCH] hw/display/qxl: Suppress clang-7 warning about misaligned atomic operation
[not found] <20180927155538.699-1-peter.maydell@linaro.org>
@ 2018-10-05 9:24 ` Gerd Hoffmann
2018-10-05 14:13 ` Eric Blake
1 sibling, 0 replies; 2+ messages in thread
From: Gerd Hoffmann @ 2018-10-05 9:24 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, patches, Paolo Bonzini, Philippe Mathieu-Daudé
On Thu, Sep 27, 2018 at 04:55:38PM +0100, Peter Maydell wrote:
> If QEMU is compiled with clang-7 it results in the warning:
>
> hw/display/qxl.c:1884:19: error: misaligned or large atomic operation
> may incur significant performance penalty [-Werror,-Watomic-alignment]
> old_pending = atomic_fetch_or(&d->ram->int_pending, le_events);
> ^
>
> This is because the Spice headers forgot to define the QXLRam struct
> with the '__aligned__(4)' attribute. clang 7 and newer will thus
> warn that the access here to int_pending might not be 4-aligned
> (because the QXLRam object d->ram points at might start at a
> misaligned address). In fact we set up d->ram in init_qxl_ram() so
> it always starts at a 4K boundary, so we know the atomic access here
> is OK.
>
> Newer Spice versions (with Spice commit
> beda5ec7a6848be20c0cac2a9a8ef2a41e8069c1) will fix the bug;
> for older Spice versions, work around it by telling the compiler
> explicitly that the alignment is OK using __builtin_assume_aligned().
Added to vga queue.
cheers,
Gerd
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: [Qemu-devel] [PATCH] hw/display/qxl: Suppress clang-7 warning about misaligned atomic operation
[not found] <20180927155538.699-1-peter.maydell@linaro.org>
2018-10-05 9:24 ` [Qemu-devel] [PATCH] hw/display/qxl: Suppress clang-7 warning about misaligned atomic operation Gerd Hoffmann
@ 2018-10-05 14:13 ` Eric Blake
1 sibling, 0 replies; 2+ messages in thread
From: Eric Blake @ 2018-10-05 14:13 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Paolo Bonzini, Philippe Mathieu-Daudé, Gerd Hoffmann,
patches
On 9/27/18 10:55 AM, Peter Maydell wrote:
> Newer Spice versions (with Spice commit
> beda5ec7a6848be20c0cac2a9a8ef2a41e8069c1) will fix the bug;
> for older Spice versions, work around it by telling the compiler
> explicitly that the alignment is OK using __builtin_assume_aligned().
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> include/qemu/compiler.h | 9 +++++++++
> hw/display/qxl.c | 26 +++++++++++++++++++++++++-
> 2 files changed, 34 insertions(+), 1 deletion(-)
>
> +#ifdef HAS_ASSUME_ALIGNED
> +#define ALIGNED_UINT32_PTR(P) ((uint32_t *)__builtin_assume_aligned(P, 4))
> +#else
> +#define ALIGNED_UINT32_PTR(P) ((uint32_t *)P)
> +#endif
> +
> + old_pending = atomic_fetch_or(ALIGNED_UINT32_PTR(&d->ram->int_pending),
The only client of your macro is not impacted, but if you want to be
technically robust against all future uses of the macro, the fallback is
underparenthesized, and would be safer as:
#define ALIGNED_UINT32_PTR(P) ((uint32_t *)(P))
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 2+ messages in thread