Adding a Non-trivial Fortran Test to llvm-test-suite
As of this CMake merge request, there is now CMake support for LLVMFlang. This allows for the integration of LLVMFlang into the llvm-test-suite. To this end, this request for review takes advantage of this new CMake functionality by adding a proxy application called SNAP, which aims "to model the performance of a modern discrete ordinates neutral particle transport application". This application was picked because of its importance to Flang developers, and because there were folks who were already using it to test Flang's correctness in producing end-to-end binaries.
A recipe for building the llvm-test-suite is listed below:
#!/usr/bin/env bash LLVM_TS_BUILD_DIR="/path/to/where/you/want/llvm-test-suite/to/be/built" FLANG_PATH="/path/to/your/build/of/flang" FC="${FLANG_PATH}/bin/flang-new" PATH_TO_LIBPGMATH="/path/to/libpgmath" CMAKE_FORTRAN_FLAGS="{PATH_TO_LIBPGMATH}/libpgmath.so -flang-experimental-exec" CMAKE_OPTIONS="-GNinja \ -DCMAKE_C_COMPILER=clang \ -DCMAKE_CXX_COMPILER=clang++ \ -DCMAKE_Fortran_COMPILER=${FC} \ -DTEST_SUITE_FORTRAN=on \ -DTEST_SUITE_SUBDIRS=Fortran \ -DCMAKE_BUILD_RPATH=${PATH_TO_LIBPGMATH}" mkdir -p ${LLVM_TS_BUILD_DIR} cd ${LLVM_TS_BUILD_DIR} cmake ${CMAKE_OPTIONS} -DCMAKE_Fortran_FLAGS="${CMAKE_FORTRAN_FLAGS}" .. cmake --build . -j64
NOTE: You can save yourself some build time by just specifying -DTEST_SUITE_SUBDIRS=Fortran so you don't have to build the entire llvm-test-suite, but this option can be omitted if you want to build the whole thing.
NOTE: An option called PGMATH is specified. For now, this is required, but may not be if we decide to remove libpgmath being built _as part_ of the llvm-test-suite.
The major components of this review request are:
+ adding the necessary source files from the SNAP project repository
+ CMakeLists.txt for the SNAP proxy application
- Adding SNAP Source Files
- Location
I have taken the SNAP project repo and placed it in llvm-test-suite/Fortran.
Files
From the SNAP project repo, I have taken the files in src and added them to the llvm-test-suite. I have also explicitly added a LICENSE.md file. Based on the license, we should be able to include the source in llvm-test-suite. I have removed some stuff files from the original SNAP repo that really shouldn't have been added -- thank you @rovka for the feedback! -- but there are some other files that I have included from the SNAP repo that I have kept in the revision (but ultimately probably shouldn't be there):
- docs
- qasnap (the input test files that aren't qasnap/mms_src/2d_mms_st.inp)
- really, there's only one input that we test, which is listed above. It would be possible to test other inputs to act as additional regression tests, but I would agree with the sentiment that the other inputs can be removed and then subsequently added if folks wanted to try more inputs.
CMake
I've removed already modified some of the CMake stuff that @rovka pointed out wasn't really necessary, but here are the salient bits of the CMake build infrastructure that I've added.
There are two files: Fortran/SNAP/CMakeLists.txt and Fortran/SNAP/src/CMakeLists.txt.
- src/CMakeLists.txt
- Why not llvm_test_executable
There are methods that allow you to build targets that consist of multiple source files. In general, though, using this call does not give enough fine-grained control over the target and how it is compiled/linked.
Additionally, this question breaks down into:
- Why doesn't llvm_test_executable_no_test work?
- this CMake function only mutates the global CFLAGS, CPPFLAGS, and CXXFLAGS variables, which doesn't apply to Fortran. Not using llvm_test_executable_no_test frees me up to use functions like target_compile_options, target_link_libraries`, and the like.
- Why doesn't llvm_add_tests_for_target work?
- this function assumes that the name of the target is also the base name for the input file, which is not the case for SNAP. Since this assumption doesn't hold, I have to roll my own CMake for this bit, though admittedly there is a lot of overlap since the name is the only issue.
For future applications that require more fine-grained control of parameterizing the target, it might make sense to create a .cmake file geared for Fortran based on the work I've done here. But I'll call that future work and outside the scope of this PR.
TESTSCRIPT
The RUN: and VERIFY: commands that eventually get run are, in part, based on work done by @awarzynski and others. Specifically, I am running the application with the input that they have verified and validated to be correct. As noted above, I am using the SNAP/qasnap/mms_src/2d_mms_st.inp file.
libpgmath with ExternalProject_Add
Currently, using LLVMFlang means that you need to externally link libpgmath so that those symbols are defined. Since this is the current state of Flang, I have added the functionality to do this without the programmer/developer having to supply the locaiton of a libpgmath build themselves. As @rovka points out, I hope that this isn't always the case. But since it is, that's why I've made the design choice. I am happy to remove it in this PR, or to leave it and then remove it once the math intrinsics are properly handled within LLVMFlang. Let's keep it that way.
This is interesting, but I wouldn't say it's the test-suite's job to install libpgmath. It's not needed by the tests themselves, but by the compiler (and hopefully not for long). I think it's reasonable to assume that whoever wants to run the test-suite with flang already has libpgmath installed and they can provide whatever CMAKE_Fortran_FLAGS are needed to make things work (see e.g. my experimental buildbot running the test-suite)