Use modern CMake's target_* functions to set compiler flags/features
Hi all,
Thank you for this great piece of software, it's blazingly fast and memory efficient! The past few days I have been playing around a bit with the API, and trying to integrate it in one of my projects.
In my own C++ projects, I usually include third party depencies as git submodules in a vendor/ directory. Then in my own CMakeLists.txt files, I can call add_subdirectory on those third party dependencies, and all their targets will be available in my own CMakeList.txt too (makes it easy to link them, and configure all include directories etc.)
I did the same thing with Bifrost, however, one of my executables was segfaulting, even though it was a very tiny toy example that hardly did anything. After some debugging, I found that my executable missed the -march=native compiler flag, and therefore the instruction sets of bifrost_static and my executable mismatched, resulting in stack corruption.
The new target_* based API introduced in CMake 3 is supposed to better deal with these kinds of situations. In this pull request I changed the CMakeLists.txt a bit to make use of this new API. This makes it easier to include Bifrost in other (CMake based) projects, by simply including Bifrost using add_directory and linking to bifrost_static or bifrost_dynamic, and ensures that the required compiler flags get propagated to the linked executables too.
I'll admit, I've only minimally changed it, until it fixed my problem 😬 I've not checked the options you seem to use for profiling etc. Anyway hope it's a useful starting point. I've found this guide on modern Cmake helpful: https://cliutils.gitlab.io/modern-cmake/
Hi there,
First of all, thank you for your contribution! I am very glad that you like Bifrost so let us know if we can do anything else to help integrate Bifrost into your own project. Your pull request looks very sound to me and I just want to try it out before merging it ;) I'm a little bit busy those days so no worries, I didn't forget about it, it shouldn't take too long.
Thank you again for your work!
Best, Guillaume
I've updated the cmake scripts with improved support for profiling. Setting CMAKE_BUILD_TYPE to Profile should enable the -pg flags when using GCC.
Following guidelines from: https://stackoverflow.com/questions/24460486/cmake-build-type-is-not-being-used-in-cmakelists-txt/24470998#24470998
Hey @lrvdijk,
Thank you again for your contribution and your efforts on PyBifrost. It took me some time to get back to you on this PR because I'm developing Bifrost on my free time nowadays.
I have a couple of questions regarding your PR if you don't mind:
- This line and this line seems to be GCC specific. I could have a look at what Clang proposes for profiling flags but in the meantime, I think either Cmake should fail to produce a Makefile for Clang + Profiling or display an warning message and still compile with "-pg" (Clang will compile with this flag).
- Is the Debug mode gone (I can see only the Profiling mode)?
- Lines like this one add the requirement to compile C++ code with C++11. What about the requirement of compiling the C code using C11?
- I see that the minimum Cmake version requirement is Cmake 3.12. Is it possible to lower that version requirement even further? We will use that version if need be but we've been trying since the beginning to use a cmake version as low as possible to work on older clusters.
Thank you for your time.
Thanks for your time!
I've updated the CMakeLists.txt a bit more, which should address most of your points! However, I noticed that target_link_options is added in CMake 3.13, so the minimum version actually increased.
Debug mode is not gone, as can be seen on this line:
set(CMAKE_CONFIGURATION_TYPES "${CMAKE_CONFIGURATION_TYPES};Profile")
I append Profile to the existing value of CMAKE_CONFIGURATION_TYPES, which is by default Debug;Release;RelWithDebInfo;MinSizeRel, and with reasonable default compiler flags.
https://cmake.org/cmake/help/latest/variable/CMAKE_BUILD_TYPE.html