Re: [dpdk-dev] [PATCH] eal: fix build of external apps with clang on armv8

[Ilya Maximets <i.maximets@samsung.com>]

On 14.01.2019 19:46, Richardson, Bruce wrote:
> 
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ilya Maximets
>> Sent: Monday, January 14, 2019 4:15 PM
>> To: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>
>> Cc: Pavan Nikhilesh <pbhagavatula@marvell.com>; Ilya Maximets
>> <i.maximets@samsung.com>; stable@dpdk.org
>> Subject: [dpdk-dev] [PATCH] eal: fix build of external apps with clang on
>> armv8
>>
>> In case DPDK built using GCC, RTE_TOOLCHAIN_CLANG is not defined.
>> But 'rte_atomic.h' is a generic header that included to the external apps
>> like OVS while building with DPDK. As a result, clang build of OVS fails
>> on ARMv8 if DPDK built using gcc:
>>
>>     include/generic/rte_atomic.h:215:9: error:
>>             implicit declaration of function '__atomic_exchange_2'
>>             is invalid in C99
>>     include/generic/rte_atomic.h:494:9: error:
>>             implicit declaration of function '__atomic_exchange_4'
>>             is invalid in C99
>>     include/generic/rte_atomic.h:772:9: error:
>>             implicit declaration of function '__atomic_exchange_8'
>>             is invalid in C99
>>
>> We need to check for current compiler, not the compiler used for DPDK
>> build.
>>
>> Fixes: 7bdccb93078e ("eal: fix ARM build with clang")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>> ---
>>  lib/librte_eal/common/include/generic/rte_atomic.h | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/librte_eal/common/include/generic/rte_atomic.h
>> b/lib/librte_eal/common/include/generic/rte_atomic.h
>> index b99ba4688..d0c464fb1 100644
>> --- a/lib/librte_eal/common/include/generic/rte_atomic.h
>> +++ b/lib/librte_eal/common/include/generic/rte_atomic.h
>> @@ -212,7 +212,7 @@ rte_atomic16_exchange(volatile uint16_t *dst, uint16_t
>> val);  static inline uint16_t  rte_atomic16_exchange(volatile uint16_t
>> *dst, uint16_t val)  { -#if defined(RTE_ARCH_ARM64) &&
>> defined(RTE_TOOLCHAIN_CLANG)
>> +#if defined(RTE_ARCH_ARM64) && defined(__clang__)
>>  	return __atomic_exchange_n(dst, val, __ATOMIC_SEQ_CST);  #else
>>  	return __atomic_exchange_2(dst, val, __ATOMIC_SEQ_CST); @@ -495,7
>> +495,7 @@ rte_atomic32_exchange(volatile uint32_t *dst, uint32_t val);
>> static inline uint32_t  rte_atomic32_exchange(volatile uint32_t *dst,
>> uint32_t val)  { -#if defined(RTE_ARCH_ARM64) &&
>> defined(RTE_TOOLCHAIN_CLANG)
>> +#if defined(RTE_ARCH_ARM64) && defined(__clang__)
>>  	return __atomic_exchange_n(dst, val, __ATOMIC_SEQ_CST);  #else
>>  	return __atomic_exchange_4(dst, val, __ATOMIC_SEQ_CST); @@ -777,7
>> +777,7 @@ rte_atomic64_exchange(volatile uint64_t *dst, uint64_t val);
>> static inline uint64_t  rte_atomic64_exchange(volatile uint64_t *dst,
>> uint64_t val)  { -#if defined(RTE_ARCH_ARM64) &&
>> defined(RTE_TOOLCHAIN_CLANG)
>> +#if defined(RTE_ARCH_ARM64) && defined(__clang__)
>>  	return __atomic_exchange_n(dst, val, __ATOMIC_SEQ_CST);  #else
>>  	return __atomic_exchange_8(dst, val, __ATOMIC_SEQ_CST);
>> --
>> 2.17.1
> 
> Is this really architecture-specific? Would the same issue not occur on e.g. x86 or PPC?
> 

Yes. I looked a bit deeper and found that x86 build is also broken.
We have not encountered this problem earlier on other platforms because
CONFIG_RTE_FORCE_INTRINSICS enabled by default only for armv8.

I've sent a patch:
  http://patches.dpdk.org/patch/49829/

Best regards, Ilya Maximets.