[igt-dev] [PATCH i-g-t v3 1/4] meson: add libatomic dependency

[Ser, Simon]

On Tue, 2019-06-18 at 14:59 +0100, Guillaume Tucker wrote:
> On 18/06/2019 14:20, Ser, Simon wrote:
> > On Tue, 2019-06-18 at 13:27 +0100, Guillaume Tucker wrote:
> > > Add conditional dependency on libatomic in order to be able to use the
> > > __atomic_* functions instead of the older __sync_* ones.  The
> > > libatomic library is only needed when there aren't any native support
> > > on the current architecture, so a linker test is used for this
> > > purpose.  This enables atomic operations to be on a wider number of
> > > architectures including MIPS.
> > > 
> > > Signed-off-by: Guillaume Tucker <guillaume.tucker at collabora.com>
> > > ---
> > > 
> > > Notes:
> > >     v2: add linker test for libatomic
> > >     v3: use null_dep
> > > 
> > >  meson.build | 14 ++++++++++++++
> > >  1 file changed, 14 insertions(+)
> > > 
> > > diff --git a/meson.build b/meson.build
> > > index 6268c58d3634..118ad667ffb5 100644
> > > --- a/meson.build
> > > +++ b/meson.build
> > > @@ -180,6 +180,20 @@ realtime = cc.find_library('rt')
> > >  dlsym = cc.find_library('dl')
> > >  zlib = cc.find_library('z')
> > >  
> > > +if cc.links('''
> > > +#include <stdint.h>
> > > +int main(void) {
> > > +  uint32_t x32 = 0;
> > > +  uint64_t x64 = 0;
> > > +  __atomic_load_n(&x32, __ATOMIC_SEQ_CST);
> > > +  __atomic_load_n(&x64, __ATOMIC_SEQ_CST);
> > 
> > See my reply for v2. I've looked into this a little bit more and it
> > looks like __atomic_* functions are a GCC implementation detail. OIn
> > other words, the C11 standard [1] defines only atomic_* functions, and
> > GCC implements them with __atomic_* builtins when the platform supports
> > it, but other compilers might not expose those builtins and still
> > support atomic_* functions without them. This also seems to be what [2]
> > explains:
> > 
> > > The first set of library functions are named __atomic_*. This set has
> > > been “standardized” by GCC, and is described below. (See also GCC’s
> > > documentation)
> > 
> > (Notice the quotes around “standardized”, meaning they are a GCC
> > extension)
> 
> Quite, and while the stdatomic.h API is part of the C11 standard,
> libatomic is part of GCC.  So this test is to determine whether
> linking against GCC's libatomic.so is needed for its __atomic_*
> fallback implementation.
> 
> It raises the question of what to do with other compilers, but
> igt has other build errors with clang on mips at the moment.
> With a quick search, it looks like its __atomic_* functions are
> part of libclang.so for clang.

I don't see anything in `readelf -s /usr/lib/libclang.so.8`.

> Maybe this test should only be used when the compiler name is
> gcc?  In practice it does work with both gcc and clang though, as
> they both use the same naming convention for atomic built-ins.

Hmm. I'm still not quite sure I understand why checking with __atomic_*
is preferred.

- If the compiler has __atomic_* builtins: this won't link with
  libatomic
- If the compiler doesn't have __atomic_* builtins: this will link with
  libatomic even if stdatomic.h works without it

What we're really interested in is stdatomic.h support, not __atomic_*.
So I still think checking for atomic_* is better than __atomic_*. Am I
missing something?

> Guillaume
> 
> > [1]: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1548.pdf
> > [2]: https://llvm.org/docs/Atomics.html
> > 
> > > +  return 0;
> > > +}''', name : 'built-in atomics')
> > > +	libatomic = null_dep
> > > +else
> > > +	libatomic = cc.find_library('atomic')
> > > +endif
> > > +
> > >  if cc.has_header('linux/kd.h')
> > >  	config.set('HAVE_LINUX_KD_H', 1)
> > >  endif