search

lancaster-university / microbit-v2-samples

CODAL build tools and sample programs for the micro:bit v2.x
MIT License
64 stars 83 forks source link

Configure debug info to DWARF v4. #82

Open microbit-carlos opened 10 months ago

microbit-carlos commented 10 months ago

As done by @thegecko in https://github.com/lancaster-university/microbit-v2-samples/commit/b0ab5ffeafe6b3a47a11d65f611fe289feff4df0.

This should also help with some of the issues I've seen with bloaty parsing DWARF v5 files produced by arm-none-eabi-gcc v12.3.

microbit-carlos commented 10 months ago

@Johnn333 could you apply the same dwarf flags to the LLVM PR and ensure they compile correctly?

thegecko commented 10 months ago

BTW, if you set the build mode to regular 'debug' you will also need these flags set:

microbit-carlos commented 10 months ago

Looking at the docs it looks like CMAKE_<lang>_FLAGS_DEBUG_INIT initialise the flags for CMAKE_<lang>_FLAGS_DEBUG, so that should be fine? https://cmake.org/cmake/help/latest/variable/CMAKE_LANG_FLAGS_CONFIG_INIT.html

However, the docs indicate that it was only introduced in CMake v3.11 and we need to support 3.6, so maybe we need to replace the CMAKE_<lang>_FLAGS_<config>_INIT entries with CMAKE_<lang>_FLAGS_<config> anyway.

thegecko commented 10 months ago

Looking at the docs it looks like CMAKE__FLAGS_DEBUGINIT initialise the flags for CMAKE_FLAGS_DEBUG, so that should be fine?

This didn't work for me, perhaps because versions

Johnn333 commented 10 months ago

@Johnn333 could you apply the same dwarf flags to the LLVM PR and ensure they compile correctly?

Seems okay on my end, it still compiles, full of warnings as ever. I'm hoping to spend some time at some point getting the GCC built libraries to be compatiable with LLD (currently Clang has to build all of CODAL and use those libraries), but with some of the changes to CODAL its much closer to being able to digest the default GCC ones. Wouldn't worry too much about the LLVM build :).

microbit-carlos commented 9 months ago

Okay, so essentially the CMAKE_<LANG>_FLAGS_<CONFIG>_INIT CMake variables didn't seem to be working, and they weren't overwriting the default values for CMAKE_<LANG>_FLAGS_<CONFIG>. More info can be found in:

Since we have to support older version of CMake anyway, I've replace CMAKE_<LANG>_FLAGS_<CONFIG>_INIT with CMAKE_<LANG>_FLAGS_<CONFIG> (setting the variable, rather than appending to it, as we want these to be the starting values).

With this change and comparing the before and after this patch, there are significant changes in the build. That's because until now we've been building with -O2 instead of -Os.

As a quick test, I've change the optimisation level on top of this patch to O2, built again, and compared that with a build from master. With this patch + O2, doing a diff of this map file and the map from master, and the only difference is the size of the debug strings, everything else is the same, so that's a decent test that this PR is working as expected.

Before we can merge this PR with -Os, we should have a look at the affected components, as listed in https://github.com/lancaster-university/codal-microbit-v2/issues/373#issuecomment-1712037165 to see if anything that might be performance critical could be negatively affected.

@JohnVidler could you have a look at the list there and confirm if we are relatively safe to introduce performance issues?