In preparation for Flang compiler, add support for Fortran in CMake. Include a simple Fortran90 hello world test. Tested with GFortran.
Details
Diff Detail
- Repository
- rOLDT svn-test-suite
- Build Status
Buildable 84242 Build 114583: arc lint + arc unit
Event Timeline
CMakeLists.txt | ||
---|---|---|
32 | Aren't CMAKE variables by convention upper case only? |
CMakeLists.txt | ||
---|---|---|
9–13 | Did you consider using enable_language? It supports an OPTIONAL flag for autodetect the presence of a Fortran compiler. (Not suggesting to use it, it might break buildbots that happen to have gfortran installed, and you will probably not want to run tests for a well-tested distribution provided compiler). | |
30 | [Style] In other places in this file there is no space between if and ( | |
31 | There is probably a reason why the _COMPILER vars are marked as advanced, they cannot be changed after the initial configuration. I don't see a reason to make it non-advanced. | |
42 | [not a change requirest] While good here for consistency with C/CXX, newer Cmake should use add_compile_definitions since _FLAGS is supposed to be controlled by the user, and TEST_SUITE_EXTRA_Fortran_FLAGS not be necessary. | |
197 | [nit] unrelated whitespace change | |
SingleSource/UnitTests/Fortran/CMakeLists.txt | ||
2 | Isn't this redundant with what llvm_singlesource does anyway? |
CMakeLists.txt | ||
---|---|---|
32 | Weirdly, cmake defines these as CMAKE_<Lang>_COMPILER, where <Lang> is Fortran, in Pascal case. Also see https://cmake.org/cmake/help/latest/variable/CMAKE_LANG_COMPILER.html and explcitly shown at https://cmake.org/cmake/help/latest/variable/CMAKE_LANG_FLAGS.html. |
CMakeLists.txt | ||
---|---|---|
9–13 | Yes, I looked into that, but the OPTIONAL flag does not work at the moment according to the CMake docs. https://cmake.org/cmake/help/latest/command/enable_language.html | |
30 | I will fix this. | |
31 | Was this just a comment or where there any action you wanted me to to take? Here I am just following a parallel constructions similar to the prior mark_as_advance(...). | |
32 | As @Meinersbur notes, for some reason CMake uses Fortran instead of FORTRAN. | |
42 | Thanks for the comments. I noted some other older CMake-isms but did not want to change to much in the first pass. | |
197 | I will fix this. | |
SingleSource/UnitTests/Fortran/CMakeLists.txt | ||
2 | Good question and I didn't know the answer until after I did a little bit of experimentation. Source is an input variable into llvm_singlesource(), without it the target is built in the wrong directory and CMake complains. The structure of CMakeLists.txt is similar to what is done elsewhere in the testsuite: |
CMakeLists.txt | ||
---|---|---|
9–13 | I will make this change as well. |
CMakeLists.txt | ||
---|---|---|
31 | OK, didn't notice this is also done for CMAKE_C_COMPILER, CMAKE_CXX_COMPILER. I don't this is good, but keeps the Fortran support consistent with C/CXX, like modifying _FLAGS. | |
SingleSource/UnitTests/Fortran/CMakeLists.txt | ||
2 | There's something wrong if this is not working without defining Source. See cmake/modules/SingleMultiSource.cmake: if(DEFINED Source) set(sources ${Source}) else() file(GLOB sources *.c *.cpp *.cc *.f *.F *.f90 *.F90) endif() i.e. if Source is undefined, it does the file search by itself. Defining Source should only be necessary if there are additional source files that should not be compiled. I don't know why SingleSource/UnitTests/C++11 does it this way. The majority of other programs rely on that it does work. |
- Use CMake enable_language instead.
- Fix style on conditionals.
- Cleaner implementation of Fortran unit test example.
SingleSource/UnitTests/Fortran/CMakeLists.txt | ||
---|---|---|
2 | Thanks for pushing on this. The problem was that Fortran tests came at the end of the CMakeLists.txt file, so unbeknownst to me this line in the parent CMakeLIsts.txt file was interfering: I just moved up the Fortran test to be enabled before this line. |
LGTM.
I expected more changes to be necessary, to make it work with litsupport modules, perf/timeit, statistics, compile time (Interestingly, CMAKE_Fortran_COMPILE_OBJECT is already defined), LNT, ... . But if we find they don't work yet, we can fix that later.
Did you consider using enable_language? It supports an OPTIONAL flag for autodetect the presence of a Fortran compiler. (Not suggesting to use it, it might break buildbots that happen to have gfortran installed, and you will probably not want to run tests for a well-tested distribution provided compiler).