[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

Wed Feb 22 07:19:45 PST 2023

jdoerfert added a comment.

I'm worried this makes use of LLVM on HPC machines even harder. That said, I'm open to suggestions and I am not well versed in all the ways we can make it work.
Our problem is that there are N `libomptarget.so` files and N `libomptarget.nvptx.so` files on a system, including in system directories and in directories you have on your LD_LIBRARY_PATH.
However, we want a clang to pick up its own versions of those files. The former is linked into clang, the latter is dynamically loaded with dlopen. That is, IIRC, roughly our use case.

> I'd argue that such systems should specify -Wl,-rpath explicitly or in a Clang configuration file.

Could you work me through this, please. We can't install a config file in a user or system directory. So all we have is the clang install directory.
Should we not set this flag but then install a file (by default) that says `-Wl,-rpath=...`. Is that what you mean? If so, what's the difference for the user?
Or would we add `--offload-add-rpath` to the clang build if OpenMP offload is enabled?

================
Comment at: clang/include/clang/Driver/Options.td:4218-4223
 defm openmp_implicit_rpath: BoolFOption<"openmp-implicit-rpath",
   LangOpts<"OpenMP">,
-  DefaultTrue,
+  DefaultFalse,
   PosFlag<SetTrue, [], "Set rpath on OpenMP executables">,
   NegFlag<SetFalse>,
   BothFlags<[NoArgumentUnused]>>;
----------------
JonChesterfield wrote:
> MaskRay wrote:
> > yaxunl wrote:
> > > I am wondering whether this option can be aliased to `--offload-add-rpath`, which seems to have the same purpose. (https://reviews.llvm.org/D136854)
> > It can be aliased to `--[no-]offload-add-rpath`, but I think it probably makes sense to drop this option completely. This patch as-is is useful for backporting into 16.x.
> Folding the flags together seems good to me, I'd have probably used offload-add-rpath in the initial implementation if I spotted it.
Merging them makes sense. FWIW, this flag was introduced 10 months earlier (I think).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143306