From cb0c80d8dc7510435c15431d5e39c9a9e484948d Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 9 May 2022 02:22:16 +0200 Subject: [PATCH 1/8] add option and cmake module for lto support --- CMakeLists.txt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index bfe3da84..d34511e8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -24,6 +24,13 @@ set(FSFW_CATCH2_LIB_VERSION v${FSFW_CATCH2_LIB_MAJOR_VERSION}.0.0-preview5 CACHE set(FSFW_ETL_LIB_NAME etl) +include(CheckIPOSupported) +check_ipo_supported(RESULT IPO_SUPPORTED OUTPUT IPO_ERROR) +if(NOT IPO_SUPPORTED) + message(STATUS "FSFW | IPO/LTO not supported: ${IPO_ERROR}") +endif() + +option(FSFW_ENABLE_IPO "Enable interprocedural optimization or link-time optimization if available" ON) option(FSFW_GENERATE_SECTIONS "Generate function and data sections. Required to remove unused code" ON ) From a943e4eebb1aeb3cc2a01e8df7e1069341ac085b Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 9 May 2022 02:23:20 +0200 Subject: [PATCH 2/8] enable LTO where applicable --- CMakeLists.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index d34511e8..1700236f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -69,6 +69,10 @@ set(FSFW_DUMMY_TGT fsfw-dummy) project(${LIB_FSFW_NAME}) add_library(${LIB_FSFW_NAME}) +if(IPO_SUPPORTED AND FSFW_ENABLE_IPO) + set_property(TARGET ${LIB_FSFW_NAME} PROPERTY INTERPROCEDURAL_OPTIMIZATION TRUE) +endif() + if(FSFW_BUILD_UNITTESTS) message(STATUS "Building the FSFW unittests in addition to the static library") # Check whether the user has already installed Catch2 first From a4bd5a2aaaff36e7ad1d207a48533eb42e51439d Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 9 May 2022 10:31:03 +0200 Subject: [PATCH 3/8] update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d28c5c6..4d713f26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## Additions +- LTO support: Allow using LTO/IPO by setting `FSFW_ENABLE_LTO=1`. CMake is able to detect whether + the user compiler supports IPO/LPO. LTO is on by default now. Most modern compilers support it + and can make good use of it and is usually makes the code faster and/or larger. + PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/616 - Linux HAL: Add wiretapping option for I2C. Enabled with `FSFW_HAL_I2C_WIRETAPPING` defined to 1 - Dedicated Version class and constant `fsfw::FSFW_VERSION` containing version information inside `fsfw/version.h` From 637512ad77023a13c15214b5b513535eee4f7f40 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 9 May 2022 10:34:14 +0200 Subject: [PATCH 4/8] changelog update --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d713f26..dc9577f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,8 +44,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## Additions - LTO support: Allow using LTO/IPO by setting `FSFW_ENABLE_LTO=1`. CMake is able to detect whether - the user compiler supports IPO/LPO. LTO is on by default now. Most modern compilers support it - and can make good use of it and is usually makes the code faster and/or larger. + the user compiler supports IPO/LPO. LTO is on by default now. Most modern compilers support it, + can make good use of it and it usually makes the code faster and/or smaller. PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/616 - Linux HAL: Add wiretapping option for I2C. Enabled with `FSFW_HAL_I2C_WIRETAPPING` defined to 1 - Dedicated Version class and constant `fsfw::FSFW_VERSION` containing version information From fd112ed5973bb522a351ed42e1e89995c484d3ad Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 9 May 2022 16:07:05 +0200 Subject: [PATCH 5/8] enable lto for test target --- CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 93f8c24f..2f4ffe75 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -106,6 +106,9 @@ if(FSFW_BUILD_UNITTESTS) project(${FSFW_TEST_TGT} CXX C) add_executable(${FSFW_TEST_TGT}) + if(IPO_SUPPORTED AND FSFW_ENABLE_IPO) + set_property(TARGET ${FSFW_TEST_TGT} PROPERTY INTERPROCEDURAL_OPTIMIZATION TRUE) + endif() if(FSFW_TESTS_GEN_COV) message(STATUS "Generating coverage data for the library") From 0fe1b70baedfe250d5d39be3e5545bab3806a9f0 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 10 May 2022 11:19:29 +0200 Subject: [PATCH 6/8] keep LTO option off by default --- CHANGELOG.md | 10 ++++++++++ CMakeLists.txt | 17 +++++++++-------- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0452e58b..f3523a73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,16 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - LTO support: Allow using LTO/IPO by setting `FSFW_ENABLE_LTO=1`. CMake is able to detect whether the user compiler supports IPO/LPO. LTO is on by default now. Most modern compilers support it, can make good use of it and it usually makes the code faster and/or smaller. + After some more research: + Enabling LTO will actually cause the compiler to only produce thin LTO by adding + `-flto -fno-fat-lto-objects` to the compiler options. I am not sure this is an ideal choice + because if an application linking against the FSFW does not use LTO, there can be compile + issues (e.g. observed when compiling the FSFW tests without LTO). This is a known issue as + can be seen in the multiple CMake issues for it: + - https://gitlab.kitware.com/cmake/cmake/-/issues/22913, + - https://gitlab.kitware.com/cmake/cmake/-/issues/16808, + - https://gitlab.kitware.com/cmake/cmake/-/issues/21696 + Easiest solution for now: Keep this option OFF by default. PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/616 - Linux HAL: Add wiretapping option for I2C. Enabled with `FSFW_HAL_I2C_WIRETAPPING` defined to 1 - Dedicated Version class and constant `fsfw::FSFW_VERSION` containing version information diff --git a/CMakeLists.txt b/CMakeLists.txt index 9f686d1d..61145e1e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -17,11 +17,12 @@ set(FSFW_REVISION 0) # Add the cmake folder so the FindSphinx module is found set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake" ${CMAKE_MODULE_PATH}) +set(FSFW_ETL_LIB_NAME etl) set(FSFW_ETL_LIB_MAJOR_VERSION 20 CACHE STRING - "ETL library major version requirement" + "ETL library major version requirement" ) set(FSFW_ETL_LIB_VERSION ${FSFW_ETL_LIB_MAJOR_VERSION}.27.3 CACHE STRING - "ETL library exact version requirement" + "ETL library exact version requirement" ) set(FSFW_ETL_LINK_TARGET etl::etl) @@ -32,26 +33,26 @@ set(FSFW_CATCH2_LIB_VERSION v${FSFW_CATCH2_LIB_MAJOR_VERSION}.0.0-preview5 CACHE "Catch2 library exact version requirement" ) -set(FSFW_ETL_LIB_NAME etl) - include(CheckIPOSupported) check_ipo_supported(RESULT IPO_SUPPORTED OUTPUT IPO_ERROR) if(NOT IPO_SUPPORTED) message(STATUS "FSFW | IPO/LTO not supported: ${IPO_ERROR}") endif() -option(FSFW_ENABLE_IPO "Enable interprocedural optimization or link-time optimization if available" ON) +# Keep this off by default for now. See PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/616 +# for information which keeping this on by default is problematic +option(FSFW_ENABLE_IPO "Enable interprocedural optimization or link-time optimization if available" OFF) option(FSFW_GENERATE_SECTIONS - "Generate function and data sections. Required to remove unused code" ON + "Generate function and data sections. Required to remove unused code" ON ) if(FSFW_GENERATE_SECTIONS) - option(FSFW_REMOVE_UNUSED_CODE "Remove unused code" ON) + option(FSFW_REMOVE_UNUSED_CODE "Remove unused code" ON) endif() option(FSFW_BUILD_UNITTESTS "Build unittest binary in addition to static library" OFF) option(FSFW_BUILD_DOCS "Build documentation with Sphinx and Doxygen" OFF) if(FSFW_BUILD_UNITTESTS) - option(FSFW_TESTS_GEN_COV "Generate coverage data for unittests" ON) + option(FSFW_TESTS_GEN_COV "Generate coverage data for unittests" ON) endif() option(FSFW_WARNING_SHADOW_LOCAL_GCC "Enable -Wshadow=local warning in GCC" ON) From 25775614de4bf6e7d11e70f8b15965247b9a51ee Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 10 May 2022 11:56:51 +0200 Subject: [PATCH 7/8] only check IPO support if enabled --- CMakeLists.txt | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 61145e1e..e1ab522a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -33,15 +33,18 @@ set(FSFW_CATCH2_LIB_VERSION v${FSFW_CATCH2_LIB_MAJOR_VERSION}.0.0-preview5 CACHE "Catch2 library exact version requirement" ) +# Keep this off by default for now. See PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/616 +# for information which keeping this on by default is problematic +option(FSFW_ENABLE_IPO "Enable interprocedural optimization or link-time optimization if available" OFF) + +if(FSFW_ENABLE_IPO) include(CheckIPOSupported) check_ipo_supported(RESULT IPO_SUPPORTED OUTPUT IPO_ERROR) if(NOT IPO_SUPPORTED) message(STATUS "FSFW | IPO/LTO not supported: ${IPO_ERROR}") endif() +endif() -# Keep this off by default for now. See PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/616 -# for information which keeping this on by default is problematic -option(FSFW_ENABLE_IPO "Enable interprocedural optimization or link-time optimization if available" OFF) option(FSFW_GENERATE_SECTIONS "Generate function and data sections. Required to remove unused code" ON ) From e05c72b062836eb5f9cd4cdc1d35b0a2846653e0 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 10 May 2022 13:08:14 +0200 Subject: [PATCH 8/8] minor formatting fix --- CMakeLists.txt | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e1ab522a..3cca727e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -36,13 +36,12 @@ set(FSFW_CATCH2_LIB_VERSION v${FSFW_CATCH2_LIB_MAJOR_VERSION}.0.0-preview5 CACHE # Keep this off by default for now. See PR: https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/616 # for information which keeping this on by default is problematic option(FSFW_ENABLE_IPO "Enable interprocedural optimization or link-time optimization if available" OFF) - if(FSFW_ENABLE_IPO) -include(CheckIPOSupported) -check_ipo_supported(RESULT IPO_SUPPORTED OUTPUT IPO_ERROR) -if(NOT IPO_SUPPORTED) - message(STATUS "FSFW | IPO/LTO not supported: ${IPO_ERROR}") -endif() + include(CheckIPOSupported) + check_ipo_supported(RESULT IPO_SUPPORTED OUTPUT IPO_ERROR) + if(NOT IPO_SUPPORTED) + message(STATUS "FSFW | IPO/LTO not supported: ${IPO_ERROR}") + endif() endif() option(FSFW_GENERATE_SECTIONS