diff mbox series

meson: drop --enable-avx* options

Message ID 20241010091322.1790604-1-pbonzini@redhat.com (mailing list archive)
State New
Headers show
Series meson: drop --enable-avx* options | expand

Commit Message

Paolo Bonzini Oct. 10, 2024, 9:13 a.m. UTC 
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(-)

Comments

Daniel P. Berrangé Oct. 10, 2024, 9:24 a.m. UTC | #1 
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
Richard Henderson Oct. 11, 2024, 5:32 p.m. UTC | #2 
On 10/10/24 02:24, Daniel P. Berrangé wrote:
> 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()
Agreed.  Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~
diff mbox series

Patch

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
+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 ;;