-
-
Notifications
You must be signed in to change notification settings - Fork 52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add more optimization flags (no exception/no rtti) #182
Conversation
@@ -78,6 +78,8 @@ macro(dynamic_project_options) | |||
"ENABLE_CPPCHECK\;OFF\;${MAKEFILE_OR_NINJA}\;Enable cppcheck analysis during compilation" | |||
"ENABLE_INTERPROCEDURAL_OPTIMIZATION\;OFF\;OFF\;Enable whole-program optimization (e.g. LTO)" | |||
"ENABLE_NATIVE_OPTIMIZATION\;OFF\;OFF\;Enable the optimizations specific to the build machine (e.g. SSE4_1, AVX2, etc.)." | |||
"DISABLE_EXCEPTIONS\;OFF\;OFF\;Disable Exceptions (no-exceptions flag)" | |||
"DISABLE_RTTI\;OFF\;OFF\;Disable RTTI (no-rtti flag)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are interesting options for embedded targets. There's a third one that comes to mind -fno-threadsafe-statics
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not emit the extra code to use the routines specified in the C++ ABI for thread-safe initialization of local statics. You can use this option to reduce code size slightly in code that doesn’t need to be thread-safe.
https://gcc.gnu.org/onlinedocs/gcc/C_002b_002b-Dialect-Options.html#index-fno-threadsafe-statics
(interesting for shaving some extra bytes from the binary, haven't used this option, yet...)
if it's only one and only get used in GCC, I would just add this option with target_compile_options(mytarget INTERFACE $<$<CXX_COMPILER_ID:GNU,Clang>:-fno-threadsafe-statics>)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason I mentioned that flag is because it may not be desirable (or possible at all) to have mutexes (or some other form of critical sections / disabling interrupts) when static variables are accessed on some systems. A quick google tells that MSVC has a /Zc:threadSafeInit-
flag.
I took an interests in your pull request because I'm trying to do similar stuff. @aminya is probably in a better position to judge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this optimization is unsafe, I prefer that we do not add it. In general, it would be important not to hinder the safety/correctness of a program for minor performance improvements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you are sure that one part of the code doesn't need to be thread-safe, you should use the thread_local
attributes for that specific variable. You cannot assume that this is true for all the programs and your dependencies.
https://en.cppreference.com/w/cpp/language/storage_duration
thread_local size_t ABC = 1;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The conflicts need to be fixed
Just to make life easier with MSVC- vs. GNU-flags.