diff mbox series

[for-8.0,15/29] include/qemu/int128: Add vector type to Int128Alias

Message ID 20221118094754.242910-16-richard.henderson@linaro.org
State New
Headers show
Series tcg: Improve atomicity support | expand

Commit Message

Richard Henderson Nov. 18, 2022, 9:47 a.m. UTC
Adding a vector type will make it easier to handle i386
have_atomic16 via AVX.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/qemu/int128.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Philippe Mathieu-Daudé Nov. 21, 2022, 11:45 p.m. UTC | #1
On 18/11/22 10:47, Richard Henderson wrote:
> Adding a vector type will make it easier to handle i386
> have_atomic16 via AVX.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/qemu/int128.h | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Philippe Mathieu-Daudé Nov. 22, 2022, 6:21 p.m. UTC | #2
On 18/11/22 10:47, Richard Henderson wrote:
> Adding a vector type will make it easier to handle i386
> have_atomic16 via AVX.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/qemu/int128.h | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/qemu/int128.h b/include/qemu/int128.h
> index f62a46b48c..f29f90e6f4 100644
> --- a/include/qemu/int128.h
> +++ b/include/qemu/int128.h
> @@ -479,16 +479,16 @@ static inline void bswap128s(Int128 *s)
>   /*
>    * When compiler supports a 128-bit type, define a combination of
>    * a possible structure and the native types.  Ease parameter passing
> - * via use of the transparent union extension.
> + * via use of the transparent union extension.  Provide a vector type
> + * for use in atomicity on some hosts.
>    */
> -#ifdef CONFIG_INT128
>   typedef union {
>       Int128 s;
> +    uint64_t v __attribute__((vector_size(16)));
> +#ifdef CONFIG_INT128
>       __int128_t i;
>       __uint128_t u;
> -} Int128Alias __attribute__((transparent_union));
> -#else
> -typedef Int128 Int128Alias;
>   #endif /* CONFIG_INT128 */
> +} Int128Alias __attribute__((transparent_union));
>   
>   #endif /* INT128_H */

This triggers a warning with GCC:

include/qemu/int128.h:487:14: warning: alignment of field 'v' (128 bits) 
does not match the alignment of the first field in transparent union; 
transparent_union attribute ignored [-Wignored-attributes]
     uint64_t v __attribute__((vector_size(16)));
              ^
include/qemu/int128.h:486:12: note: alignment of first field is 64 bits
     Int128 s;
            ^
Meson:

Project version: 7.1.91
C compiler for the host machine: gcc-12 (gcc 12.2.0 "gcc-12 (Homebrew 
GCC 12.2.0) 12.2.0")
C linker for the host machine: gcc-12 ld64 819.6
Host machine cpu family: aarch64
Host machine cpu: arm64
Philippe Mathieu-Daudé Nov. 22, 2022, 6:31 p.m. UTC | #3
On 22/11/22 19:21, Philippe Mathieu-Daudé wrote:
> On 18/11/22 10:47, Richard Henderson wrote:
>> Adding a vector type will make it easier to handle i386
>> have_atomic16 via AVX.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   include/qemu/int128.h | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/qemu/int128.h b/include/qemu/int128.h
>> index f62a46b48c..f29f90e6f4 100644
>> --- a/include/qemu/int128.h
>> +++ b/include/qemu/int128.h
>> @@ -479,16 +479,16 @@ static inline void bswap128s(Int128 *s)
>>   /*
>>    * When compiler supports a 128-bit type, define a combination of
>>    * a possible structure and the native types.  Ease parameter passing
>> - * via use of the transparent union extension.
>> + * via use of the transparent union extension.  Provide a vector type
>> + * for use in atomicity on some hosts.
>>    */
>> -#ifdef CONFIG_INT128
>>   typedef union {
>>       Int128 s;
>> +    uint64_t v __attribute__((vector_size(16)));
>> +#ifdef CONFIG_INT128
>>       __int128_t i;
>>       __uint128_t u;
>> -} Int128Alias __attribute__((transparent_union));
>> -#else
>> -typedef Int128 Int128Alias;
>>   #endif /* CONFIG_INT128 */
>> +} Int128Alias __attribute__((transparent_union));
>>   #endif /* INT128_H */
> 
> This triggers a warning with GCC:

Ah no, looking closer, even configured as ''--cc=gcc-12 --host-cc=gcc-12
--cxx=/bin/false', Clang got selected for ObjC, and this warning comes
from it:

Compiling Objective-C object libcommon.fa.p/ui_cocoa.m.o
In file included from ../../ui/cocoa.m:36:
In file included from include/sysemu/sysemu.h:5:
In file included from include/qemu/timer.h:4:
In file included from include/qemu/bitops.h:16:
In file included from include/qemu/host-utils.h:35:

> include/qemu/int128.h:487:14: warning: alignment of field 'v' (128 bits) 
> does not match the alignment of the first field in transparent union; 
> transparent_union attribute ignored [-Wignored-attributes]
>      uint64_t v __attribute__((vector_size(16)));
>               ^
> include/qemu/int128.h:486:12: note: alignment of first field is 64 bits
>      Int128 s;
>             ^
> Meson:
> 
> Project version: 7.1.91
> C compiler for the host machine: gcc-12 (gcc 12.2.0 "gcc-12 (Homebrew 
> GCC 12.2.0) 12.2.0")
> C linker for the host machine: gcc-12 ld64 819.6
> Host machine cpu family: aarch64
> Host machine cpu: arm64

Objective-C compiler for the host machine: clang (clang 14.0.0)
Objective-C linker for the host machine: clang ld64 819.6

Regards,

Phil.
diff mbox series

Patch

diff --git a/include/qemu/int128.h b/include/qemu/int128.h
index f62a46b48c..f29f90e6f4 100644
--- a/include/qemu/int128.h
+++ b/include/qemu/int128.h
@@ -479,16 +479,16 @@  static inline void bswap128s(Int128 *s)
 /*
  * When compiler supports a 128-bit type, define a combination of
  * a possible structure and the native types.  Ease parameter passing
- * via use of the transparent union extension.
+ * via use of the transparent union extension.  Provide a vector type
+ * for use in atomicity on some hosts.
  */
-#ifdef CONFIG_INT128
 typedef union {
     Int128 s;
+    uint64_t v __attribute__((vector_size(16)));
+#ifdef CONFIG_INT128
     __int128_t i;
     __uint128_t u;
-} Int128Alias __attribute__((transparent_union));
-#else
-typedef Int128 Int128Alias;
 #endif /* CONFIG_INT128 */
+} Int128Alias __attribute__((transparent_union));
 
 #endif /* INT128_H */