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/22 09:33:21 UTC
[GitHub] [incubator-nuttx] CV-Bowen opened a new pull request, #6132: compiler.h: optimization option is not supported before GCC 4.6
CV-Bowen opened a new pull request, #6132:
URL: https://github.com/apache/incubator-nuttx/pull/6132
## Summary
-fno-tree-loop-distribute-patterns is not supported before GCC 4.6
Optimization option -ftree_loop_distribute_patterns is enabled by default at -O3 at GCC 4.6
I confirmed that by searching "OPT_ftree_loop_distribute_patterns" in gcc 4.5.4 (https://ftp.gnu.org/gnu/gcc/gcc-4.5.4/) and gcc 4.6.0 (https://ftp.gnu.org/gnu/gcc/gcc-4.6.0/) release source code.
## Testing
compile with gcc 4.5.1 and gcc 9.2.1
--
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 #6132: compiler.h: optimization option is not supported before GCC 4.6
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6132:
URL: https://github.com/apache/incubator-nuttx/pull/6132#discussion_r858438817
##########
include/nuttx/compiler.h:
##########
@@ -379,9 +379,11 @@
# if defined(__clang__)
# define no_builtin(n) __attribute__((no_builtin(n)))
-# else
+# elif (__GNUC__ == 4 && __GNUC_MINOR__ >= 6) || (__GNUC__ > 4)
# define no_builtin(n) __attribute__((__optimize__("-fno-tree-loop-distribute-patterns")))
Review Comment:
Those are not an easy questions :)
1. I'm thinking that the more compiler specific can be moved to make subsystem the better it is. But that is only my personal opinion.
2. Here it depends what should be the behaviour of the final system. I see reasonable to apply it to all the files because it is not logical to me that NuttX provides implementation for `memcpy`, but some parts of the system use it while other use builtin version. I mean that we can have an option either to use NuttX `memcpy` or builtin `memcpy` but not the mix. But again this is a point for discussion and maybe I'm missing the bigger picture.
--
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 #6132: compiler.h: optimization option is not supported before GCC 4.6
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6132:
URL: https://github.com/apache/incubator-nuttx/pull/6132#discussion_r858355943
##########
include/nuttx/compiler.h:
##########
@@ -379,9 +379,11 @@
# if defined(__clang__)
# define no_builtin(n) __attribute__((no_builtin(n)))
-# else
+# elif (__GNUC__ == 4 && __GNUC_MINOR__ >= 6) || (__GNUC__ > 4)
# define no_builtin(n) __attribute__((__optimize__("-fno-tree-loop-distribute-patterns")))
Review Comment:
1. -fno-builtin-xxx is specific to gcc/clang. Yes, no_builtin is also gcc/clang specific, but c macro is more simple.
2. do you want to apply -fno-builtin-xxx for individual file.
--
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 #6132: compiler.h: optimization option is not supported before GCC 4.6
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6132:
URL: https://github.com/apache/incubator-nuttx/pull/6132#discussion_r858355943
##########
include/nuttx/compiler.h:
##########
@@ -379,9 +379,11 @@
# if defined(__clang__)
# define no_builtin(n) __attribute__((no_builtin(n)))
-# else
+# elif (__GNUC__ == 4 && __GNUC_MINOR__ >= 6) || (__GNUC__ > 4)
# define no_builtin(n) __attribute__((__optimize__("-fno-tree-loop-distribute-patterns")))
Review Comment:
Some question:
1. -fno-builtin-xxx is specific to gcc/clang. Yes, no_builtin is also gcc/clang specific, but c macro is more simple.
2. do you want to apply -fno-builtin-xxx for individual file.
--
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 #6132: compiler.h: optimization option is not supported before GCC 4.6
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6132:
URL: https://github.com/apache/incubator-nuttx/pull/6132#discussion_r858573952
##########
include/nuttx/compiler.h:
##########
@@ -379,9 +379,11 @@
# if defined(__clang__)
# define no_builtin(n) __attribute__((no_builtin(n)))
-# else
+# elif (__GNUC__ == 4 && __GNUC_MINOR__ >= 6) || (__GNUC__ > 4)
# define no_builtin(n) __attribute__((__optimize__("-fno-tree-loop-distribute-patterns")))
Review Comment:
> Those are not an easy questions :)
>
> 1. I'm thinking that the more compiler specific can be moved to make subsystem the better it is. But that is only my personal opinion.
> 2. Here it depends what should be the behaviour of the final system. I see reasonable to apply it to all the files because it is not logical to me that NuttX provides implementation for `memcpy`, but some parts of the system use it while other use builtin version.
I think builtin here mean that compiler can assume these special functions implement the behavior defined by standard, so the compiler can do more optimization(e.g. replace memcpy with load/store instruction). but these built-in functions can be implemented by anyone(e.g. glibc, musl, newlib, or nuttx).
> I mean that we can have an option either to use NuttX `memcpy` or builtin `memcpy` but not the mix. But again this is a point for discussion and maybe I'm missing the bigger picture.
Even we provide some option to skip compile these builtin function, there is still many other function(e.g. open/close, malloc/free) duplicated in the third party library(e.g. newlib) and nuttx.
--
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 #6132: compiler.h: optimization option is not supported before GCC 4.6
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6132:
URL: https://github.com/apache/incubator-nuttx/pull/6132#discussion_r858026137
##########
include/nuttx/compiler.h:
##########
@@ -379,9 +379,11 @@
# if defined(__clang__)
# define no_builtin(n) __attribute__((no_builtin(n)))
-# else
+# elif (__GNUC__ == 4 && __GNUC_MINOR__ >= 6) || (__GNUC__ > 4)
# define no_builtin(n) __attribute__((__optimize__("-fno-tree-loop-distribute-patterns")))
Review Comment:
@xiaoxiang781216 maybe we can explore if we can drop `no_builtin` at all and use `-fno-builtin-memcmp`, `-fno-builtin-memcpy`, etc. instead?
--
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 pull request #6132: compiler.h: optimization option is not supported before GCC 4.6
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6132:
URL: https://github.com/apache/incubator-nuttx/pull/6132#issuecomment-1106433182
> The CI failed, but it seems it's unrelated with this PR? @xiaoxiang781216
Fix here: https://github.com/apache/incubator-nuttx/pull/6133. Please rebase to the last mainline.
--
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 #6132: compiler.h: optimization option is not supported before GCC 4.6
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6132:
URL: https://github.com/apache/incubator-nuttx/pull/6132#discussion_r858573952
##########
include/nuttx/compiler.h:
##########
@@ -379,9 +379,11 @@
# if defined(__clang__)
# define no_builtin(n) __attribute__((no_builtin(n)))
-# else
+# elif (__GNUC__ == 4 && __GNUC_MINOR__ >= 6) || (__GNUC__ > 4)
# define no_builtin(n) __attribute__((__optimize__("-fno-tree-loop-distribute-patterns")))
Review Comment:
> Those are not an easy questions :)
>
> 1. I'm thinking that the more compiler specific can be moved to make subsystem the better it is. But that is only my personal opinion.
> 2. Here it depends what should be the behaviour of the final system. I see reasonable to apply it to all the files because it is not logical to me that NuttX provides implementation for `memcpy`, but some parts of the system use it while other use builtin version.
I think builtin here mean that compiler can assume these special functions implement the behavior defined by standard, so the compiler can do more optimization(e.g. replace memcpy with load/store instruction). but these built-in functions can be implemented by anyone(e.g. glibc, musl, newlib, or nuttx).
> I mean that we can have an option either to use NuttX `memcpy` or builtin `memcpy` but not the mix. But again this is a point for discussion and maybe I'm missing the bigger picture.
Even we provide some option to skip compile these builtin function, there is still many other functions(e.g. open/close, malloc/free) duplicated in the third party library(e.g. newlib) and nuttx. It's hard to tell the linker pull memcpy... from 3rd party library and malloc/fopen... from nuttx.
--
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 #6132: compiler.h: optimization option is not supported before GCC 4.6
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6132:
URL: https://github.com/apache/incubator-nuttx/pull/6132#discussion_r858595191
##########
include/nuttx/compiler.h:
##########
@@ -379,9 +379,11 @@
# if defined(__clang__)
# define no_builtin(n) __attribute__((no_builtin(n)))
-# else
+# elif (__GNUC__ == 4 && __GNUC_MINOR__ >= 6) || (__GNUC__ > 4)
# define no_builtin(n) __attribute__((__optimize__("-fno-tree-loop-distribute-patterns")))
Review Comment:
Ok. Let's keep things like they are now until someone is not satisfied or anything gets broken (hopefully never).
--
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 #6132: compiler.h: optimization option is not supported before GCC 4.6
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #6132:
URL: https://github.com/apache/incubator-nuttx/pull/6132
--
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 #6132: compiler.h: optimization option is not supported before GCC 4.6
Posted by GitBox <gi...@apache.org>.
CV-Bowen commented on PR #6132:
URL: https://github.com/apache/incubator-nuttx/pull/6132#issuecomment-1106430638
The CI failed, but it seems it's unrelated with this PR? @xiaoxiang781216
--
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 pull request #6132: compiler.h: optimization option is not supported before GCC 4.6
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6132:
URL: https://github.com/apache/incubator-nuttx/pull/6132#issuecomment-1106341920
> -fno-tree-loop-distribute-patterns is not supported before GCC 4.6
>
> I confirmed that by searching "ftree-loop-distribute-patterns" in gcc 4.5.4 (https://ftp.gnu.org/gnu/gcc/gcc-4.5.4/) and gcc 4.6.0 (https://ftp.gnu.org/gnu/gcc/gcc-4.6.0/) release source code. In the gcc source code, file gcc/common.opt lists all the optimize option.
>
Thanks for the detailed analysis.
--
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