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: link to MPI::MPI_Fortran using INTERFACE #3853

Merged
merged 2 commits into from
Oct 24, 2023

Conversation

caitlinross
Copy link
Collaborator

combining fortran and cpp files in the same target was causing fortran flags to be applied to cpp files, which could cause build failures with clang. Fixes issue #3253

@vicentebolea
Copy link
Collaborator

@caitlinross thanks for the bugfix I think that this should go to our next patch release. I want to understand better this, I am trying to reproduce this locally but I cannot seem to hit the error, in which env do we get this build error?

@caitlinross
Copy link
Collaborator Author

caitlinross commented Oct 18, 2023

I run into the issue on mac with AppleClang 14.0.3 and gfortran 13.1.0. I just learned that AppleClang has differences from normal clang, so maybe it's not generally an issue with clang?

@vicentebolea
Copy link
Collaborator

@caitlinross I see, do you think that a generator expression at the top cmake_fortran_flag that only applies the flag when is language:Fortran will do the the trick? That would be more concise.

@caitlinross
Copy link
Collaborator Author

I don't think so. So I tried

set(CMAKE_Fortran_FLAGS "${CMAKE_Fortran_FLAGS} $<$<COMPILE_LANGUAGE:Fortran>:-fallow-argument-mismatch>")

I get the following error:

CMake Error:
   Running

    '/opt/homebrew/bin/ninja' '-C' '/Users/caitlin.ross/dev/adios/fortran-fix/build' '-t' 'recompact'

   failed with:

    ninja: error: build.ninja:5867: bad $-escape (literal $ must be written as $$)
     FLAGS = $<$<COMPILE_LANGUAGE:Fortran>: -fallow-argument-mismatch> -O3 ...
             ^ near here

I don't think it's a mistake in the generator expression itself, but maybe I'm doing something else wrong there?

But I did a bit more digging and realized it's only happening when MPI is enabled. So if not building with MPI, it seems that -fallow-argument-mismatch is correctly only added to the f90 files and none of the cpp files. But when MPI is turned on, that flag is for some reason being added to the cpp files (but only adios2_f2c_adios_mpi.cpp and adios2_f2c_io_mpi.cpp, not the non-mpi f2c files). I have no idea why that flag is being added only for those 2 files.

@caitlinross
Copy link
Collaborator Author

It appears that flag is being added to those 2 cpp files from somewhere else. I commented out the set(CMAKE_Fortran_FLAGS "${CMAKE_Fortran_FLAGS} -fallow-argument-mismatch) line and I am still getting the error about the unknown argument when building those files.

@vicentebolea
Copy link
Collaborator

It appears that flag is being added to those 2 cpp files from somewhere else. I commented out the set(CMAKE_Fortran_FLAGS "${CMAKE_Fortran_FLAGS} -fallow-argument-mismatch) line and I am still getting the error about the unknown argument when building those files.

This is very odd since we only add this flag once at this CMakeLists.txt file. I think that the culcrip is when we link the library with the MPI::MPI_Fortran, I believe that this will inject the CMAKE_Fortran_FLAGS.

How about this approach. We keep the same targets but we do not set CMake_Fortran_FLAGS, we instead set that flag for all the targets in the file using the property: https://cmake.org/cmake/help/latest/prop_tgt/COMPILE_OPTIONS.html#prop_tgt:COMPILE_OPTIONS

We can use a generator expression to set the property only for Fortran files. This will be more concised. I wish I could try this but I cannot reproduce this in my setup. I tried different combinations and no luck.

@caitlinross
Copy link
Collaborator Author

So I tried a bunch of things that didn't work and finally found the really simple solution that was overlooked. In target_link_libraries MPI::MPI_Fortran should be INTERFACE not PUBLIC.

Copy link
Collaborator

@vicentebolea vicentebolea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Thanks for getting to the bottom of this 👍

@caitlinross caitlinross changed the title separate fortran and cpp files into their own targets cmake: link to MPI::MPI_Fortran using INTERFACE Oct 23, 2023
@caitlinross caitlinross merged commit 4907f2b into ornladios:master Oct 24, 2023
32 of 33 checks passed
@caitlinross caitlinross deleted the fix-clang-fortran-build branch October 24, 2023 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants