[flang-commits] [PATCH] D154373: [llvm-test-suite][Fortran/gfortran] Refactor build system

[Leandro Lupori via Phabricator via flang-commits]

luporl added a comment.

Thanks for the refactoring!
LGTM overall, but I'm hitting a compilation error on AArch64 (see comment).



================
Comment at: Fortran/gfortran/CMakeLists.txt:129
+  -mfpmath=387
+  -mfpmath=sse
   # -Os might eventually be supported, so this might also need to be removed
----------------
I get an error on AArch64 when `-mtune=amdfam10` is used. It seems to be an X86-specific flag.


================
Comment at: Fortran/gfortran/CMakeLists.txt:240
+# called from each subdirectory containing tests. It is expected that the
+# subdirectory will contain a fail named "DisabledFiles.cmake" which will
+# provide the list of files that are disabled in that subdirectory for various
----------------
nit: s/fail/file/


================
Comment at: Fortran/gfortran/CMakeLists.txt:536-546
+      elseif (line MATCHES "dg-additional-sources[ ]*[\"]?(.+)[\"]?[ ]*[}]")
+        separate_arguments(others UNIX_COMMAND ${CMAKE_MATCH_1})
+        list(TRANSFORM others STRIP)
+      elseif (line MATCHES "dg-(additional-)?options [{]?[ ]*\"([^\"]*)\"[ ]*[}]?(.*)")
+        # TODO: We completely ignore any target-specific options that may be
+        # present. These are usually in the form of target directives. For now,
+        # it seems to be ok, but as these tests are enabled on more platforms,
----------------
It seems there is some code for parsing DejaGNU's directives that is common between compile and execute tests.
As it's currently only a few lines, it doesn't seem worth to move it to a new function now, but it's something to consider in the future, if more common directives appear.


Repository:
  rT test-suite

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

https://reviews.llvm.org/D154373