[Openmp-commits] [PATCH] D148778: [flang][openmp] Fortran offloading test

[Slava Zakharin via Phabricator via Openmp-commits]

vzakhari added inline comments.


================
Comment at: openmp/libomptarget/test/lit.cfg:97
+  config.available_features.add('flang')
+  tools.append(ToolSubst('%flang', command=FindTool('flang-new'), unresolved='fatal'))
+
----------------
elmcdonough wrote:
> vzakhari wrote:
> > I think you have to make sure that `check-libomptarget` CMake target has a dependency on whatever target produces `flang-new` tool.  Alternatively, you may follow the same logic that is used for `clang` with `config.test_*_compiler` properties, but then it does not really matter if Flang is in `LLVM_ENABLE_PROJECTS`.
> I wanted to make it so that the flang dependency is optional.  The offloading tests should be run if flang is an enabled project, otherwise, they should be ignored.  I had a `config.test_fortran_compiler` in my original build that inherited from `CMAKE_Fortran_COMPILER` in standalone builds and looked for `flang-new` in the binary dir with CMake's `find_program` for in-tree builds.  Unfortunately, that version didn't work in certain circumstances.
> 
> The current revision seems to work better and is cleaner IMO, but I'm open to the idea of adding an extra build flag for fortran tests if others think it would work better.  @jdoerfert Any thoughts on this?
> ... and looked for flang-new in the binary dir with CMake's find_program for in-tree builds. Unfortunately, that version didn't work in certain circumstances.
Right, without the CMake dependency between `check-libomptarget` and `flang-new` this will not work when `flang-new` is built later than all prerequisites for `check-libomptarget` are satisfied.
I believe the current version has the same issue.

One of the solutions is to add the target dependency in `libomptarget/test/CMakeLists.txt` when `flang` is present in `LLVM_ENABLE_PROJECTS`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148778