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

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

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~

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

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