[PATCH 0/6] host/i386: allow configuring the x86-64 baseline

[PATCH 0/6] host/i386: allow configuring the x86-64 baseline

[Paolo Bonzini]

As discussed, add a Meson option to configure which x86-64 instruction
set to use.  QEMU will now default to x86-64-v1 + cmpxchg16b for
64-bit builds (that corresponds to a Pentium 4 for 32-bit builds).

The baseline can be tuned down to Pentium Pro for 32-bit builds (with
-Dx86_version=0), or up as desired.

Patch "host/i386: assume presence of CMOV" is not reverted because
CMOV appeared first in the Pentium Pro.

Paolo

Paolo Bonzini (6):
  Revert "host/i386: assume presence of POPCNT"
  Revert "host/i386: assume presence of SSSE3"
  Revert "host/i386: assume presence of SSE2"
  meson: allow configuring the x86-64 baseline
  meson: remove dead optimization option
  meson: require compiler support for chosen x86-64 instructions

 meson.build                      | 56 ++++++++++++++++++++------------
 host/include/i386/host/cpuinfo.h |  2 ++
 tcg/i386/tcg-target.h            |  5 +--
 util/bufferiszero.c              |  4 +--
 util/cpuinfo-i386.c              |  6 ++--
 meson_options.txt                |  5 +--
 scripts/meson-buildoptions.sh    |  3 ++
 7 files changed, 52 insertions(+), 29 deletions(-)

-- 
2.45.2

[PATCH 1/6] Revert "host/i386: assume presence of POPCNT"

[Paolo Bonzini]

This reverts commit 45ccdbcb24baf99667997fac5cf60318e5e7db51.
The x86-64 instruction set can now be tuned down to x86-64 v1
or i386 Pentium Pro.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 host/include/i386/host/cpuinfo.h | 1 +
 tcg/i386/tcg-target.h            | 5 +++--
 util/cpuinfo-i386.c              | 1 +
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/host/include/i386/host/cpuinfo.h b/host/include/i386/host/cpuinfo.h
index c1e94d75ce1..72f6fad61e5 100644
--- a/host/include/i386/host/cpuinfo.h
+++ b/host/include/i386/host/cpuinfo.h
@@ -11,6 +11,7 @@
 #define CPUINFO_ALWAYS          (1u << 0)  /* so cpuinfo is nonzero */
 #define CPUINFO_MOVBE           (1u << 2)
 #define CPUINFO_LZCNT           (1u << 3)
+#define CPUINFO_POPCNT          (1u << 4)
 #define CPUINFO_BMI1            (1u << 5)
 #define CPUINFO_BMI2            (1u << 6)
 #define CPUINFO_AVX1            (1u << 9)
diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h
index ecc69827287..2f67a97e059 100644
--- a/tcg/i386/tcg-target.h
+++ b/tcg/i386/tcg-target.h
@@ -111,6 +111,7 @@ typedef enum {
 #endif
 
 #define have_bmi1         (cpuinfo & CPUINFO_BMI1)
+#define have_popcnt       (cpuinfo & CPUINFO_POPCNT)
 #define have_avx1         (cpuinfo & CPUINFO_AVX1)
 #define have_avx2         (cpuinfo & CPUINFO_AVX2)
 #define have_movbe        (cpuinfo & CPUINFO_MOVBE)
@@ -142,7 +143,7 @@ typedef enum {
 #define TCG_TARGET_HAS_nor_i32          0
 #define TCG_TARGET_HAS_clz_i32          1
 #define TCG_TARGET_HAS_ctz_i32          1
-#define TCG_TARGET_HAS_ctpop_i32        1
+#define TCG_TARGET_HAS_ctpop_i32        have_popcnt
 #define TCG_TARGET_HAS_deposit_i32      1
 #define TCG_TARGET_HAS_extract_i32      1
 #define TCG_TARGET_HAS_sextract_i32     1
@@ -177,7 +178,7 @@ typedef enum {
 #define TCG_TARGET_HAS_nor_i64          0
 #define TCG_TARGET_HAS_clz_i64          1
 #define TCG_TARGET_HAS_ctz_i64          1
-#define TCG_TARGET_HAS_ctpop_i64        1
+#define TCG_TARGET_HAS_ctpop_i64        have_popcnt
 #define TCG_TARGET_HAS_deposit_i64      1
 #define TCG_TARGET_HAS_extract_i64      1
 #define TCG_TARGET_HAS_sextract_i64     0
diff --git a/util/cpuinfo-i386.c b/util/cpuinfo-i386.c
index 8f2694d88f2..6d474a6259a 100644
--- a/util/cpuinfo-i386.c
+++ b/util/cpuinfo-i386.c
@@ -35,6 +35,7 @@ unsigned __attribute__((constructor)) cpuinfo_init(void)
         __cpuid(1, a, b, c, d);
 
         info |= (c & bit_MOVBE ? CPUINFO_MOVBE : 0);
+        info |= (c & bit_POPCNT ? CPUINFO_POPCNT : 0);
         info |= (c & bit_PCLMUL ? CPUINFO_PCLMUL : 0);
 
         /* NOTE: our AES support requires SSSE3 (PSHUFB) as well. */
-- 
2.45.2

[PATCH 2/6] Revert "host/i386: assume presence of SSSE3"

[Paolo Bonzini]

This reverts commit 433cd6d94a8256af70a5200f236dc8047c3c1468.
The x86-64 instruction set can now be tuned down to x86-64 v1
or i386 Pentium Pro.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 util/cpuinfo-i386.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/util/cpuinfo-i386.c b/util/cpuinfo-i386.c
index 6d474a6259a..ca74ef04f54 100644
--- a/util/cpuinfo-i386.c
+++ b/util/cpuinfo-i386.c
@@ -38,8 +38,8 @@ unsigned __attribute__((constructor)) cpuinfo_init(void)
         info |= (c & bit_POPCNT ? CPUINFO_POPCNT : 0);
         info |= (c & bit_PCLMUL ? CPUINFO_PCLMUL : 0);
 
-        /* NOTE: our AES support requires SSSE3 (PSHUFB) as well. */
-        info |= (c & bit_AES) ? CPUINFO_AES : 0;
+        /* Our AES support requires PSHUFB as well. */
+        info |= ((c & bit_AES) && (c & bit_SSSE3) ? CPUINFO_AES : 0);
 
         /* For AVX features, we must check available and usable. */
         if ((c & bit_AVX) && (c & bit_OSXSAVE)) {
-- 
2.45.2

[PATCH 3/6] Revert "host/i386: assume presence of SSE2"

[Paolo Bonzini]

This reverts commit b18236897ca15c3db1506d8edb9a191dfe51429c.
The x86-64 instruction set can now be tuned down to x86-64 v1
or i386 Pentium Pro.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 host/include/i386/host/cpuinfo.h | 1 +
 util/bufferiszero.c              | 4 ++--
 util/cpuinfo-i386.c              | 1 +
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/host/include/i386/host/cpuinfo.h b/host/include/i386/host/cpuinfo.h
index 72f6fad61e5..81771733eaa 100644
--- a/host/include/i386/host/cpuinfo.h
+++ b/host/include/i386/host/cpuinfo.h
@@ -14,6 +14,7 @@
 #define CPUINFO_POPCNT          (1u << 4)
 #define CPUINFO_BMI1            (1u << 5)
 #define CPUINFO_BMI2            (1u << 6)
+#define CPUINFO_SSE2            (1u << 7)
 #define CPUINFO_AVX1            (1u << 9)
 #define CPUINFO_AVX2            (1u << 10)
 #define CPUINFO_AVX512F         (1u << 11)
diff --git a/util/bufferiszero.c b/util/bufferiszero.c
index 11c080e02cf..74864f7b782 100644
--- a/util/bufferiszero.c
+++ b/util/bufferiszero.c
@@ -188,14 +188,14 @@ static biz_accel_fn const accel_table[] = {
 
 static unsigned best_accel(void)
 {
-#ifdef CONFIG_AVX2_OPT
     unsigned info = cpuinfo_init();
 
+#ifdef CONFIG_AVX2_OPT
     if (info & CPUINFO_AVX2) {
         return 2;
     }
 #endif
-    return 1;
+    return info & CPUINFO_SSE2 ? 1 : 0;
 }
 
 #elif defined(__aarch64__) && defined(__ARM_NEON)
diff --git a/util/cpuinfo-i386.c b/util/cpuinfo-i386.c
index ca74ef04f54..90f92a42dc8 100644
--- a/util/cpuinfo-i386.c
+++ b/util/cpuinfo-i386.c
@@ -34,6 +34,7 @@ unsigned __attribute__((constructor)) cpuinfo_init(void)
     if (max >= 1) {
         __cpuid(1, a, b, c, d);
 
+        info |= (d & bit_SSE2 ? CPUINFO_SSE2 : 0);
         info |= (c & bit_MOVBE ? CPUINFO_MOVBE : 0);
         info |= (c & bit_POPCNT ? CPUINFO_POPCNT : 0);
         info |= (c & bit_PCLMUL ? CPUINFO_PCLMUL : 0);
-- 
2.45.2

[PATCH 4/6] meson: allow configuring the x86-64 baseline

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build                   | 41 ++++++++++++++++++++++++++++-------
 meson_options.txt             |  3 +++
 scripts/meson-buildoptions.sh |  3 +++
 3 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/meson.build b/meson.build
index 97e00d6f59b..6e694ecd9fe 100644
--- a/meson.build
+++ b/meson.build
@@ -336,15 +336,40 @@ if host_arch == 'i386' and not cc.links('''
   qemu_common_flags = ['-march=i486'] + qemu_common_flags
 endif
 
-# Assume x86-64-v2 (minus CMPXCHG16B for 32-bit code)
-if host_arch == 'i386'
-  qemu_common_flags = ['-mfpmath=sse'] + qemu_common_flags
-endif
+# Pick x86-64 baseline version
 if host_arch in ['i386', 'x86_64']
-  qemu_common_flags = ['-mpopcnt', '-msse4.2'] + qemu_common_flags
-endif
-if host_arch == 'x86_64'
-  qemu_common_flags = ['-mcx16'] + qemu_common_flags
+  if get_option('x86_version') == '0' and host_arch == 'x86_64'
+    error('x86_64-v1 required for x86-64 hosts')
+  endif
+
+  # add flags for individual instruction set extensions
+  if get_option('x86_version') >= '1'
+    if host_arch == 'i386'
+      qemu_common_flags = ['-mfpmath=sse'] + qemu_common_flags
+    else
+      # present on basically all processors but technically not part of
+      # x86-64-v1, so only include -mneeded for x86-64 version 2 and above
+      qemu_common_flags = ['-mcx16'] + qemu_common_flags
+    endif
+  endif
+  if get_option('x86_version') >= '2'
+    qemu_common_flags = ['-mpopcnt'] + qemu_common_flags
+    qemu_common_flags = cc.get_supported_arguments('-mneeded') + qemu_common_flags
+  endif
+  if get_option('x86_version') >= '3'
+    qemu_common_flags = ['-mmovbe', '-mabm', '-mbmi1', '-mbmi2', '-mfma', '-mf16c'] + qemu_common_flags
+  endif
+
+  # add required vector instruction set (each level implies those below)
+  if get_option('x86_version') == '1'
+    qemu_common_flags = ['-msse2'] + qemu_common_flags
+  elif get_option('x86_version') == '2'
+    qemu_common_flags = ['-msse4.2'] + qemu_common_flags
+  elif get_option('x86_version') == '3'
+    qemu_common_flags = ['-mavx2'] + qemu_common_flags
+  elif get_option('x86_version') == '4'
+    qemu_common_flags = ['-mavx512f', '-mavx512bw', '-mavx512cd', '-mavx512dq', '-mavx512vl'] + qemu_common_flags
+  endif
 endif
 
 if get_option('prefer_static')
diff --git a/meson_options.txt b/meson_options.txt
index 7a79dd89706..6065ed2d352 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -370,3 +370,6 @@ option('qemu_ga_version', type: 'string', value: '',
 
 option('hexagon_idef_parser', type : 'boolean', value : true,
        description: 'use idef-parser to automatically generate TCG code for the Hexagon frontend')
+
+option('x86_version', type : 'combo', choices : ['0', '1', '2', '3', '4'], value: '1',
+       description: 'tweak required x86_64 architecture version beyond compiler default')
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index 58d49a447d5..62842d47e88 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -82,6 +82,8 @@ meson_options_help() {
   printf "%s\n" '  --with-suffix=VALUE      Suffix for QEMU data/modules/config directories'
   printf "%s\n" '                           (can be empty) [qemu]'
   printf "%s\n" '  --with-trace-file=VALUE  Trace file prefix for simple backend [trace]'
+  printf "%s\n" '  --x86-version=CHOICE     tweak required x86_64 architecture version beyond'
+  printf "%s\n" '                           compiler default [1] (choices: 0/1/2/3)'
   printf "%s\n" ''
   printf "%s\n" 'Optional features, enabled with --enable-FEATURE and'
   printf "%s\n" 'disabled with --disable-FEATURE, default is enabled if available'
@@ -552,6 +554,7 @@ _meson_option_parse() {
     --disable-werror) printf "%s" -Dwerror=false ;;
     --enable-whpx) printf "%s" -Dwhpx=enabled ;;
     --disable-whpx) printf "%s" -Dwhpx=disabled ;;
+    --x86-version=*) quote_sh "-Dx86_version=$2" ;;
     --enable-xen) printf "%s" -Dxen=enabled ;;
     --disable-xen) printf "%s" -Dxen=disabled ;;
     --enable-xen-pci-passthrough) printf "%s" -Dxen_pci_passthrough=enabled ;;
-- 
2.45.2

Re: [PATCH 4/6] meson: allow configuring the x86-64 baseline

[Daniel P. Berrangé]

On Thu, Jun 20, 2024 at 03:02:52PM +0200, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  meson.build                   | 41 ++++++++++++++++++++++++++++-------
>  meson_options.txt             |  3 +++
>  scripts/meson-buildoptions.sh |  3 +++
>  3 files changed, 39 insertions(+), 8 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 97e00d6f59b..6e694ecd9fe 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -336,15 +336,40 @@ if host_arch == 'i386' and not cc.links('''
>    qemu_common_flags = ['-march=i486'] + qemu_common_flags
>  endif
>  
> -# Assume x86-64-v2 (minus CMPXCHG16B for 32-bit code)
> -if host_arch == 'i386'
> -  qemu_common_flags = ['-mfpmath=sse'] + qemu_common_flags
> -endif
> +# Pick x86-64 baseline version
>  if host_arch in ['i386', 'x86_64']
> -  qemu_common_flags = ['-mpopcnt', '-msse4.2'] + qemu_common_flags
> -endif
> -if host_arch == 'x86_64'
> -  qemu_common_flags = ['-mcx16'] + qemu_common_flags
> +  if get_option('x86_version') == '0' and host_arch == 'x86_64'
> +    error('x86_64-v1 required for x86-64 hosts')
> +  endif
> +
> +  # add flags for individual instruction set extensions
> +  if get_option('x86_version') >= '1'
> +    if host_arch == 'i386'
> +      qemu_common_flags = ['-mfpmath=sse'] + qemu_common_flags
> +    else
> +      # present on basically all processors but technically not part of
> +      # x86-64-v1, so only include -mneeded for x86-64 version 2 and above
> +      qemu_common_flags = ['-mcx16'] + qemu_common_flags
> +    endif
> +  endif
> +  if get_option('x86_version') >= '2'
> +    qemu_common_flags = ['-mpopcnt'] + qemu_common_flags
> +    qemu_common_flags = cc.get_supported_arguments('-mneeded') + qemu_common_flags
> +  endif
> +  if get_option('x86_version') >= '3'
> +    qemu_common_flags = ['-mmovbe', '-mabm', '-mbmi1', '-mbmi2', '-mfma', '-mf16c'] + qemu_common_flags
> +  endif
> +
> +  # add required vector instruction set (each level implies those below)
> +  if get_option('x86_version') == '1'
> +    qemu_common_flags = ['-msse2'] + qemu_common_flags
> +  elif get_option('x86_version') == '2'
> +    qemu_common_flags = ['-msse4.2'] + qemu_common_flags
> +  elif get_option('x86_version') == '3'
> +    qemu_common_flags = ['-mavx2'] + qemu_common_flags
> +  elif get_option('x86_version') == '4'
> +    qemu_common_flags = ['-mavx512f', '-mavx512bw', '-mavx512cd', '-mavx512dq', '-mavx512vl'] + qemu_common_flags
> +  endif
>  endif


Any particular reason you chose to list various instructions individually
rather than just ask GCC for the full ABI ? I'd think all of the above
condences down to just

  # add flags for individual instruction set extensions
  if get_option('x86_version') >= '1'
    if host_arch == 'i386'
      qemu_common_flags = ['-mfpmath=sse'] + qemu_common_flags
    else
      # present on basically all processors but technically not part of
      # x86-64-v1, so only include -mneeded for x86-64 version 2 and above
      qemu_common_flags = ['-mcx16'] + qemu_common_flags
    endif
  endif
  if get_option('x86_version') >= '2'
    qemu_common_flags = ['-march=x86-64-v' + get_option('x86_version'), '-mneeded'] + qemu_common_flags
  endif


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 4/6] meson: allow configuring the x86-64 baseline

[Paolo Bonzini]

On Thu, Jun 20, 2024 at 4:55 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> Any particular reason you chose to list various instructions individually
> rather than just ask GCC for the full ABI ? I'd think all of the above
> condences down to just

To avoid that the default ('1') forces a lower level than the compiler default.

Something like what you propose below could work by adding a 'default'
value to the x86_version option, that leaves the flags entirely alone
apart from -mcx16.

However, doing so would prevent QEMU from changing the default to
x86-64-v2 in meson_options.txt, because then even a compiler that
defaults to x86-64-v3 would build a QEMU with AVX2 disabled.

For AVX2 specifically this is not a huge deal because the decision to
use AVX2 code is mostly done at runtime; but it would be a problem for
future integer instruction set extensions---for example if the distro
compiler uses APX you don't want to disable it.

>   # add flags for individual instruction set extensions
>   if get_option('x86_version') >= '1'
>     if host_arch == 'i386'
>       qemu_common_flags = ['-mfpmath=sse'] + qemu_common_flags

Also -msse2 here, but yes.

Paolo

Re: [PATCH 4/6] meson: allow configuring the x86-64 baseline

[Richard Henderson]

On 6/20/24 06:02, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> ---
>   meson.build                   | 41 ++++++++++++++++++++++++++++-------
>   meson_options.txt             |  3 +++
>   scripts/meson-buildoptions.sh |  3 +++
>   3 files changed, 39 insertions(+), 8 deletions(-)

Acked-by: Richard Henderson <richard.henderson@linaro.org>

For -mneeded, we need gcc 11 and for enforcing GNU_PROPERTY_X86_ISA_1_NEEDED we need glibc 
2.33, so:

   debian 12
   fedora 34
   ubuntu 2204
   suse leap 15.6 or tumbleweed.
   centos stream 9

I believe the -mneeded option will be accepted by FreeBSD's clang, but the note will not 
be enforced by the dynamic linker at startup.

However, since this is all optional, requiring an explicit configure option, I don't think 
any of this versioning should stand in the way.


r~

[PATCH 5/6] meson: remove dead optimization option

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build       | 13 -------------
 meson_options.txt |  2 --
 2 files changed, 15 deletions(-)

diff --git a/meson.build b/meson.build
index 6e694ecd9fe..54e6b09f4fb 100644
--- a/meson.build
+++ b/meson.build
@@ -2874,18 +2874,6 @@ config_host_data.set('CONFIG_AVX2_OPT', get_option('avx2') \
     int main(int argc, char *argv[]) { return bar(argv[argc - 1]); }
   '''), error_message: 'AVX2 not available').allowed())
 
-config_host_data.set('CONFIG_AVX512F_OPT', get_option('avx512f') \
-  .require(have_cpuid_h, error_message: 'cpuid.h not available, cannot enable AVX512F') \
-  .require(cc.links('''
-    #include <cpuid.h>
-    #include <immintrin.h>
-    static int __attribute__((target("avx512f"))) bar(void *a) {
-      __m512i x = *(__m512i *)a;
-      return _mm512_test_epi64_mask(x, x);
-    }
-    int main(int argc, char *argv[]) { return bar(argv[argc - 1]); }
-  '''), error_message: 'AVX512F not available').allowed())
-
 config_host_data.set('CONFIG_AVX512BW_OPT', get_option('avx512bw') \
   .require(have_cpuid_h, error_message: 'cpuid.h not available, cannot enable AVX512BW') \
   .require(cc.links('''
@@ -4283,7 +4271,6 @@ summary_info += {'mutex debugging':   get_option('debug_mutex')}
 summary_info += {'memory allocator':  get_option('malloc')}
 summary_info += {'avx2 optimization': config_host_data.get('CONFIG_AVX2_OPT')}
 summary_info += {'avx512bw optimization': config_host_data.get('CONFIG_AVX512BW_OPT')}
-summary_info += {'avx512f optimization': config_host_data.get('CONFIG_AVX512F_OPT')}
 summary_info += {'gcov':              get_option('b_coverage')}
 summary_info += {'thread sanitizer':  get_option('tsan')}
 summary_info += {'CFI support':       get_option('cfi')}
diff --git a/meson_options.txt b/meson_options.txt
index 6065ed2d352..0269fa0f16e 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -119,8 +119,6 @@ option('membarrier', type: 'feature', value: 'disabled',
 
 option('avx2', type: 'feature', value: 'auto',
        description: 'AVX2 optimizations')
-option('avx512f', type: 'feature', value: 'disabled',
-       description: 'AVX512F optimizations')
 option('avx512bw', type: 'feature', value: 'auto',
        description: 'AVX512BW optimizations')
 option('keyring', type: 'feature', value: 'auto',
-- 
2.45.2

Re: [PATCH 5/6] meson: remove dead optimization option

[Richard Henderson]

On 6/20/24 06:02, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> ---
>   meson.build       | 13 -------------
>   meson_options.txt |  2 --
>   2 files changed, 15 deletions(-)

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

r~

[PATCH 6/6] meson: require compiler support for chosen x86-64 instructions

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/meson.build b/meson.build
index 54e6b09f4fb..c5360fbd299 100644
--- a/meson.build
+++ b/meson.build
@@ -2863,6 +2863,7 @@ have_cpuid_h = cc.links('''
 config_host_data.set('CONFIG_CPUID_H', have_cpuid_h)
 
 config_host_data.set('CONFIG_AVX2_OPT', get_option('avx2') \
+  .enable_auto_if(get_option('x86_version') >= '3') \
   .require(have_cpuid_h, error_message: 'cpuid.h not available, cannot enable AVX2') \
   .require(cc.links('''
     #include <cpuid.h>
@@ -2875,6 +2876,7 @@ config_host_data.set('CONFIG_AVX2_OPT', get_option('avx2') \
   '''), error_message: 'AVX2 not available').allowed())
 
 config_host_data.set('CONFIG_AVX512BW_OPT', get_option('avx512bw') \
+  .enable_auto_if(get_option('x86_version') >= '4') \
   .require(have_cpuid_h, error_message: 'cpuid.h not available, cannot enable AVX512BW') \
   .require(cc.links('''
     #include <cpuid.h>
-- 
2.45.2

Re: [PATCH 6/6] meson: require compiler support for chosen x86-64 instructions

[Daniel P. Berrangé]

On Thu, Jun 20, 2024 at 03:02:54PM +0200, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  meson.build | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/meson.build b/meson.build
> index 54e6b09f4fb..c5360fbd299 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2863,6 +2863,7 @@ have_cpuid_h = cc.links('''
>  config_host_data.set('CONFIG_CPUID_H', have_cpuid_h)
>  
>  config_host_data.set('CONFIG_AVX2_OPT', get_option('avx2') \
> +  .enable_auto_if(get_option('x86_version') >= '3') \
>    .require(have_cpuid_h, error_message: 'cpuid.h not available, cannot enable AVX2') \
>    .require(cc.links('''
>      #include <cpuid.h>
> @@ -2875,6 +2876,7 @@ config_host_data.set('CONFIG_AVX2_OPT', get_option('avx2') \
>    '''), error_message: 'AVX2 not available').allowed())
>  
>  config_host_data.set('CONFIG_AVX512BW_OPT', get_option('avx512bw') \
> +  .enable_auto_if(get_option('x86_version') >= '4') \
>    .require(have_cpuid_h, error_message: 'cpuid.h not available, cannot enable AVX512BW') \
>    .require(cc.links('''
>      #include <cpuid.h>

I'm not sure this makes sense. The CONFIG_AVX* options are used only
to validate whether the toolchain has support for this. The QEMU
code then has a runtime, so it automagically uses AVX2/AVX512
if-and-only-if running on a suitably new CPU.  IOW, we want this
enabled always when the toolchain supports it, regardless of what
x86_version is set.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 6/6] meson: require compiler support for chosen x86-64 instructions

[Paolo Bonzini]

On Thu, Jun 20, 2024 at 5:01 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >  config_host_data.set('CONFIG_AVX2_OPT', get_option('avx2') \
> > +  .enable_auto_if(get_option('x86_version') >= '3') \
> >    .require(have_cpuid_h, error_message: 'cpuid.h not available, cannot enable AVX2') \
> >    .require(cc.links('''
> >      #include <cpuid.h>
> > @@ -2875,6 +2876,7 @@ config_host_data.set('CONFIG_AVX2_OPT', get_option('avx2') \
> >    '''), error_message: 'AVX2 not available').allowed())
> >
> >  config_host_data.set('CONFIG_AVX512BW_OPT', get_option('avx512bw') \
> > +  .enable_auto_if(get_option('x86_version') >= '4') \
> >    .require(have_cpuid_h, error_message: 'cpuid.h not available, cannot enable AVX512BW') \
> >    .require(cc.links('''
> >      #include <cpuid.h>
>
> I'm not sure this makes sense. The CONFIG_AVX* options are used only
> to validate whether the toolchain has support for this. The QEMU
> code then has a runtime, so it automagically uses AVX2/AVX512
> if-and-only-if running on a suitably new CPU.  IOW, we want this
> enabled always when the toolchain supports it, regardless of what
> x86_version is set.

The difference is that if the toolchain does not support AVX2/AVX512
intrinsics for some reason, and you require -Dx86_version={3,4}, meson
would report an error with this patch.

Paolo

Re: [PATCH 6/6] meson: require compiler support for chosen x86-64 instructions

[Richard Henderson]

On 6/20/24 08:01, Daniel P. Berrangé wrote:
> On Thu, Jun 20, 2024 at 03:02:54PM +0200, Paolo Bonzini wrote:
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>   meson.build | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/meson.build b/meson.build
>> index 54e6b09f4fb..c5360fbd299 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -2863,6 +2863,7 @@ have_cpuid_h = cc.links('''
>>   config_host_data.set('CONFIG_CPUID_H', have_cpuid_h)
>>   
>>   config_host_data.set('CONFIG_AVX2_OPT', get_option('avx2') \
>> +  .enable_auto_if(get_option('x86_version') >= '3') \
>>     .require(have_cpuid_h, error_message: 'cpuid.h not available, cannot enable AVX2') \
>>     .require(cc.links('''
>>       #include <cpuid.h>
>> @@ -2875,6 +2876,7 @@ config_host_data.set('CONFIG_AVX2_OPT', get_option('avx2') \
>>     '''), error_message: 'AVX2 not available').allowed())
>>   
>>   config_host_data.set('CONFIG_AVX512BW_OPT', get_option('avx512bw') \
>> +  .enable_auto_if(get_option('x86_version') >= '4') \
>>     .require(have_cpuid_h, error_message: 'cpuid.h not available, cannot enable AVX512BW') \
>>     .require(cc.links('''
>>       #include <cpuid.h>
> 
> I'm not sure this makes sense. The CONFIG_AVX* options are used only
> to validate whether the toolchain has support for this. The QEMU
> code then has a runtime, so it automagically uses AVX2/AVX512
> if-and-only-if running on a suitably new CPU.  IOW, we want this
> enabled always when the toolchain supports it, regardless of what
> x86_version is set.

Indeed.  To me this means they should not be configure options at all.
We should simply detect compiler support, end of.


r~

Re: [PATCH 6/6] meson: require compiler support for chosen x86-64 instructions

[Paolo Bonzini]

On Thu, Jun 20, 2024 at 7:22 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
> > I'm not sure this makes sense. The CONFIG_AVX* options are used only
> > to validate whether the toolchain has support for this. The QEMU
> > code then has a runtime, so it automagically uses AVX2/AVX512
> > if-and-only-if running on a suitably new CPU.  IOW, we want this
> > enabled always when the toolchain supports it, regardless of what
> > x86_version is set.
>
> Indeed.  To me this means they should not be configure options at all.
> We should simply detect compiler support, end of.

Ok, I'll drop this patch then for now.

Paolo