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(-)
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 <thuth@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(-)
diff --git a/host/include/generic/host/atomic128-cas.h b/host/include/generic/host/atomic128-cas.h
index 991d3da082..6b40cc2271 100644
--- a/host/include/generic/host/atomic128-cas.h
+++ b/host/include/generic/host/atomic128-cas.h
@@ -28,7 +28,7 @@ atomic16_cmpxchg(Int128 *ptr, Int128 cmp, Int128 new)
static inline Int128 ATTRIBUTE_ATOMIC128_OPT
atomic16_cmpxchg(Int128 *ptr, Int128 cmp, Int128 new)
{
- __int128_t *ptr_align = __builtin_assume_aligned(ptr, 16);
+ Int128Aligned *ptr_align = __builtin_assume_aligned(ptr, 16);
Int128Alias r, c, n;
c.s = cmp;
diff --git a/host/include/generic/host/atomic128-ldst.h b/host/include/generic/host/atomic128-ldst.h
index 80fff0643a..691e6a8531 100644
--- a/host/include/generic/host/atomic128-ldst.h
+++ b/host/include/generic/host/atomic128-ldst.h
@@ -58,7 +58,7 @@ atomic16_read_rw(Int128 *ptr)
static inline void ATTRIBUTE_ATOMIC128_OPT
atomic16_set(Int128 *ptr, Int128 val)
{
- __int128_t *ptr_align = __builtin_assume_aligned(ptr, 16);
+ Int128Aligned *ptr_align = __builtin_assume_aligned(ptr, 16);
__int128_t old;
Int128Alias new;
diff --git a/include/qemu/int128.h b/include/qemu/int128.h
index 73624e8be7..44530d3e10 100644
--- a/include/qemu/int128.h
+++ b/include/qemu/int128.h
@@ -10,6 +10,7 @@
*/
#if defined(CONFIG_INT128) && !defined(CONFIG_TCG_INTERPRETER)
typedef __int128_t Int128;
+typedef __int128_t __attribute__((aligned(16))) Int128Aligned;
static inline Int128 int128_make64(uint64_t a)
{
--
2.41.0On 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 <thuth@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.henderson@linaro.org> r~
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 <thuth@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.henderson@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
On 13/11/23 11:34, Thomas Huth wrote: > 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 <thuth@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.henderson@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; Indeed. > > /* > * We guarantee that the in-memory byte representation of an > > I'll add that when picking up the patch. LGTM.
On 8/11/23 09: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 <thuth@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: Philippe Mathieu-Daudé <philmd@linaro.org>
© 2016 - 2024 Red Hat, Inc.