Re: [PATCH] meson: drop --enable-avx* options

["Daniel P. Berrangé" <berrange@redhat.com>]

On Thu, Oct 10, 2024 at 11:13:22AM +0200, Paolo Bonzini wrote:
> Just detect compiler support and always enable the optimizations if
> it is avilable; warn if the user did request AVX2/AVX512 use via
> -Dx86_version= but the intrinsics are not available.
> 
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  meson.build                   | 30 +++++++++++++++++++-----------
>  meson_options.txt             |  4 ----
>  scripts/meson-buildoptions.sh |  6 ------
>  3 files changed, 19 insertions(+), 21 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index e4b2af138da..b4418d54e0a 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2955,22 +2955,16 @@ config_host_data.set('CONFIG_ASM_HWPROBE_H',
>                       cc.has_header_symbol('asm/hwprobe.h',
>                                            'RISCV_HWPROBE_EXT_ZBA'))
>  
> -config_host_data.set('CONFIG_AVX2_OPT', get_option('avx2') \
> -  .require(have_cpuid_h, error_message: 'cpuid.h not available, cannot enable AVX2') \
> -  .require(cc.links('''
> -    #include <cpuid.h>
> +if have_cpuid_h
> +  have_avx2 = cc.links('''
>      #include <immintrin.h>
>      static int __attribute__((target("avx2"))) bar(void *a) {
>        __m256i x = *(__m256i *)a;
>        return _mm256_testz_si256(x, x);
>      }
>      int main(int argc, char *argv[]) { return bar(argv[argc - 1]); }
> -  '''), error_message: 'AVX2 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('''
> -    #include <cpuid.h>
> +  ''')
> +  have_avx512bw = cc.links('''
>      #include <immintrin.h>
>      static int __attribute__((target("avx512bw"))) bar(void *a) {
>        __m512i *x = a;
> @@ -2978,7 +2972,21 @@ config_host_data.set('CONFIG_AVX512BW_OPT', get_option('avx512bw') \
>        return res[1];
>      }
>      int main(int argc, char *argv[]) { return bar(argv[0]); }
> -  '''), error_message: 'AVX512BW not available').allowed())
> +  ''')
> +  if get_option('x86_version') >= '3' and not have_avx2
> +    warning('Cannot enable AVX optimizations due to missing intrinsics')
> +  elif get_option('x86_version') >= '4' and not have_avx512bw
> +    warning('Cannot enable AVX512 optimizations due to missing intrinsics')
> +  endif

Should these perhaps be error() rather than warning() ?

We only support GCC & CLang. If both GCC 7.4.0 and CLang 10.0 (our
min versions) have the intrinsics, then I'd say this is an impossible
scenario if x86_version is large, and thus would deserve error()

> +else
> +  have_avx2 = false
> +  have_avx512bw = false
> +  if get_option('x86_version') >= '3'
> +    warning('Cannot enable AVX optimizations due to missing cpuid.h')
> +  endif
> +endif
> +config_host_data.set('CONFIG_AVX2_OPT', have_avx2)
> +config_host_data.set('CONFIG_AVX512BW_OPT', have_avx512bw)
>  
>  # For both AArch64 and AArch32, detect if builtins are available.
>  config_host_data.set('CONFIG_ARM_AES_BUILTIN', cc.compiles('''
> diff --git a/meson_options.txt b/meson_options.txt
> index 5a5c2300261..d5abf5193f5 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -119,10 +119,6 @@ option('tpm', type : 'feature', value : 'auto',
>  option('membarrier', type: 'feature', value: 'disabled',
>         description: 'membarrier system call (for Linux 4.14+ or Windows')
>  
> -option('avx2', type: 'feature', value: 'auto',
> -       description: 'AVX2 optimizations')
> -option('avx512bw', type: 'feature', value: 'auto',
> -       description: 'AVX512BW optimizations')
>  option('keyring', type: 'feature', value: 'auto',
>         description: 'Linux keyring support')
>  option('libkeyutils', type: 'feature', value: 'auto',
> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> index 6d08605b771..dc796f48bad 100644
> --- a/scripts/meson-buildoptions.sh
> +++ b/scripts/meson-buildoptions.sh
> @@ -94,8 +94,6 @@ meson_options_help() {
>    printf "%s\n" '  alsa            ALSA sound support'
>    printf "%s\n" '  attr            attr/xattr support'
>    printf "%s\n" '  auth-pam        PAM access control'
> -  printf "%s\n" '  avx2            AVX2 optimizations'
> -  printf "%s\n" '  avx512bw        AVX512BW optimizations'
>    printf "%s\n" '  blkio           libblkio block device driver'
>    printf "%s\n" '  bochs           bochs image format support'
>    printf "%s\n" '  bpf             eBPF support'
> @@ -238,10 +236,6 @@ _meson_option_parse() {
>      --audio-drv-list=*) quote_sh "-Daudio_drv_list=$2" ;;
>      --enable-auth-pam) printf "%s" -Dauth_pam=enabled ;;
>      --disable-auth-pam) printf "%s" -Dauth_pam=disabled ;;
> -    --enable-avx2) printf "%s" -Davx2=enabled ;;
> -    --disable-avx2) printf "%s" -Davx2=disabled ;;
> -    --enable-avx512bw) printf "%s" -Davx512bw=enabled ;;
> -    --disable-avx512bw) printf "%s" -Davx512bw=disabled ;;
>      --enable-gcov) printf "%s" -Db_coverage=true ;;
>      --disable-gcov) printf "%s" -Db_coverage=false ;;
>      --enable-lto) printf "%s" -Db_lto=true ;;
> -- 
> 2.46.2
> 
> 

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 :|