Set LTO option only if it's supported and the build is not a debug build #1002
+31
−7
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, the CMakeLists.txt files added `-flto` to some of the compilation targets unconditionally. The addition should only be done if the compiler supports LTO, and it shouldn't be done if the cmake configuration is the debug configuration.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request correctly refactors the CMake build configuration to conditionally enable Link-Time Optimization (LTO). It introduces a check for LTO support and uses the INTERPROCEDURAL_OPTIMIZATION property, which is correctly disabled for debug builds. This is a good improvement for build robustness and developer experience. My main feedback is about code duplication introduced across the various CMakeLists.txt files for different CPU architectures. I've suggested a refactoring to abstract this logic into a reusable function to improve maintainability, in line with the project's style guide.
mhucka
changed the title
The CMake files now check if OpenMP can be used. The Makefiles should too.
github-actions
bot
added
size: M
github-actions
bot
added
size: S
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, the CMakeLists.txt files added
-fltoto some of the compilation targets unconditionally. The addition should only be done if the compiler supports LTO, and it shouldn't be done if the cmake configuration is the debug configuration.This PR moves the detection of availability of LTO to the top-level CMakeLists.txt file, and defines a CMake macro to set it in the sub-CMakeLists.txt files. The macro avoids repeating some gnarly CMake syntax all over the place.