[Openmp-commits] [PATCH] D14254: [OpenMP] Initial implementation of OpenMP offloading library - libomptarget device RTLs.

[George Rokos via Phabricator via Openmp-commits]

grokos added inline comments.


================
Comment at: libomptarget/deviceRTLs/nvptx/CMakeLists.txt:116-122
+    else()
+      find_program(LIBOMPTARGET_NVPTX_SELECTED_CUDA_COMPILER clang++)
+      if(NOT LIBOMPTARGET_NVPTX_SELECTED_CUDA_COMPILER)
+        libomptarget_say("Cannot find a CUDA compiler capable of emitting LLVM bitcode.")
+        libomptarget_say("Please configure with flag -DLIBOMPTARGET_NVPTX_CUDA_COMPILER")
+      endif()
+    endif()
----------------
Hahnfeld wrote:
> (Probably this case will go then too...)
I left it there. The logic now is as follows:

  # If the user has specified a compiler via `-DLIBOMPTARGET_NVPTX_CUDA_COMPILER` then we'll use that.
  # If not, then we check whether `CMAKE_C_COMPILER` is clang; if yes, then we'll use that.
  # Otherwise, we try to search for a clang compiler. If we don't find any, then we don't build bclib.




================
Comment at: libomptarget/deviceRTLs/nvptx/CMakeLists.txt:124-139
+    if (NOT LIBOMPTARGET_NVPTX_BC_LINKER STREQUAL "")
+      set(LIBOMPTARGET_NVPTX_SELECTED_BC_LINKER ${LIBOMPTARGET_NVPTX_BC_LINKER})
+    elseif(NOT ${LIBOMPTARGET_STANDALONE_BUILD})
+        if(MSVC)
+          set(LIBOMPTARGET_NVPTX_SELECTED_BC_LINKER ${LLVM_TOOLS_BINARY_DIR}/llvm-link.exe)
+        else()
+          set(LIBOMPTARGET_NVPTX_SELECTED_BC_LINKER ${LLVM_TOOLS_BINARY_DIR}/llvm-link)
----------------
Hahnfeld wrote:
> 1. Do not depend on in-tree components.
> 2. I think you should be able to do the "link" step with Clang as well, can you please test this?
  # Changed the linker-setting logic to the same as the compiler-setting logic above.
  # Clang can be used to link but with different command line options. For consistency I am sticking to `llvm-link`. My rationale is that if we are building libomptarget with clang then `llvm-link` must also be around, so if `CMAKE_C_COMPILER_ID` equals "Clang", then use `llvm-link`.



================
Comment at: libomptarget/deviceRTLs/nvptx/src/libcall.cu:401-402
+EXTERN int omp_test_lock(omp_lock_t *lock) {
+  // int atomicCAS(int* address, int compare, int val);
+  // (old == compare ? val : old)
+  int compare = UNSET;
----------------
Hahnfeld wrote:
> What's this comment about?
It explains how compare-and-swap works.


Repository:
  rL LLVM

https://reviews.llvm.org/D14254