Re: [PATCH] meson: ensure we enable CMPXCHG128 on x86_64

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

On Fri, Oct 04, 2024 at 03:01:23PM -0700, Pierrick Bouvier wrote:
> Alex discovered that CMPXCHG128 was not enabled when building for
> x86_64, resulting in slow execution for wide atomic instructions,
> creating a huge contention when combined with a high number of cpus
> (found while booting android aarch64 guest on x86_64 host).
> 
> The problem is that even though we enable -mcx16 option for x86_64, this
> is not used when testing for CMPXCHG128. Thus, we silently turn it off.
> 
> x86_64 is the only architecture adding machine flags for now, so the
> problem is limited to this host architecture.
> 
> Meson compiler tests are supposed to be independent of environment flags
> (https://mesonbuild.com/Reference-manual_returned_compiler.html#returned-by).
> However, CFLAGS are used anyway, thus masking the problem when using
> something like CFLAGS='-march=native'. This is a meson bug and was reported:
> https://github.com/mesonbuild/meson/issues/13757
> 
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>  meson.build | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/meson.build b/meson.build
> index b18c2a54ab5..af2ce595dcc 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2867,6 +2867,13 @@ if has_int128_type
>      config_host_data.set('CONFIG_ATOMIC128_OPT', has_atomic128_opt)
>  
>      if not has_atomic128_opt
> +
> +      host_flags = []
> +      if host_arch == 'x86_64'
> +        # for x86_64, x86_version must be >= 1, and we always enable cmpxchg16
> +        # in this case.
> +        host_flags += ['-mcx16']
> +      endif

IMHO, we shouldn't be assuming that the earlier code passed '-mcx16',
as it requires that we keep 2 far away parts of meson.build in sync.

>        config_host_data.set('CONFIG_CMPXCHG128', cc.links('''
>          int main(void)
>          {
> @@ -2874,7 +2881,8 @@ if has_int128_type
>            __sync_val_compare_and_swap_16(&x, y, x);
>            return 0;
>          }
> -      '''))
> +      ''',
> +      args: host_flags))

Just pass 'qemu_common_flags' here.

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