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

[Richard Henderson <richard.henderson@linaro.org>]

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~