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

[Michael Kruse via Phabricator via Openmp-commits]

Meinersbur added inline comments.


================
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>
----------------
tianshilei1992 wrote:
> 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`.
Adding project-specific options is already done for `COMPILER_RT`. This seems to be the established approach.


================
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.
----------------
tianshilei1992 wrote:
> 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.
As mentioned in the summary, automatically using the host compiler may result in unpredictable LLVM-IR that e.g. include vendor extensions. That is, the C/C++ to host-assembly compiler is just the wrong tool for CUDA-to-LLVM-IR compilation.

I think the only clang we would want to support is the clang from that same git commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101663