blt icon indicating copy to clipboard operation
blt copied to clipboard

Incorrect concatenation of link flags in SetupMPI.cmake

Open klevzoff opened this issue 5 years ago • 1 comments

CMake: 3.14.3 BLT: https://github.com/LLNL/blt/commit/938345715810c6457d3e57a2c20d4ee0fa253632

Link flags for C, C++ and Fortran obtained from their respective compiler wrappers are concatenated with a ; using list(APPEND ...):

https://github.com/LLNL/blt/blob/938345715810c6457d3e57a2c20d4ee0fa253632/cmake/thirdparty/SetupMPI.cmake#L84

However MPI_C_LINK_FLAGS, MPI_CXX_LINK_FLAGS and MPI_Fortran_LINK_FLAGS are space-separated strings, not lists to begin with. So it ends up with something like -a -b -c;-a -b, which is not processed correctly by separate_arguments later in blt_add_target_link_flags - you end up with -c;-a added as a link flag.

There used to be a line replacing ; with a space, but it was removed in https://github.com/LLNL/blt/commit/1e6c7ef3b9d39033b02ca8e8b3633f1066f7ff7e .

I suggest replacing list(APPEND ...) with string(APPEND ...) in the line above and adding a space, at least it fixes it for me:

   set(_mpi_link_flags ${MPI_C_LINK_FLAGS})
    if (NOT "${MPI_C_LINK_FLAGS}" STREQUAL "${MPI_CXX_LINK_FLAGS}")
        string(APPEND _mpi_link_flags " ${MPI_CXX_LINK_FLAGS}")
    endif()
    if (ENABLE_FORTRAN)
        if (NOT "${MPI_C_LINK_FLAGS}" STREQUAL "${MPI_Fortran_LINK_FLAGS}")
            string(APPEND _mpi_link_flags " ${MPI_Fortran_LINK_FLAGS}")
        endif()
    endif()

In addition, I think Fortran was intended instead of CXX on the following line: https://github.com/LLNL/blt/blob/938345715810c6457d3e57a2c20d4ee0fa253632/cmake/thirdparty/SetupMPI.cmake#L88

klevzoff avatar Jul 25 '19 04:07 klevzoff

With list():

-- BLT MPI Link Flags:     -Xlinker --enable-new-dtags -Xlinker -rpath -Xlinker /usr/local/Intel_16/compilers_and_libraries_2017.1.132/linux/mpi/intel64/lib/release_mt -Xlinker -rpath -Xlinker /usr/local/Intel_16/compilers_and_libraries_2017.1.132/linux/mpi/intel64/lib -Xlinker -rpath -Xlinker /opt/intel/mpi-rt/2107.0.0/intel64/lib/release_mt -Xlinker -rpath -Xlinker /opt/intel/mpi-rt/2017.0.0/intel64/lib;-Xlinker --enable-new-dtags -Xlinker -rpath -Xlinker /usr/local/Intel_16/compilers_and_libraries_2017.1.132/linux/mpi/intel64/lib/release_mt -Xlinker -rpath -Xlinker /usr/local/Intel_16/compilers_and_libraries_2017.1.132/linux/mpi/intel64/lib -Xlinker -rpath -Xlinker /opt/intel/mpi-rt/2017.0.0/intel64/lib/release_mt -Xlinker -rpath -Xlinker /opt/intel/mpi-rt/2017.0.0/intel64/lib

With string()

-- BLT MPI Link Flags:     -Xlinker --enable-new-dtags -Xlinker -rpath -Xlinker /usr/local/Intel_16/compilers_and_libraries_2017.1.132/linux/mpi/intel64/lib/release_mt -Xlinker -rpath -Xlinker /usr/local/Intel_16/compilers_and_libraries_2017.1.132/linux/mpi/intel64/lib -Xlinker -rpath -Xlinker /opt/intel/mpi-rt/2107.0.0/intel64/lib/release_mt -Xlinker -rpath -Xlinker /opt/intel/mpi-rt/2017.0.0/intel64/lib -Xlinker --enable-new-dtags -Xlinker -rpath -Xlinker /usr/local/Intel_16/compilers_and_libraries_2017.1.132/linux/mpi/intel64/lib/release_mt -Xlinker -rpath -Xlinker /usr/local/Intel_16/compilers_and_libraries_2017.1.132/linux/mpi/intel64/lib -Xlinker -rpath -Xlinker /opt/intel/mpi-rt/2017.0.0/intel64/lib/release_mt -Xlinker -rpath -Xlinker /opt/intel/mpi-rt/2017.0.0/intel64/lib

(if you look closely, the semicolon in the middle in intel64/lib;-Xlinker is now a space)

klevzoff avatar Jul 25 '19 05:07 klevzoff