[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: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:
> grokos wrote:
> > 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`.
> > 
> I disagree: `CMAKE_C_COMPILER` doesn't need to be in `PATH` so we might not be able to find `llvm-link` or end up with a different installation. And using `clang` for linking would be consistent with the usual behavior (build system invokes compiler with object files and the compiler driver invokes the linker). Conclusion: It is better and possible to use `clang` for linking and we should do it.
After looking into this, from what I could find clang cannot link bitcode files into a single bitcode file - this can only be done with `llvm-link`. What clang can do is link multiple `.bc` files into a target-specific object file (which is not what we want here) and for this it needs `libLTO` and the `LLVM gold` plugin (which are not necessarily installed on every system and asking the user to provide paths makes things more complex). If anyone knows of a simple way to make clang link multiple `.bc` files into a single bitcode library, please let me know. Otherwise, I'm afraid we'll have to stick to `llvm-link`.


Repository:
  rL LLVM

https://reviews.llvm.org/D14254