[Openmp-commits] [PATCH] D130926: [OpenMP][libomp] Copy OpenMP headers to clang build directory

[Jonathan Peyton via Phabricator via Openmp-commits]

jlpeyton added inline comments.


================
Comment at: openmp/runtime/src/CMakeLists.txt:221
+# so it can find & use them before install
+if (NOT OPENMP_STANDALONE_BUILD)
+  # Had to manually copy this since CLANG_VERSION may be empty
----------------
Meinersbur wrote:
> A non-standalone build doesn't mean that clang is built as well. `LLVM_TOOL_CLANG_BUILD` might be what you are looking for (like https://github.com/llvm/llvm-project/blob/eb5aeee02f70369c1e9b15cfafb3d100494dbb28/openmp/libomptarget/DeviceRTL/CMakeLists.txt#L40). However, it will not be set in an `LLVM_ENABLE_RUNTIMES=openmp` build. 
> 
> It not a big issue, at worst we have an unused `omp.h` in the build directory.
Thanks! I've added `LLVM_TOOL_CLANG_BUILD` and also a check for `LLVM_RUNTIMES_BUILD` which LLVM defines for the runtimes to know that `LLVM_ENABLE_RUNTIMES=...` is occurring.


================
Comment at: openmp/runtime/src/CMakeLists.txt:226
+  set(patch "${LLVM_VERSION_PATCH}")
+  set(output_dir ${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${major}.${minor}.${patch}/include)
+  function(copy_header_to_output_dir src_dir file)
----------------
Meinersbur wrote:
> In an `LLVM_ENABLE_RUNTIMES=openmp` configuration, does `LLVM_LIBRARY_OUTPUT_INTDIR` point to build the dir where clang is built?
> 
> I think it would be ok to only copy over `omp.h`/`omp-tools.h` in `LLVM_ENABLE_PROJECTS=openmp` builds for now.
Conveniently, it does point to the correct directory for `LLVM_ENABLE_RUNTIMES=openmp`


================
Comment at: openmp/runtime/src/CMakeLists.txt:237
+  if(LIBOMP_OMPT_SUPPORT)
+    copy_header_to_output_dir(${CMAKE_CURRENT_BINARY_DIR} omp-tools.h)
+  endif()
----------------
Meinersbur wrote:
> `omp-tools.h` is also installed as `ompt.h`
> 
> ```
>   # install under legacy name ompt.h
>   install(FILES ${CMAKE_CURRENT_BINARY_DIR}/omp-tools.h DESTINATION ${LIBOMP_HEADERS_INSTALL_PATH} RENAME ompt.h)
> ```
Added ompt.h to list of things to copy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130926