Skip to content
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

CMake set release flags properly #2138

Merged
merged 3 commits into from
Dec 20, 2024
Merged

Conversation

islas
Copy link
Collaborator

@islas islas commented Dec 3, 2024

TYPE: bug fix

KEYWORDS: cmake, flags, optimization

SOURCE: internal

DESCRIPTION OF CHANGES:
Problem:
To feed initial flags into the cmake build, a toolchain file is used.

However, for particular build configurations, namely release and debug, despite the configuration being "Release" and "Debug", respectively, the corresponding CMAKE_<LANG>_FLAGS_<CONFIG>_INIT has the configuration name capitalized.

Additionally, these initial flags supplement the starting flags and do not override any inherited flags that CMake automatically appends. This can cause issues with certain compilers where subsequent flags that normally should supersede previously listed options are ignored. As an example, using nvfortran/pgi the following will cause forced optimization, which is not viable for some files within WRF :
https://forums.developer.nvidia.com/t/nvfortran-reducing-optimation-level-by-multiple-on-does-not-work/191825
https://forums.developer.nvidia.com/t/how-to-override-pgccs-optimization-flag/136275

Finally, certain files do not have optimization overridden even if it were to work correctly

Solution:
Ensure correct variable name in toolchain file to provide initial starting flags, and wipe CMake-injected release flags to avoid significant deviation from provided optimization flags. Add proper directory scope to files that need these reduced optimizations.

TESTS CONDUCTED:

  1. Tested with CMake build on Derecho selecting nvhpc/pgi compiler stanzas

RELEASE NOTE: Override CMake-injected optimization flags in favor of the flags set by the build system and provided stanza information.

islas added 3 commits December 2, 2024 13:06
Depending on architecture and compiler, CMake may prepend flags into
global flags. While this may work for general CMake projects, the use
of arch/configure.defaults allows for specific control of flag options
where additional unintended optimization flags may negatively impact
compilation, performance, or even model validity.
@islas islas requested review from a team as code owners December 3, 2024 22:19
@amstokely amstokely self-requested a review December 10, 2024 22:00
@islas islas merged commit b6542b0 into wrf-model:develop Dec 20, 2024
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants