From 5b09725f5168d01d75395ec97ee4db4bf55e07e6 Mon Sep 17 00:00:00 2001 From: Anthony Islas <128631809+islas@users.noreply.github.com> Date: Fri, 10 Jan 2025 13:27:09 -0700 Subject: [PATCH] CMake Fix split command flags to be correctly populated (#2108) TYPE: bug fix KEYWORDS: cmake, mpi, compilation SOURCE: internal DESCRIPTION OF CHANGES: Problem: The `arch/configure_reader.py` does the job of parsing, organizing, and sanitizing input from configuration stanzas into a CMake toolchain file which can then be used to inform the build about which compilers, flags, and options to use. Occasionally, stanzas fields inject flags into a compiler or other command field (like `DM_FC`) so that the actual command is a command + flags. Part of the `arch/configure_reader.py` organization is breaking these up into separable sections automatically. With the example of `DM_FC = mpif90 -f90=gfortran` (`$(SFC)` already expanded) this _should_ be broken into `DM_FC = mpif90` and `DM_FC_FLAGS = -f90=gfortran`. Currently, the `*_FLAGS` field when split out for certain keys in a stanza is not populated due to using the wrong index from the Python `str.partition()` call. Secondly, when these fields are actually provided to CMake compilation breaks for MPI specifically. Since the MPI "compilers" are wrappers, they are then interrogated for the underlying flags and options meaning further adding the flags back into compilation results in things like `gfortran -f90=gfortran`. This is incorrect, and instead the flags should be provided to the MPI flags used during wrapper interrogation on a per-language basis. Furthermore, for certain MPI implementations supplying any flags renders the query command (e.g. `-show`, `-showme`, or `-compileinfo`) useless. For instance, OpenMPI `mpif90 -f90=gfortran -show` only outputs `gfortran -f90=gfortran` which is also wrong. Solution: 1. The Python call to split the fields should take the actual values 2. MPI flags per language should be supplied to the interrogation flags `MPI__COMPILER_FLAGS` if needed 3. The `FindMPI` module already feeds in the underlying compiler specification for wrappers that support it so flags like `-f90=$(SFC)` should be filtered out from `DM_*_FLAGS` before being written to the `wrf_config.cmake` toolchain file TESTS CONDUCTED: 1. Flags in specific command fields are now split and appearing in the `wrf_config.cmake` file 2. MPI compilation works with compiler specification filtered out but correct underlying compiler still selected even when multiple compilers are in the same environment (note: this was the original behavior but now is deliberate) --- CMakeLists.txt | 19 ++++++++----------- arch/configure_reader.py | 22 +++++++++++++++++++++- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e889c01ff6..53432fa1c9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -405,23 +405,20 @@ if ( ${USE_MPI} ) # Which may or may not get polluted by the environment # It still technically finds MPI but the output is nonintuitive # saying things like hdf5 or pthread - find_package( MPI REQUIRED COMPONENTS Fortran C ) - list( APPEND PROJECT_COMPILE_DEFINITIONS_OPTIONS - USE_MPI=1 - DM_PARALLEL - ) + # Supply any language-specific flags for interrogation if ( DEFINED WRF_MPI_Fortran_FLAGS AND NOT "${WRF_MPI_Fortran_FLAGS}" STREQUAL "" ) - list( APPEND PROJECT_COMPILE_OPTIONS_OPTIONS - $<$:${WRF_MPI_Fortran_FLAGS}> - ) + set( MPI_Fortran_COMPILER_FLAGS ${WRF_MPI_Fortran_FLAGS} ) endif() if ( DEFINED WRF_MPI_C_FLAGS AND NOT "${WRF_MPI_C_FLAGS}" STREQUAL "" ) - list( APPEND PROJECT_COMPILE_OPTIONS_OPTIONS - $<$:${WRF_MPI_C_FLAGS}> - ) + set( MPI_C_COMPILER_FLAGS ${WRF_MPI_C_FLAGS} ) endif() + find_package( MPI REQUIRED COMPONENTS Fortran C ) + list( APPEND PROJECT_COMPILE_DEFINITIONS_OPTIONS + USE_MPI=1 + DM_PARALLEL + ) # Check if MPI in all its glory has forced IPO down our throats due to hard-coding the wrapper flags # https://www.open-mpi.org/faq/?category=mpi-apps#why-no-rpath LOL! diff --git a/arch/configure_reader.py b/arch/configure_reader.py index f5e3efa153..0da6e52efa 100755 --- a/arch/configure_reader.py +++ b/arch/configure_reader.py @@ -141,7 +141,7 @@ def splitIntoFieldAndFlags( self, field ) : fieldValue = self.kvPairs_[ field ] self.kvPairs_[field] = fieldValue.partition(" ")[0] - self.kvPairs_[field + "_FLAGS"] = fieldValue.partition(" ")[1] + self.kvPairs_[field + "_FLAGS"] = fieldValue.partition(" ")[2] ###################################################################################################################### ## @@ -182,6 +182,26 @@ def sanitize( self ) : # And for final measure strip self.kvPairs_[ key ] = self.kvPairs_[ key ].strip() + # Finally, further sanitize MPI compilers, we don't need to specify underlying + # compiler since CMake already does that + filters = [ + self.kvPairs_[ "SFC" ], + self.kvPairs_[ "SCC" ], + "-compiler" + ] + keysToSanitize = [ "DM_FC_FLAGS", "DM_CC_FLAGS" ] + + for keyToSan in keysToSanitize : + if self.kvPairs_[ keyToSan ] : + allFlags = self.kvPairs_[ keyToSan ].split( " " ) + newFlags = [] + for flag in allFlags : + if not any( [ f in flag for f in filters ] ) : + newFlags.append( flag ) + + # We always need this field updated + self.kvPairs_[ keyToSan ] = " ".join( newFlags ) + def serialCompilersAvailable( self ) : return which( self.kvPairs_["SFC"] ) is not None and which( self.kvPairs_["SCC"] ) is not None