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

[Michael Kruse via Phabricator via Openmp-commits]

Meinersbur 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
----------------
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.


================
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)
----------------
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.


================
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()
----------------
`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)
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130926