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

["Ser, Simon" <simon.ser@intel.com>]

On Tue, 2019-06-18 at 09:33 +0100, Guillaume Tucker wrote:
> On 14/06/2019 09:07, Ser, Simon wrote:
> > On Thu, 2019-06-13 at 14:53 +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@collabora.com>
> > > ---
> > >  meson.build | 13 +++++++++++++
> > >  1 file changed, 13 insertions(+)
> > > 
> > > diff --git a/meson.build b/meson.build
> > > index 6268c58d3634..da25a28f3268 100644
> > > --- a/meson.build
> > > +++ b/meson.build
> > > @@ -179,6 +179,19 @@ math = cc.find_library('m')
> > >  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);
> > 
> > Minor: maybe we could've used stdatomic.h's atomic_* functions (without
> > the "__" prefix) for consistency with the actual IGT code?
> 
> I actually thought it was more correct to use the __atomic_*
> functions directly from the libatomic library as this is a linker
> test.  If for any reason stdatomic.h changes between versions or
> compilers and uses something else than libatomic, then this test
> would become invalid.

I don't know whether __atomic_* functions are standard builtins or if
they are just an implementation detail.

If they are standard builtins, it makes more sense to check for them as
this patch does.

If they aren't, we instead should try to link a program using
stdatomic.h's atomic_* functions, and if it doesn't work assume
libatomic.so is required.

> > > +  return 0;
> > > +}''', name : 'built-in atomics')
> > > +	libatomic = []
> > 
> > We could use null_dep instead, to make it consistent with the other
> > branch.
> 
> Ack, will fix that in v3.
> 
> > > +else
> > > +	libatomic = cc.find_library('atomic')
> > > +endif
> > >  
> > >  if cc.has_header('linux/kd.h')
> > >  	config.set('HAVE_LINUX_KD_H', 1)
> 
> Thanks,
> Guillaume
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev