On 08/11/2023 18.57, Richard Henderson wrote:
On 11/8/23 00:59, Thomas Huth wrote:
When compiling QEMU with Clang 17 on a s390x, the compilation fails:

In file included from ../accel/tcg/cputlb.c:32:
In file included from /root/qemu/include/exec/helper-proto-common.h:10:
In file included from /root/qemu/include/qemu/atomic128.h:62:
/root/qemu/host/include/generic/host/atomic128-ldst.h:68:15: error:
  __sync builtin operation MUST have natural alignment (consider using __
atomic). [-Werror,-Wsync-alignment]
    68 |     } while (!__sync_bool_compare_and_swap_16(ptr_align, old, new.i));
       |               ^
In file included from ../accel/tcg/cputlb.c:32:
In file included from /root/qemu/include/exec/helper-proto-common.h:10:
In file included from /root/qemu/include/qemu/atomic128.h:61:
/root/qemu/host/include/generic/host/atomic128-cas.h:36:11: error:
  __sync builtin operation MUST have natural alignment (consider using __a
tomic). [-Werror,-Wsync-alignment]
    36 |     r.i = __sync_val_compare_and_swap_16(ptr_align, c.i, n.i);
       |           ^
2 errors generated.

It's arguably a bug in Clang since we already use __builtin_assume_aligned()
to tell the compiler that the pointer is properly aligned. But according to
https://github.com/llvm/llvm-project/issues/69146 it seems like the Clang
folks don't see an easy fix on their side and recommend to use a type
declared with __attribute__((aligned(16))) to work around this problem.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1934
Signed-off-by: Thomas Huth <th...@redhat.com>
---
  host/include/generic/host/atomic128-cas.h  | 2 +-
  host/include/generic/host/atomic128-ldst.h | 2 +-
  include/qemu/int128.h                      | 1 +
  3 files changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.hender...@linaro.org>

I just noticed that this new type needs to be declared for the #else part, too, otherwise the compilation breaks with --enable-tcg-interpreter :

diff --git a/include/qemu/int128.h b/include/qemu/int128.h
--- a/include/qemu/int128.h
+++ b/include/qemu/int128.h
@@ -225,6 +225,7 @@ static inline Int128 int128_rems(Int128 a, Int128 b)
 #else /* !CONFIG_INT128 */

 typedef struct Int128 Int128;
+typedef struct Int128 __attribute__((aligned(16))) Int128Aligned;

 /*
  * We guarantee that the in-memory byte representation of an

I'll add that when picking up the patch.

 Thomas


Reply via email to