-
Notifications
You must be signed in to change notification settings - Fork 54
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
CODAL has been building with -O2
flag instead of -Os
#373
Labels
Comments
-O2
by default instead of -Os
-O
flag instead of -Os
-O
flag instead of -Os
-O
flag instead of -Os
The -O2 flag can be seen in CI as well simply by building with the
|
Changing
|
-O
flag instead of -Os
-O2
flag instead of -Os
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
If we build with the verbose output we can see it at the end of each file compilation:
The interesting part is this bit:
Which I believe is the CMake flags configured for the
RelWithDebInfo
CMake build type (the default). To test this, we can add these messages at the top of the CMakefile.txt file and see the result:Output:
The microbit-v2-samples repo is meant to be setting this value with
CMAKE_CXX_FLAGS_RELWITHDEBINFO_INIT
:lancaster-university/microbit-v2-samples/utils/cmake/toolchains/ARM_GCC/compiler-flags.cmake#L42
But we have two issues:
CMAKE_<lang>_FLAGS_<config>_INIT
was only introduced in CMake 3.11, and we support 3.6+, so in those versionsCMAKE_CXX_FLAGS_RELWITHDEBINFO_INIT
is not doing anythingSo we should probably explicitly set the build type to
RelWithDebInfo
(while still being configurable via CMake flag) and correctly configure optimisation flags by usingCMAKE_<lang>_FLAGS_<config>
instead ofCMAKE_<lang>_FLAGS_<config>_INIT
, but the question might be:Do we want to move to
-Os
, if it might change the memory footprint?Usually the tradeoffs for flash might be performance (less inlining and things like that), but is it possible it might increase RAM usage at this stage?
The text was updated successfully, but these errors were encountered: