[flang-commits] [PATCH] D146485: Add tests from gfortran test suite [build files]

[Kiran Chandramohan via Phabricator via flang-commits]

kiranchandramohan added a comment.

LGTM. Thanks for your effort in setting this up.

Will the changes to https://github.com/llvm/llvm-test-suite/blob/main/LICENSE.TXT come in a separate patch. i.e listing gfortran tests as test having a different license?

The summary will need minor updates (like enabled on AArch64 as well).

Otherwise, the split into `Unsupported, Unimplemented, Skipped, Failing` looks OK. Not sure whether we need to rename `Skipped/Failing` to make it clear that `Skipped` is build failures and `Failing` is runtime failures.

BTW, the detailed characterisation (even though it is probably at a coarse-level) looks very good and possibly a good source for folks to look at and start fixing issues.



================
Comment at: Fortran/gfortran/CMakeLists.txt:1
+# Check the version of Ninja since newer versions have proper Fortran support.
+include(CheckFortranCompilerFlag)
----------------
tarunprabhu wrote:
> kiranchandramohan wrote:
> > Ninja or cmake version?
> > 
> > We should have a cmake version check (CMake 3.24.0) for the version that enabled it. Or is that what the following check does?
> That comment is completely wrong. I confused it with something else. Removed. 
Can we add a minimum CMake version (3.24.0 I think) check? I had to download the latest cmake version to get this to work. 
https://cmake.org/cmake/help/latest/command/cmake_minimum_required.html


================
Comment at: Fortran/gfortran/CMakeLists.txt:186
+find_file(ISO_FORTRAN_C_HEADER
+  ISO_Fortran_binding.h
+  PATHS ${Fortran_PREFIX}/include/flang
----------------
tarunprabhu wrote:
> kiranchandramohan wrote:
> > My check-flang build did not have this and I had to copy it over. Is this copied over in the normal build or an install?
> > ninja check-flang vs ninja vs ninja install.
> It is definitely available with ninja install. I am not sure with just Ninja, I will check. 
> 
> Do you have any suggestions on how this can be dealt with? I think many of the flang developers only run check-flang builds, right?
Yes, I generally run check-flang. This is fine for now, we can check with others whether this needs to be moved. Possible option is to add this in the build step. I think similar copying of files to build directory is in `flang/tools/f18/CMakeLists.txt`.


Repository:
  rT test-suite

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146485/new/

https://reviews.llvm.org/D146485