You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/04/28 03:42:33 UTC
[GitHub] [incubator-nuttx] CV-Bowen opened a new pull request, #6169: libc/arch_atomic: add weak attribute for all atomic functions.
CV-Bowen opened a new pull request, #6169:
URL: https://github.com/apache/incubator-nuttx/pull/6169
## Summary
Some compilers have defined __atomic builtin functions, so add weak attribute for all atomic functions implemented by libc.
## Impact
all chip select LIBC_ARCH_ATOMIC.
## Testing
CI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6169: libc/arch_atomic: add weak attribute for all atomic functions.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6169:
URL: https://github.com/apache/incubator-nuttx/pull/6169#discussion_r860805050
##########
libs/libc/machine/arch_atomic.c:
##########
@@ -34,7 +34,8 @@
#define STORE(n, type) \
\
- void __atomic_store_ ## n (volatile void *ptr, \
+ void weak_function __atomic_store_ ## n ( \
+ volatile void *ptr, \
Review Comment:
please attach a new patch to add FAR, @CV-Bowen .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] CV-Bowen commented on pull request #6169: libc/arch_atomic: add weak attribute for all atomic functions.
Posted by GitBox <gi...@apache.org>.
CV-Bowen commented on PR #6169:
URL: https://github.com/apache/incubator-nuttx/pull/6169#issuecomment-1112835820
> it seems pointless to make those functions `weak`.
Yes, `weak_function` seems unnecessary and i removed them just recently. Please have a look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6169: libc/arch_atomic: add weak attribute for all atomic functions.
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6169:
URL: https://github.com/apache/incubator-nuttx/pull/6169#discussion_r860810077
##########
libs/libc/machine/arch_atomic.c:
##########
@@ -401,3 +425,35 @@ FETCH_XOR(4, uint32_t)
****************************************************************************/
FETCH_XOR(8, uint64_t)
+
+/* Clang define the __sync builtins, add #ifndef to avoid
+ * redefined/redeclared problem.
+ */
+
+#ifndef __clang__
+
+/****************************************************************************
+ * Name: __sync_add_and_fetch_1
+ ****************************************************************************/
+
+SYNC_ADD_FETCH(1, uint8_t)
Review Comment:
do we need `__sync_sub_and_fetch` and others?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] CV-Bowen commented on a diff in pull request #6169: libc/arch_atomic: add weak attribute for all atomic functions.
Posted by GitBox <gi...@apache.org>.
CV-Bowen commented on code in PR #6169:
URL: https://github.com/apache/incubator-nuttx/pull/6169#discussion_r860813251
##########
libs/libc/machine/arch_atomic.c:
##########
@@ -401,3 +425,35 @@ FETCH_XOR(4, uint32_t)
****************************************************************************/
FETCH_XOR(8, uint64_t)
+
+/* Clang define the __sync builtins, add #ifndef to avoid
+ * redefined/redeclared problem.
+ */
+
+#ifndef __clang__
+
+/****************************************************************************
+ * Name: __sync_add_and_fetch_1
+ ****************************************************************************/
+
+SYNC_ADD_FETCH(1, uint8_t)
Review Comment:
Yes, i will do this in this PR later today.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6169: libc/arch_atomic: add weak attribute for all atomic functions.
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6169:
URL: https://github.com/apache/incubator-nuttx/pull/6169#discussion_r860785593
##########
libs/libc/machine/arch_atomic.c:
##########
@@ -34,7 +34,8 @@
#define STORE(n, type) \
\
- void __atomic_store_ ## n (volatile void *ptr, \
+ void weak_function __atomic_store_ ## n ( \
+ volatile void *ptr, \
Review Comment:
Can we add `FAR` to pointers? Or maybe in the next PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #6169: libc/arch_atomic: add gcc legacy __sync buitins support and add FAR to pointers.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #6169:
URL: https://github.com/apache/incubator-nuttx/pull/6169
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] CV-Bowen commented on pull request #6169: libc/arch_atomic: add weak attribute for all atomic functions.
Posted by GitBox <gi...@apache.org>.
CV-Bowen commented on PR #6169:
URL: https://github.com/apache/incubator-nuttx/pull/6169#issuecomment-1112833311
> In the link that you posted in PR description I also see `__sync_fetch_and_add` and others. Do we need it?
Yes, __atomic_add_fetch and others are also needed and i want to put these modifications to next PR. Actually, the implementation of __atomic_ops_fetch and __sync_ops_and_fetch is the same and i want to unity them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on pull request #6169: libc/arch_atomic: add weak attribute for all atomic functions.
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on PR #6169:
URL: https://github.com/apache/incubator-nuttx/pull/6169#issuecomment-1112271713
Shouldn't we remove the `LIBC_ARCH_ATOMIC` config and make these files part of the build for every chip unconditionally now?
Otherwise it seems pointless to make those functions `weak`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] CV-Bowen commented on a diff in pull request #6169: libc/arch_atomic: add weak attribute for all atomic functions.
Posted by GitBox <gi...@apache.org>.
CV-Bowen commented on code in PR #6169:
URL: https://github.com/apache/incubator-nuttx/pull/6169#discussion_r860925842
##########
libs/libc/machine/arch_atomic.c:
##########
@@ -401,3 +425,35 @@ FETCH_XOR(4, uint32_t)
****************************************************************************/
FETCH_XOR(8, uint64_t)
+
+/* Clang define the __sync builtins, add #ifndef to avoid
+ * redefined/redeclared problem.
+ */
+
+#ifndef __clang__
+
+/****************************************************************************
+ * Name: __sync_add_and_fetch_1
+ ****************************************************************************/
+
+SYNC_ADD_FETCH(1, uint8_t)
Review Comment:
@pkarashchenko Done
##########
libs/libc/machine/arch_atomic.c:
##########
@@ -34,7 +34,8 @@
#define STORE(n, type) \
\
- void __atomic_store_ ## n (volatile void *ptr, \
+ void weak_function __atomic_store_ ## n ( \
+ volatile void *ptr, \
Review Comment:
@xiaoxiang781216 Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #6169: libc/arch_atomic: add weak attribute for all atomic functions.
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6169:
URL: https://github.com/apache/incubator-nuttx/pull/6169#issuecomment-1112606723
In the link that you posted in PR description I also see `__sync_fetch_and_add` and others. Do we need it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org