diff mbox

[dpdk-dev] eal: fix clang compilation error on ARM64

Message ID 20180406110103.29163-1-pbhagavatula@caviumnetworks.com (mailing list archive)
State Rejected, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail apply issues

Commit Message

Pavan Nikhilesh April 6, 2018, 11:01 a.m. UTC 
  Use __atomic_exchange_n instead of __atomic_exchange_(2/4/8).

Fixes: ff2863570fcc ("eal: introduce atomic exchange operation")

Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
---
 lib/librte_eal/common/include/generic/rte_atomic.h | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Thomas Monjalon April 6, 2018, 4:24 p.m. UTC | #1 
06/04/2018 13:01, Pavan Nikhilesh:
> Use __atomic_exchange_n instead of __atomic_exchange_(2/4/8).
> 
> Fixes: ff2863570fcc ("eal: introduce atomic exchange operation")

Please, could you provide a log of the error?

Why __atomic_exchange_n is fixing the compilation?
Pavan Nikhilesh April 6, 2018, 6:25 p.m. UTC | #2 
On Fri, Apr 06, 2018 at 06:24:34PM +0200, Thomas Monjalon wrote:
> 06/04/2018 13:01, Pavan Nikhilesh:
> > Use __atomic_exchange_n instead of __atomic_exchange_(2/4/8).
> >
> > Fixes: ff2863570fcc ("eal: introduce atomic exchange operation")
>
> Please, could you provide a log of the error?

  CC eal.o
In file included from /root/dpdk/lib/librte_eal/linuxapp/eal/eal.c:33:
In file included from /root/dpdk/build/include/rte_eal_memconfig.h:13:
In file included from /root/dpdk/build/include/rte_rwlock.h:10:
In file included from /root/dpdk/build/include/generic/rte_rwlock.h:25:
In file included from /root/dpdk/build/include/rte_atomic.h:37:
In file included from /root/dpdk/build/include/rte_atomic_64.h:16:
/root/dpdk/build/include/generic/rte_atomic.h:215:9: error: implicit declaration of function '__atomic_exchange_2' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
        return __atomic_exchange_2(dst, val, __ATOMIC_SEQ_CST);
               ^
/root/dpdk/build/include/generic/rte_atomic.h:215:9: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
/root/dpdk/build/include/generic/rte_atomic.h:494:9: error: implicit declaration of function '__atomic_exchange_4' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
        return __atomic_exchange_4(dst, val, __ATOMIC_SEQ_CST);
               ^
/root/dpdk/build/include/generic/rte_atomic.h:494:9: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
/root/dpdk/build/include/generic/rte_atomic.h:772:9: error: implicit declaration of function '__atomic_exchange_8' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
        return __atomic_exchange_8(dst, val, __ATOMIC_SEQ_CST);
               ^
/root/dpdk/build/include/generic/rte_atomic.h:772:9: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
6 errors generated.
make[5]: *** [/root/dpdk/mk/internal/rte.compile-pre.mk:116: eal.o] Error 1


>
> Why __atomic_exchange_n is fixing the compilation?

I guess __atomic_exchange_2/5/8 fall under c++11 standard?
https://llvm.org/docs/Atomics.html
>
>
>
Thomas Monjalon April 10, 2018, 3:07 p.m. UTC | #3 
06/04/2018 20:25, Pavan Nikhilesh:
> On Fri, Apr 06, 2018 at 06:24:34PM +0200, Thomas Monjalon wrote:
> > 06/04/2018 13:01, Pavan Nikhilesh:
> > > Use __atomic_exchange_n instead of __atomic_exchange_(2/4/8).
> > >
> > > Fixes: ff2863570fcc ("eal: introduce atomic exchange operation")
> >
> > Please, could you provide a log of the error?
> 
>   CC eal.o
> In file included from /root/dpdk/lib/librte_eal/linuxapp/eal/eal.c:33:
> In file included from /root/dpdk/build/include/rte_eal_memconfig.h:13:
> In file included from /root/dpdk/build/include/rte_rwlock.h:10:
> In file included from /root/dpdk/build/include/generic/rte_rwlock.h:25:
> In file included from /root/dpdk/build/include/rte_atomic.h:37:
> In file included from /root/dpdk/build/include/rte_atomic_64.h:16:
> /root/dpdk/build/include/generic/rte_atomic.h:215:9: error: implicit declaration of function '__atomic_exchange_2' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
>         return __atomic_exchange_2(dst, val, __ATOMIC_SEQ_CST);
>                ^
> /root/dpdk/build/include/generic/rte_atomic.h:215:9: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
> /root/dpdk/build/include/generic/rte_atomic.h:494:9: error: implicit declaration of function '__atomic_exchange_4' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
>         return __atomic_exchange_4(dst, val, __ATOMIC_SEQ_CST);
>                ^
> /root/dpdk/build/include/generic/rte_atomic.h:494:9: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
> /root/dpdk/build/include/generic/rte_atomic.h:772:9: error: implicit declaration of function '__atomic_exchange_8' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
>         return __atomic_exchange_8(dst, val, __ATOMIC_SEQ_CST);
>                ^
> /root/dpdk/build/include/generic/rte_atomic.h:772:9: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
> 6 errors generated.
> make[5]: *** [/root/dpdk/mk/internal/rte.compile-pre.mk:116: eal.o] Error 1
> 
> 
> >
> > Why __atomic_exchange_n is fixing the compilation?
> 
> I guess __atomic_exchange_2/5/8 fall under c++11 standard?
> https://llvm.org/docs/Atomics.html

Applied (with error log), thanks
Thomas Monjalon April 10, 2018, 9:35 p.m. UTC | #4 
Hi, big issue here.
This patch does not compile on Linux with ICC or GCC < 4.9
because of a missing C11 header:
	#include <stdatomic.h>

GCC 4.9 is recommended in doc/guides/linux_gsg/sys_reqs.rst.
But GCC 4.8 is used by SLES 12, RHEL 7, etc...

Note: Intel compilation tests are running with a backlog of one week,
so cannot catch such fail.

Exceptionnaly, I have decided to remove this patch pushed few hours ago
(not reverting), in order to avoid a serious "git bisect" breakage
in the middle of the git history.

We'll need to find a better way of fixing the compilation error
seen on ARM with clang.
To make it clear: I believe it is more important to preserve GCC 4.8
than clang compilation.
By the way, what is the version of clang which was causing the error?

The error was:
	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

The proposed solution was:
	Use __atomic_exchange_n instead of __atomic_exchange_(2/4/8),
	and include stdatomic.h.



10/04/2018 17:07, Thomas Monjalon:
> 06/04/2018 20:25, Pavan Nikhilesh:
> > On Fri, Apr 06, 2018 at 06:24:34PM +0200, Thomas Monjalon wrote:
> > > 06/04/2018 13:01, Pavan Nikhilesh:
> > > > Use __atomic_exchange_n instead of __atomic_exchange_(2/4/8).
> > > >
> > > > Fixes: ff2863570fcc ("eal: introduce atomic exchange operation")
[...]
> Applied (with error log), thanks
Pavan Nikhilesh April 11, 2018, 8:38 a.m. UTC | #5 
On Tue, Apr 10, 2018 at 11:35:15PM +0200, Thomas Monjalon wrote:
> Hi, big issue here.
> This patch does not compile on Linux with ICC or GCC < 4.9
> because of a missing C11 header:
> 	#include <stdatomic.h>
>
> GCC 4.9 is recommended in doc/guides/linux_gsg/sys_reqs.rst.
> But GCC 4.8 is used by SLES 12, RHEL 7, etc...
>
> Note: Intel compilation tests are running with a backlog of one week,
> so cannot catch such fail.
>
> Exceptionnaly, I have decided to remove this patch pushed few hours ago
> (not reverting), in order to avoid a serious "git bisect" breakage
> in the middle of the git history.
>
> We'll need to find a better way of fixing the compilation error
> seen on ARM with clang.
> To make it clear: I believe it is more important to preserve GCC 4.8
> than clang compilation.
> By the way, what is the version of clang which was causing the error?

I have tried with clang 4/5/6 and all have the same issue.

>
> The error was:
> 	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
>
> The proposed solution was:
> 	Use __atomic_exchange_n instead of __atomic_exchange_(2/4/8),
> 	and include stdatomic.h.
>
>
>
> 10/04/2018 17:07, Thomas Monjalon:
> > 06/04/2018 20:25, Pavan Nikhilesh:
> > > On Fri, Apr 06, 2018 at 06:24:34PM +0200, Thomas Monjalon wrote:
> > > > 06/04/2018 13:01, Pavan Nikhilesh:
> > > > > Use __atomic_exchange_n instead of __atomic_exchange_(2/4/8).
> > > > >
> > > > > Fixes: ff2863570fcc ("eal: introduce atomic exchange operation")
> [...]
> > Applied (with error log), thanks
>
>
>
Thomas Monjalon April 11, 2018, 8:46 a.m. UTC | #6 
11/04/2018 10:38, Pavan Nikhilesh:
> On Tue, Apr 10, 2018 at 11:35:15PM +0200, Thomas Monjalon wrote:
> > Hi, big issue here.
> > This patch does not compile on Linux with ICC or GCC < 4.9
> > because of a missing C11 header:
> > 	#include <stdatomic.h>
> >
> > GCC 4.9 is recommended in doc/guides/linux_gsg/sys_reqs.rst.
> > But GCC 4.8 is used by SLES 12, RHEL 7, etc...
> >
> > Note: Intel compilation tests are running with a backlog of one week,
> > so cannot catch such fail.
> >
> > Exceptionnaly, I have decided to remove this patch pushed few hours ago
> > (not reverting), in order to avoid a serious "git bisect" breakage
> > in the middle of the git history.
> >
> > We'll need to find a better way of fixing the compilation error
> > seen on ARM with clang.
> > To make it clear: I believe it is more important to preserve GCC 4.8
> > than clang compilation.
> > By the way, what is the version of clang which was causing the error?
> 
> I have tried with clang 4/5/6 and all have the same issue.

Do you know why the issue is not seen on x86?


> > The error was:
> > 	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
> >
> > The proposed solution was:
> > 	Use __atomic_exchange_n instead of __atomic_exchange_(2/4/8),
> > 	and include stdatomic.h.
> >
> >
> >
> > 10/04/2018 17:07, Thomas Monjalon:
> > > 06/04/2018 20:25, Pavan Nikhilesh:
> > > > On Fri, Apr 06, 2018 at 06:24:34PM +0200, Thomas Monjalon wrote:
> > > > > 06/04/2018 13:01, Pavan Nikhilesh:
> > > > > > Use __atomic_exchange_n instead of __atomic_exchange_(2/4/8).
> > > > > >
> > > > > > Fixes: ff2863570fcc ("eal: introduce atomic exchange operation")
> > [...]
> > > Applied (with error log), thanks
Pavan Nikhilesh April 11, 2018, 9 a.m. UTC | #7 
On Wed, Apr 11, 2018 at 10:46:47AM +0200, Thomas Monjalon wrote:
> 11/04/2018 10:38, Pavan Nikhilesh:
> > On Tue, Apr 10, 2018 at 11:35:15PM +0200, Thomas Monjalon wrote:
> > > Hi, big issue here.
> > > This patch does not compile on Linux with ICC or GCC < 4.9
> > > because of a missing C11 header:
> > > 	#include <stdatomic.h>
> > >
> > > GCC 4.9 is recommended in doc/guides/linux_gsg/sys_reqs.rst.
> > > But GCC 4.8 is used by SLES 12, RHEL 7, etc...
> > >
> > > Note: Intel compilation tests are running with a backlog of one week,
> > > so cannot catch such fail.
> > >
> > > Exceptionnaly, I have decided to remove this patch pushed few hours ago
> > > (not reverting), in order to avoid a serious "git bisect" breakage
> > > in the middle of the git history.
> > >
> > > We'll need to find a better way of fixing the compilation error
> > > seen on ARM with clang.
> > > To make it clear: I believe it is more important to preserve GCC 4.8
> > > than clang compilation.
> > > By the way, what is the version of clang which was causing the error?
> >
> > I have tried with clang 4/5/6 and all have the same issue.
>
> Do you know why the issue is not seen on x86?

Nope but I tried including the header as well as using -std=c11 in cflags but
still observing the same issue with clang.

>
>
> > > The error was:
> > > 	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
> > >
> > > The proposed solution was:
> > > 	Use __atomic_exchange_n instead of __atomic_exchange_(2/4/8),
> > > 	and include stdatomic.h.
> > >
> > >
> > >
> > > 10/04/2018 17:07, Thomas Monjalon:
> > > > 06/04/2018 20:25, Pavan Nikhilesh:
> > > > > On Fri, Apr 06, 2018 at 06:24:34PM +0200, Thomas Monjalon wrote:
> > > > > > 06/04/2018 13:01, Pavan Nikhilesh:
> > > > > > > Use __atomic_exchange_n instead of __atomic_exchange_(2/4/8).
> > > > > > >
> > > > > > > Fixes: ff2863570fcc ("eal: introduce atomic exchange operation")
> > > [...]
> > > > Applied (with error log), thanks
>
>
>
diff mbox

Patch

diff --git a/lib/librte_eal/common/include/generic/rte_atomic.h b/lib/librte_eal/common/include/generic/rte_atomic.h
index 8652c0264..91a6d615a 100644
--- a/lib/librte_eal/common/include/generic/rte_atomic.h
+++ b/lib/librte_eal/common/include/generic/rte_atomic.h
@@ -12,7 +12,9 @@ 
  * This file defines a generic API for atomic operations.
  */
 
+#include <stdatomic.h>
 #include <stdint.h>
+
 #include <rte_common.h>
 
 #ifdef __DOXYGEN__
@@ -212,7 +214,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)
 {
-	return __atomic_exchange_2(dst, val, __ATOMIC_SEQ_CST);
+	return __atomic_exchange_n(dst, val, __ATOMIC_SEQ_CST);
 }
 #endif
 
@@ -491,7 +493,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)
 {
-	return __atomic_exchange_4(dst, val, __ATOMIC_SEQ_CST);
+	return __atomic_exchange_n(dst, val, __ATOMIC_SEQ_CST);
 }
 #endif
 
@@ -769,7 +771,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)
 {
-	return __atomic_exchange_8(dst, val, __ATOMIC_SEQ_CST);
+	return __atomic_exchange_n(dst, val, __ATOMIC_SEQ_CST);
 }
 #endif