[Openmp-commits] [PATCH] D95376: [OpenMP][Libomptarget] Fix check-libomptarget

[Giorgis Georgakoudis via Phabricator via Openmp-commits]

ggeorgakoudis added inline comments.


================
Comment at: openmp/libomptarget/CMakeLists.txt:78
   set(LIBOMPTARGET_OPENMP_HEADER_FOLDER "${CMAKE_CURRENT_BINARY_DIR}/../runtime/src")
+  set(LIBOMPTARGET_OPENMP_HOST_RTL_FOLDER "${LIBOMP_LIBRARY_DIR}")
 endif()
----------------
vzakhari wrote:
> tianshilei1992 wrote:
> > What about using `"${CMAKE_CURRENT_BINARY_DIR}/../runtime/src"` directly, like the one above?
> It looks like we should keep it as a cache variable, so that users may actually override it via cmake command even when the host runtime is also being built in-tree.  But then it will probably not help to resolve the `Support` dependency...
> 
> Alternatively, I guess, we can just keep the documentation untouched making it clear that `LIBOMPTARGET_OPENMP_HOST_RTL_FOLDER` is only honored for out-of-tree builds.
Thanks for the comment Shilei! The path `${CMAKE_CURRENT_BINARY_DIR}/../runtime/src` is correct for out-of-tree builds but not for in-tree ones. I've been actually digging more and perhaps the right way to go is to remove 
```
if(OPENMP_STANDALONE_BUILD)
```
and unify those branch to:
```
set(LIBOMPTARGET_OPENMP_HEADER_FOLDER "${LIBOMP_INCLUDE_DIR}")
set(LIBOMPTARGET_OPENMP_HOST_RTL_FOLDER "${LIBOMP_LIBRARY_DIR}")
```
since both `LIBOMP_INCLUDE_DIR`, `LIBOMP_LIBRARY_DIR` are set in libomp building and available at this scope. Does that make sense?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95376