[Openmp-commits] [PATCH] D101663: [OpenMP] Avoid unintentional use of host compiler as bclib compiler.

Fri Apr 30 13:58:51 PDT 2021

tianshilei1992 requested changes to this revision.
tianshilei1992 added inline comments.
This revision now requires changes to proceed.

================
Comment at: llvm/runtimes/CMakeLists.txt:238
                                       -DCMAKE_ASM_COMPILER_WORKS=ON
+                                      -DLIBOMPTARGET_NVPTX_CUDA_COMPILER=$<TARGET_FILE:clang>
+                                      -DLIBOMPTARGET_NVPTX_BC_LINKER=$<TARGET_FILE:llvm-link>
----------------
I don't think it's a good idea to "pollute" LLVM CMake files for this purpose. There are plenty of ways to tell whether OpenMP is built via `LLVM_ENABLE_RUNTIMES`. I'd set the two CMake variables in OpenMP by checking whether we're in `LLVM_ENABLE_RUNTIMES`.

================
Comment at: openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt:36
   set(cuda_compiler "$<TARGET_FILE:clang>")
-elseif(${CMAKE_C_COMPILER_ID} STREQUAL "Clang")
-  # Compile the device runtime with the compiler that OpenMP is built with.
----------------
Removing this can cause issue if I compile OpenMP standalone. We cannot assume people all compile OpenMP along with LLVM either with `LLVM_ENABLE_RUNTIMES` or `LLVM_ENABLE_PROJECTS`.
Like I said in your previous patch, we need a mechanism to check whether the provided `clang` is qualified.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101663