Re: [dpdk-dev] [PATCH v3 01/33] meson: add libatomic as a global dependency for i686 clang

[Bruce Richardson <bruce.richardson@intel.com>]

On Tue, Mar 31, 2020 at 01:36:46PM +0000, Pavan Nikhilesh Bhagavatula wrote:
> 
> 
> >-----Original Message-----
> >From: dev <dev-bounces@dpdk.org> On Behalf Of Bruce Richardson
> >Sent: Monday, March 30, 2020 6:47 PM
> >To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> >Cc: Thomas Monjalon <thomas@monjalon.net>; Pavan Nikhilesh
> >Bhagavatula <pbhagavatula@marvell.com>; Liang Ma
> ><liang.j.ma@intel.com>; Peter Mccarthy <peter.mccarthy@intel.com>;
> >Marko Kovacevic <marko.kovacevic@intel.com>; Ori Kam
> ><orika@mellanox.com>; Radu Nicolau <radu.nicolau@intel.com>; Akhil
> >Goyal <akhil.goyal@nxp.com>; Tomasz Kantecki
> ><tomasz.kantecki@intel.com>; Sunil Kumar Kori <skori@marvell.com>;
> >David Hunt <david.hunt@intel.com>; Honnappa Nagarahalli
> ><honnappa.nagarahalli@arm.com>; dev@dpdk.org;
> >david.marchand@redhat.com; mattias.ronnblom@ericsson.com
> >Subject: Re: [dpdk-dev] [PATCH v3 01/33] meson: add libatomic as a
> >global dependency for i686 clang
> >
> >On Sun, Mar 29, 2020 at 08:13:10PM +0530, jerinj@marvell.com wrote:
> >> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >>
> >> Add libatomic as a global dependency when compiling for 32-bit using
> >> clang. As we need libatomic for 64-bit atomic ops.
> >>
> >> Cc: bruce.richardson@intel.com
> >> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >> ---
> >>  config/meson.build                  | 10 ++++++++++
> >>  drivers/event/octeontx/meson.build  |  5 -----
> >>  drivers/event/octeontx2/meson.build |  5 -----
> >>  drivers/event/opdl/meson.build      |  5 -----
> >>  examples/l2fwd-event/meson.build    |  5 -----
> >>  lib/librte_distributor/meson.build  |  5 -----
> >>  lib/librte_rcu/meson.build          |  5 -----
> >>  7 files changed, 10 insertions(+), 30 deletions(-)
> >>
> >> diff --git a/config/meson.build b/config/meson.build
> >> index abedd76f2..6e5530110 100644
> >> --- a/config/meson.build
> >> +++ b/config/meson.build
> >> @@ -173,6 +173,16 @@ if pcap_dep.found() and
> >cc.has_header('pcap.h', dependencies: pcap_dep)
> >>  	dpdk_extra_ldflags += '-lpcap'
> >>  endif
> >>
> >> +
> >> +# for clang 32-bit compiles we need libatomic for 64-bit atomic ops
> >> +if cc.get_id() == 'clang' and dpdk_conf.get('RTE_ARCH_64') == false
> >> +	atomic_dep = cc.find_library('atomic', required: true)
> >> +	if atomic_dep.found()
> >> +		add_project_link_arguments('-latomic', language: 'c')
> >> +		dpdk_extra_ldflags += '-latomic'
> >> +	endif
> >> +endif
> >> +
> >
> >Minor nit, you don't need to check for .found(), since the configure will
> >fail if it's not found, since "required" is set to true.
> 
> Yup but I felt a bit odd adding the dependency without using .found().
> I will remove the inner if check in the next version.
> 
> >
> >For cleanliness, you may also be able to use get_pkgconfig_variable() or
> >get_configtool_variable() to get -latomic, rather than hard-coding it,
> >though in this case I suspect hard-coding is fine.
> 
> Looks like get_pkgconfig_variable/get_configtool_variable only work on dependency
> object using it on object from find_library() causes meson to error out 
> ` Unknown method "get_pkgconfig_variable" in object `
> 
Ok, thanks for checking.