[Openmp-commits] [PATCH] D122667: llvm14 patch: hwloc include directory for libompd

[Shilei Tian via Phabricator via Openmp-commits]

tianshilei1992 added a comment.

In D122667#3455047 <https://reviews.llvm.org/D122667#3455047>, @bernhardkaindl wrote:

> This is the minimum fix for exporting `${LIBOMP_HWLOC_INSTALL_DIR}/include` thru the `omp` target for import thru `target_link_libraries`.
>
>   --- openmp/runtime/src/CMakeLists.txt
>   +++ openmp/runtime/src/CMakeLists.txt
>   @@ -46,9 +46,6 @@
>      ${LIBOMP_INC_DIR}
>      ${LIBOMP_SRC_DIR}/thirdparty/ittnotify
>    )
>   -if(${LIBOMP_USE_HWLOC})
>   -  include_directories(${LIBOMP_HWLOC_INSTALL_DIR}/include)
>   -endif()
>   
>    # Building with time profiling support requires LLVM directory includes.
>    if(LIBOMP_PROFILING_SUPPORT)
>   @@ -157,6 +154,13 @@
>      # libomp must be a C++ library such that it can link libLLVMSupport
>      set(LIBOMP_LINKER_LANGUAGE CXX)
>    endif()
>   +if(${LIBOMP_USE_HWLOC})
>   +  target_include_directories(omp
>   +                             PUBLIC
>   +                             "$<BUILD_INTERFACE:${LIBOMP_HWLOC_INSTALL_DIR}/include>"
>   +                             "$<INSTALL_INTERFACE:${LIBOMP_HWLOC_INSTALL_DIR}/include>"
>   +  )
>   +endif()
>   
>   
>    if(OPENMP_MSVC_NAME_SCHEME)
>   --- openmp/libompd/src/CMakeLists.txt
>   +++ openmp/libompd/src/CMakeLists.txt
>   @@ -13,7 +13,7 @@
>   
>    add_library (ompd SHARED TargetValue.cpp omp-debug.cpp omp-state.cpp omp-icv.cpp)
>   
>   -add_dependencies(ompd omp) # ensure generated import library is created first
>   +target_link_libraries(ompd omp) # ensure generated import library is created first
>   
>    set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")
>
> This (by comparison) relatively modern cmake style is used in the llvm pstl and mlir libraries already, It is needed to attach the include path to the `omp`  CMake target, so `target_link_libraries` can pick it up from the target to use `omp`is used by another target.

Thanks for the comment. Yes, `target_link_libraries` can only work if the libraries are properly configured. Now it looks like `libomp` is not configured properly.

The good news is, based on https://discourse.llvm.org/t/rfc-stand-alone-build-support/, if we want to use external library, we have to reply on CMake instead of manual configured twisted CMake commands. It's about time to modernize the whole CMake structure of OpenMP, but of course it's beyond the scope of this patch.


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

https://reviews.llvm.org/D122667