[PATCH] D101960: [openmp] Drop requirement on library path environment variables

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:648
 
+void tools::addOpenMPRuntimeSpecificRPath(const ToolChain &TC,
+                                          const ArgList &Args,
----------------
Similar to other functions in this file, derived from aomp (by deleting some stuff for finding a debug version of the library)

I can make my peace with runpath instead if people are keen to override this with LD_LIBRARY_PATH. The rest of clang's toolchains wire things up with rpath but that doesn't mean we have to make the same choice here.


================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1703
 
+  // Add path to runtimes binary folder, used by ENABLE_RUNTIMES build
+  SmallString<256> RuntimesBinPath = llvm::sys::path::parent_path(D.Dir);
----------------
Otherwise we need LIBRARY_PATH to run from the build tree, which is awkward for tests and for ad hoc running from the build tree. This path is checked last, so only changes behaviour from error message to success when the file exists here. Split out in D101935


================
Comment at: clang/tools/amdgpu-arch/CMakeLists.txt:17
 
+set_target_properties(amdgpu-arch PROPERTIES INSTALL_RPATH_USE_LINK_PATH ON)
+
----------------
Lets amdgpu-arch work from the build tree, split out in D101926


================
Comment at: openmp/libomptarget/src/rtl.cpp:76
 
+  std::string full_plugin_name;
+  void *handle = dlopen("libomptarget.so", RTLD_NOW);
----------------
This logic is cut from D73657 without addressing any of the review comments. Idea is to look for the offloading plugins next to libomptarget, instead of in dlopen's default search path. Will address the previous comments when splitting out to a separate patch.


================
Comment at: openmp/libomptarget/test/lit.cfg:72
     config.test_flags += " -Wl,-rpath," + config.omp_host_rtl_directory
 else: # Unices
+    config.test_flags += " -Wl,-rpath," + config.library_dir
----------------
For copy & paste of a RUN line from a failing test


================
Comment at: openmp/libomptarget/test/lit.cfg:182
+
+if config.cuda_path and config.cuda_path != 'CUDA_TOOLKIT_ROOT_DIR-NOTFOUND':
   config.substitutions.append(("%cuda_flags", "--cuda-path=" + config.cuda_path))
----------------
Because otherwise we write `--cuda-path=CUDA_TOOLKIT_ROOT_DIR-NOTFOUND` at the top of non-nvptx tests, which is harmless but ugly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101960