[Openmp-commits] [PATCH] D111983: [libomptarget][DeviceRTL] Generalise and simplify cmakelists

Mon Oct 18 11:44:23 PDT 2021

JonChesterfield added inline comments.

================
Comment at: openmp/libomptarget/DeviceRTL/CMakeLists.txt:34-37
-set(LIBOMPTARGET_NVPTX_CUDA_COMPILER "" CACHE STRING
-  "Location of a CUDA compiler capable of emitting LLVM bitcode.")
-set(LIBOMPTARGET_NVPTX_BC_LINKER "" CACHE STRING
-  "Location of a linker capable of linking LLVM bitcode objects.")
----------------
Meinersbur wrote:
> JonChesterfield wrote:
> > Meinersbur wrote:
> > > This removes support for `LIBOMPTARGET_NVPTX_CUDA_COMPILER`/`LIBOMPTARGET_NVPTX_BC_LINKER` settings? Instead, it uses clang found in `LLVM_TOOLS_BINARY_DIR`, which very much prefer over `${CMAKE_C_COMPILER}`. This this work with an LLVM_DIR to a pre-installed LLVM in `/usr` or `/usr/local`? 
> > > 
> > Changes the flag yeah. This is openmp code - no particular reason why NVPTX_CUDA_COMPILER is a good name for which one to use.
> > 
> > The cmake will have a go with whatever LLVM_DIR is set, whether that will succeed or not depends on whether that compiler is recent enough to deal with the openmp features used. Variant is probably the limiting factor.
> > 
> > We're trying to encourage people to build this via ENABLE_RUNTIMES as than ensures that the clang used can actually build it, but there are still these hooks if people want to try with a different one. 
> Have you seen https://lists.llvm.org/pipermail/llvm-dev/2021-October/153238.html ?
> 
> I tried the "Default" build with openmp and it did not work:
> ```
> CMake Error at /home/meinersbur/src/llvm-project/openmp/runtime/src/CMakeLists.txt:314 (string):
>   string sub-command REGEX, mode MATCH needs at least 5 arguments total to
>   command.
> ```
I read that earlier today but haven't tried to work out the implications for openmp. 

> string(REGEX MATCH "[0-9]+\\.[0-9]+(\\.[0-9]+)?" CLANG_VERSION ${PACKAGE_VERSION})

I don't know offhand what that's trying to do. Seems unrelated to this patch? 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111983