[PATCH 1/2] meson: move int128 checks from configure

[PATCH 1/2] meson: move int128 checks from configure

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 configure   | 47 -----------------------------------------------
 meson.build | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 47 deletions(-)

diff --git a/configure b/configure
index c56ed53ee365..66315835a663 100755
--- a/configure
+++ b/configure
@@ -2452,41 +2452,6 @@ if compile_prog "" "" ; then
     int128=yes
 fi
 
-#########################################
-# See if 128-bit atomic operations are supported.
-
-atomic128=no
-if test "$int128" = "yes"; then
-  cat > $TMPC << EOF
-int main(void)
-{
-  unsigned __int128 x = 0, y = 0;
-  y = __atomic_load(&x, 0);
-  __atomic_store(&x, y, 0);
-  __atomic_compare_exchange(&x, &y, x, 0, 0, 0);
-  return 0;
-}
-EOF
-  if compile_prog "" "" ; then
-    atomic128=yes
-  fi
-fi
-
-cmpxchg128=no
-if test "$int128" = yes && test "$atomic128" = no; then
-  cat > $TMPC << EOF
-int main(void)
-{
-  unsigned __int128 x = 0, y = 0;
-  __sync_val_compare_and_swap_16(&x, y, x);
-  return 0;
-}
-EOF
-  if compile_prog "" "" ; then
-    cmpxchg128=yes
-  fi
-fi
-
 ########################################
 # check if ccache is interfering with
 # semantic analysis of macros
@@ -2930,18 +2895,6 @@ if test "$have_tsan" = "yes" && test "$have_tsan_iface_fiber" = "yes" ; then
     echo "CONFIG_TSAN=y" >> $config_host_mak
 fi
 
-if test "$int128" = "yes" ; then
-  echo "CONFIG_INT128=y" >> $config_host_mak
-fi
-
-if test "$atomic128" = "yes" ; then
-  echo "CONFIG_ATOMIC128=y" >> $config_host_mak
-fi
-
-if test "$cmpxchg128" = "yes" ; then
-  echo "CONFIG_CMPXCHG128=y" >> $config_host_mak
-fi
-
 if test "$rdma" = "yes" ; then
   echo "CONFIG_RDMA=y" >> $config_host_mak
   echo "RDMA_LIBS=$rdma_libs" >> $config_host_mak
diff --git a/meson.build b/meson.build
index 2ed1d97c97ab..a9ec3974bc67 100644
--- a/meson.build
+++ b/meson.build
@@ -1824,6 +1824,45 @@ config_host_data.set('CONFIG_ATOMIC64', cc.links('''
     return 0;
   }'''))
 
+has_int128 = cc.links('''
+  __int128_t a;
+  __uint128_t b;
+  int main (void) {
+    a = a + b;
+    b = a * b;
+    a = a * a;
+    return 0;
+  }''')
+
+config_host_data.set('CONFIG_INT128', has_int128)
+
+if has_int128
+  has_atomic128 = cc.links('''
+    int main(void)
+    {
+      unsigned __int128 x = 0, y = 0;
+      y = __atomic_load(&x, 0);
+      __atomic_store(&x, y, 0);
+      __atomic_compare_exchange(&x, &y, x, 0, 0, 0);
+      return 0;
+    }''')
+
+  config_host_data.set('CONFIG_ATOMIC128', has_atomic128)
+
+  if not has_atomic128
+    has_cmpxchg128 = cc.links('''
+      int main(void)
+      {
+        unsigned __int128 x = 0, y = 0;
+        __sync_val_compare_and_swap_16(&x, y, x);
+        return 0;
+      }
+    ''')
+
+    config_host_data.set('CONFIG_CMPXCHG128', has_cmpxchg128)
+  endif
+endif
+
 config_host_data.set('CONFIG_GETAUXVAL', cc.links(gnu_source_prefix + '''
   #include <sys/auxv.h>
   int main(void) {
-- 
2.35.1.273.ge6ebfd0e8cbb

[PATCH 2/2] meson: fix CONFIG_ATOMIC128 check

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

testfile.c: In function 'main':
testfile.c:5:11: error: incorrect number of arguments to function '__atomic_load'
    5 |       y = __atomic_load(&x, 0);
      |           ^~~~~~~~~~~~~
testfile.c:6:7: error: argument 2 of '__atomic_store' must be a pointer type
    6 |       __atomic_store(&x, y, 0);
      |       ^~~~~~~~~~~~~~
testfile.c:7:7: error: argument 3 of '__atomic_compare_exchange' must be a pointer type
    7 |       __atomic_compare_exchange(&x, &y, x, 0, 0, 0);
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~

And it must be linked with -latomic.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 meson.build | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/meson.build b/meson.build
index a9ec3974bc67..a3d8af7a501b 100644
--- a/meson.build
+++ b/meson.build
@@ -1841,13 +1841,17 @@ if has_int128
     int main(void)
     {
       unsigned __int128 x = 0, y = 0;
-      y = __atomic_load(&x, 0);
-      __atomic_store(&x, y, 0);
-      __atomic_compare_exchange(&x, &y, x, 0, 0, 0);
+      __atomic_load(&x, &y, 0);
+      __atomic_store(&x, &y, 0);
+      __atomic_compare_exchange(&x, &y, &x, 0, 0, 0);
       return 0;
-    }''')
+    }''', args: ['-latomic'])
 
   config_host_data.set('CONFIG_ATOMIC128', has_atomic128)
+  if has_atomic128
+    add_global_link_arguments('-latomic',
+                              native: false, language: ['c', 'cpp', 'objc'])
+  endif
 
   if not has_atomic128
     has_cmpxchg128 = cc.links('''
-- 
2.35.1.273.ge6ebfd0e8cbb

Re: [PATCH 2/2] meson: fix CONFIG_ATOMIC128 check

[Philippe Mathieu-Daudé]

On 28/2/22 13:07, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> testfile.c: In function 'main':
> testfile.c:5:11: error: incorrect number of arguments to function '__atomic_load'
>      5 |       y = __atomic_load(&x, 0);
>        |           ^~~~~~~~~~~~~
> testfile.c:6:7: error: argument 2 of '__atomic_store' must be a pointer type
>      6 |       __atomic_store(&x, y, 0);
>        |       ^~~~~~~~~~~~~~
> testfile.c:7:7: error: argument 3 of '__atomic_compare_exchange' must be a pointer type
>      7 |       __atomic_compare_exchange(&x, &y, x, 0, 0, 0);
>        |       ^~~~~~~~~~~~~~~~~~~~~~~~~
> 
> And it must be linked with -latomic.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   meson.build | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index a9ec3974bc67..a3d8af7a501b 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1841,13 +1841,17 @@ if has_int128
>       int main(void)
>       {
>         unsigned __int128 x = 0, y = 0;
> -      y = __atomic_load(&x, 0);
> -      __atomic_store(&x, y, 0);
> -      __atomic_compare_exchange(&x, &y, x, 0, 0, 0);
> +      __atomic_load(&x, &y, 0);
> +      __atomic_store(&x, &y, 0);
> +      __atomic_compare_exchange(&x, &y, &x, 0, 0, 0);
>         return 0;
> -    }''')
> +    }''', args: ['-latomic'])
>   
>     config_host_data.set('CONFIG_ATOMIC128', has_atomic128)
> +  if has_atomic128
> +    add_global_link_arguments('-latomic',
> +                              native: false, language: ['c', 'cpp', 'objc'])
> +  endif
>   
>     if not has_atomic128
>       has_cmpxchg128 = cc.links('''

Shouldn't this be 1/2 for bisectability?

Anyhow "We deliberately don't link against libatomic", see:
https://lore.kernel.org/qemu-devel/CAFEAcA8V-PAwbtPWghvyjgKtzi949F6g9TnBJpLsGGHt51J64w@mail.gmail.com/

Re: [PATCH 2/2] meson: fix CONFIG_ATOMIC128 check

[Peter Maydell]

On Mon, 28 Feb 2022 at 12:19, <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> testfile.c: In function 'main':
> testfile.c:5:11: error: incorrect number of arguments to function '__atomic_load'
>     5 |       y = __atomic_load(&x, 0);
>       |           ^~~~~~~~~~~~~
> testfile.c:6:7: error: argument 2 of '__atomic_store' must be a pointer type
>     6 |       __atomic_store(&x, y, 0);
>       |       ^~~~~~~~~~~~~~
> testfile.c:7:7: error: argument 3 of '__atomic_compare_exchange' must be a pointer type
>     7 |       __atomic_compare_exchange(&x, &y, x, 0, 0, 0);
>       |       ^~~~~~~~~~~~~~~~~~~~~~~~~
>
> And it must be linked with -latomic.

As Philippe says, this isn't right. What the configure test
is checking for is "do we have 128-bit atomics which are
handled inline and specifically not via libatomic". The
reason we can't use libatomic is documented in the comment
starting "GCC is a house divided" in include/qemu/atomic128.h.

thanks
-- PMM

Re: [PATCH 2/2] meson: fix CONFIG_ATOMIC128 check

[Marc-André Lureau]

Hi

On Mon, Feb 28, 2022 at 5:24 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Mon, 28 Feb 2022 at 12:19, <marcandre.lureau@redhat.com> wrote:
> >
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > testfile.c: In function 'main':
> > testfile.c:5:11: error: incorrect number of arguments to function '__atomic_load'
> >     5 |       y = __atomic_load(&x, 0);
> >       |           ^~~~~~~~~~~~~
> > testfile.c:6:7: error: argument 2 of '__atomic_store' must be a pointer type
> >     6 |       __atomic_store(&x, y, 0);
> >       |       ^~~~~~~~~~~~~~
> > testfile.c:7:7: error: argument 3 of '__atomic_compare_exchange' must be a pointer type
> >     7 |       __atomic_compare_exchange(&x, &y, x, 0, 0, 0);
> >       |       ^~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > And it must be linked with -latomic.
>
> As Philippe says, this isn't right. What the configure test
> is checking for is "do we have 128-bit atomics which are
> handled inline and specifically not via libatomic". The
> reason we can't use libatomic is documented in the comment
> starting "GCC is a house divided" in include/qemu/atomic128.h.

The arguments fix is still valid, no?

Re: [PATCH 2/2] meson: fix CONFIG_ATOMIC128 check

[Peter Maydell]

On Mon, 28 Feb 2022 at 13:28, Marc-André Lureau
<marcandre.lureau@redhat.com> wrote:
>
> Hi
>
> On Mon, Feb 28, 2022 at 5:24 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Mon, 28 Feb 2022 at 12:19, <marcandre.lureau@redhat.com> wrote:
> > >
> > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >
> > > testfile.c: In function 'main':
> > > testfile.c:5:11: error: incorrect number of arguments to function '__atomic_load'
> > >     5 |       y = __atomic_load(&x, 0);
> > >       |           ^~~~~~~~~~~~~
> > > testfile.c:6:7: error: argument 2 of '__atomic_store' must be a pointer type
> > >     6 |       __atomic_store(&x, y, 0);
> > >       |       ^~~~~~~~~~~~~~
> > > testfile.c:7:7: error: argument 3 of '__atomic_compare_exchange' must be a pointer type
> > >     7 |       __atomic_compare_exchange(&x, &y, x, 0, 0, 0);
> > >       |       ^~~~~~~~~~~~~~~~~~~~~~~~~
> > >
> > > And it must be linked with -latomic.
> >
> > As Philippe says, this isn't right. What the configure test
> > is checking for is "do we have 128-bit atomics which are
> > handled inline and specifically not via libatomic". The
> > reason we can't use libatomic is documented in the comment
> > starting "GCC is a house divided" in include/qemu/atomic128.h.
>
> The arguments fix is still valid, no?

Probably. (Does this mean we've never been correctly setting
CONFIG_ATOMIC128?)

I think we should have the 128-bit atomics check in meson.build
look the same as the existing one for 64-bit atomics, though,
unless there's a reason why they should look different.
For the 64-bit version we explicitly pass __ATOMIC_RELAXED,
and we use __atomic_load_n(), not __atomic_load().

thanks
-- PMM

Re: [PATCH 2/2] meson: fix CONFIG_ATOMIC128 check

[Richard Henderson]

On 2/28/22 03:36, Peter Maydell wrote:
> I think we should have the 128-bit atomics check in meson.build
> look the same as the existing one for 64-bit atomics, though,
> unless there's a reason why they should look different.
> For the 64-bit version we explicitly pass __ATOMIC_RELAXED,
> and we use __atomic_load_n(), not __atomic_load().

Yep, since that's what we use in qatomic_read__nocheck, etc.


r~