diff mbox series

[v3,01/33] meson: add libatomic as a global dependency for i686 clang

Message ID 20200329144342.1543749-2-jerinj@marvell.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series DPDK Trace support |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-mellanox-Performance fail Performance Testing issues
ci/iol-intel-Performance fail Performance Testing issues
ci/iol-testing fail Testing issues
ci/Intel-compilation success Compilation OK

Commit Message

Jerin Jacob Kollanukkaran March 29, 2020, 2:43 p.m. UTC 
  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(-)

Comments

Bruce Richardson March 30, 2020, 1:17 p.m. UTC | #1 
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.

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.

With the superfluous if removed

Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Pavan Nikhilesh Bhagavatula March 31, 2020, 1:36 p.m. UTC | #2 
>-----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 `

>
>With the superfluous if removed
>
>Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Bruce Richardson March 31, 2020, 4:21 p.m. UTC | #3 
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.
diff mbox series

Patch

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
+
 # add -include rte_config to cflags
 add_project_arguments('-include', 'rte_config.h', language: 'c')
 
diff --git a/drivers/event/octeontx/meson.build b/drivers/event/octeontx/meson.build
index 73118a485..2b74bb62d 100644
--- a/drivers/event/octeontx/meson.build
+++ b/drivers/event/octeontx/meson.build
@@ -11,8 +11,3 @@  sources = files('ssovf_worker.c',
 )
 
 deps += ['common_octeontx', 'mempool_octeontx', 'bus_vdev', 'pmd_octeontx']
-
-# 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
-	ext_deps += cc.find_library('atomic')
-endif
diff --git a/drivers/event/octeontx2/meson.build b/drivers/event/octeontx2/meson.build
index 56febb8d8..dfe8fc4b2 100644
--- a/drivers/event/octeontx2/meson.build
+++ b/drivers/event/octeontx2/meson.build
@@ -20,11 +20,6 @@  if not dpdk_conf.get('RTE_ARCH_64')
 	extra_flags += ['-Wno-int-to-pointer-cast', '-Wno-pointer-to-int-cast']
 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
-	ext_deps += cc.find_library('atomic')
-endif
-
 foreach flag: extra_flags
 	if cc.has_argument(flag)
 		cflags += flag
diff --git a/drivers/event/opdl/meson.build b/drivers/event/opdl/meson.build
index e67b164e3..566462f4c 100644
--- a/drivers/event/opdl/meson.build
+++ b/drivers/event/opdl/meson.build
@@ -10,8 +10,3 @@  sources = files(
 	'opdl_test.c',
 )
 deps += ['bus_vdev']
-
-# 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
-	ext_deps += cc.find_library('atomic')
-endif
diff --git a/examples/l2fwd-event/meson.build b/examples/l2fwd-event/meson.build
index c4664c3a3..4e9a069d6 100644
--- a/examples/l2fwd-event/meson.build
+++ b/examples/l2fwd-event/meson.build
@@ -16,8 +16,3 @@  sources = files(
 	'l2fwd_event_internal_port.c',
 	'l2fwd_event_generic.c'
 )
-
-# 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
-	ext_deps += cc.find_library('atomic')
-endif
diff --git a/lib/librte_distributor/meson.build b/lib/librte_distributor/meson.build
index 266af6434..bd12ddb2f 100644
--- a/lib/librte_distributor/meson.build
+++ b/lib/librte_distributor/meson.build
@@ -9,8 +9,3 @@  else
 endif
 headers = files('rte_distributor.h')
 deps += ['mbuf']
-
-# 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
-	ext_deps += cc.find_library('atomic')
-endif
diff --git a/lib/librte_rcu/meson.build b/lib/librte_rcu/meson.build
index 62920ba02..0c2d5a2e0 100644
--- a/lib/librte_rcu/meson.build
+++ b/lib/librte_rcu/meson.build
@@ -5,8 +5,3 @@  allow_experimental_apis = true
 
 sources = files('rte_rcu_qsbr.c')
 headers = files('rte_rcu_qsbr.h')
-
-# 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
-	ext_deps += cc.find_library('atomic')
-endif