[Openmp-commits] [PATCH] D110251: [OpenMP][CMake] Use in-project clang as CUDA->IR compiler for new DeviceRTL.

[Shilei Tian via Phabricator via Openmp-commits]

tianshilei1992 added a comment.

Generally looks better than previous patch. Thanks for that. This patch assumes those targets must be there in runtime build, which might not be the case. I think it could be better if we first check if those targets are there. If not, raise an error directly, saying runtime build requires `clang`, etc.



================
Comment at: openmp/libomptarget/DeviceRTL/CMakeLists.txt:46
+  # Compile the device runtime with the compiler that OpenMP is built with.
+  # This is the case with LLVM_ENABLE_RUNTIMES=openmp.
+  # FIXME: This is unreliable; the compiler can be on older version of clang
----------------
> This is the case with LLVM_ENABLE_RUNTIMES=openmp.
This is not always the case. Standalone build also falls to this branch.


================
Comment at: openmp/libomptarget/DeviceRTL/CMakeLists.txt:75
+
+if (EXISTS "${compiler_dir}/opt" AND NOT IS_DIRECTORY "${compiler_dir}/opt")
+  # Try to use the opt consistent with the CUDA compiler.
----------------
`${compiler_dir}/opt` appears three times. Can we set a variable for that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110251