[Openmp-commits] [PATCH] D55725: [OpenMP] Add libs to clang-dedicated directories

Sat Jan 26 07:02:38 PST 2019

Hahnfeld added a comment.

I think it's a good idea to have a proper RFC on cfe-dev about how to handle runtime libraries in case they are already installed in system directory. More pointers on historic discussions below.

In D55725#1371344 <https://reviews.llvm.org/D55725#1371344>, @protze.joachim wrote:

> In D55725#1370148 <https://reviews.llvm.org/D55725#1370148>, @jdenny wrote:
>
> > - `libgomp.so`, `libiomp5.so`: My understanding is that these symlinks exist solely for backward compatibility.  This patch currently doesn't affect them (doesn't bother to install them to Clang-dedicated directories).  Any reason to change that?
>
>
> From my perspective these names are not for backward compatibility, but to trick GNU and Intel compiler to use the LLVM/OpenMP runtime, when it is found before their own OpenMP runtime. By adding the path of libomp.so to the LIBRARY_PATH, gcc/icc will pick up the LLVM/OpenMP runtime during compilation.
>
> As long as libgomp does not implement OMPT and LLVM has no Fortran frontend, I would welcome the possibility to easily link this runtime into Fortran applications compiled with the GNU compiler.

I also value this use-case, and I think that should continue to work with this patch: The intention is to put libraries that Clang cares about early in the library search hierarchy. As other Compiler (Intel Compiler, GCC) don't know about and don't look into "Clang-dedicated" directories, it should be sufficient to have the symlinks in `lib/`? (which is not going to change)

In D55725#1371349 <https://reviews.llvm.org/D55725#1371349>, @jdenny wrote:

> 2. Still, my proposal is that placement of Clang's libraries early in the search path should be the default.  I've shown why that's good.  Is it ever bad?  Is there ever a case where the user expects system libraries to have precedence over Clang's own libraries by default?  I would guess a user should specify `-L` for that, but maybe some user installs alternative libraries ahead in the current path and expects that to be sufficient.

My memories are also coming back here: Searching for abandoned revisions resulted in quite some interesting patches that I authored two years ago:

- https://reviews.llvm.org/D26244: Basically the idea was to prefer Clang's libraries in `lib/` over the system default. Here's what I think was the most important point raised by Chandler: https://reviews.llvm.org/D26244#641099. I don't think this applies to the current approach because no other project should be installing to `lib/clang/`...

In the following two revisions I actually proposed something similar for `libc++`, maybe some points are relevant (can't remember all the details, but the conclusions seems to be that the `libc++` maintainers didn't like installing to a different directory):

- https://reviews.llvm.org/D30214
- https://reviews.llvm.org/D30733

In D55725#1371370 <https://reviews.llvm.org/D55725#1371370>, @jdenny wrote:

> In D55725#1370823 <https://reviews.llvm.org/D55725#1370823>, @Hahnfeld wrote:
>
> > Are you saying a manual `-L` trumps the environment variable `LIBRARY_PATH`? Yes, that makes sense, I guess.
>
>
> While that's true, that's not the point I was making here.  You suggested I set `LIBRARY_PATH` to find Clang's `libomp.so`, so I set it to my installed library directory, `/home/jdenny/llvm/lib`.  My point here is that Clang reads `LIBRARY_PATH` and adds `-L/home/jdenny/llvm/lib` to the linker command line //after// `-L/home/jdenny/llvm/bin/../lib`, which it adds regardless, so `LIBRARY_PATH` proves redundant.
>
> In other words, `LIBRARY_PATH` generally doesn't seem helpful for Clang's own `lib` directory, even when there's no conflicting system library.  Sorry, maybe that's not helpful to point out.

Okay, got it this time. Thanks for the explanation, I missed what the ordering implies here.

>> Please note that `libomp.so.5` is an invention of Debian. None of upstream's CMake files will ever create a symlink with that name!
>>  (We could think about adding such symlink, there was a complaint sometime ago that we don't have versioned shared libraries. But that wouldn't solve all problems, the linker would still use the "wrong" file.)
> 
> Let me see if I understand this.  Sorry if I have it all mixed up.  For some `N`, if Clang looks for `libomp.so.N` instead of `libomp.so`, and if we could be sure that all copies of `libomp.so.N` that might be seen are ABI-compatible, and if no such `libomp.so.N` is yet another symlink, then it shouldn't matter if the linker uses the wrong file, and all that matters is that `LD_LIBRARY_PATH` chooses the right one.  Is that right?  I feel like that ABI-compatibility assumption is shaky, but I don't have enough practical experience here.

>From my understanding the general idea is to have `libomp.so -> libomp.so.N`. When linking an application the linker resolves the link chain and adds `libomp.so.N` as a dependency. The assumption is that these versioned libraries will stay ABI-compatible for all times.
The point is that the OpenMP runtime libraries have a much stronger guarantee: The goal is to never break the ABI, we're only adding new functions and utilize additional bits that were left unused when introducing a new interface.
I don't really know why the Debian maintainers decided to add such symlink on their own, but that's part of the problem.

However, only using `LD_LIBRARY_PATH` during runtime might not be sufficient. Imagine there is a new version of the OpenMP standard which introduces a new feature. A new function is added to `libomp` and Clang is taught to generate code, calling the new function.
If linking ends up using the old `libomp` installed in a system directory, `ld` cannot find the new symbols and will fail. So I think ABI-compatibility is not equivalent to "the right library for linking" in all cases. Does that make sense?

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

https://reviews.llvm.org/D55725