RFC: Place libs in Clang-dedicated directories (affects openmp, libcxx, libunwind, compiler-rt)

classic Classic list List threaded Threaded
28 messages Options
12
Reply | Threaded
Open this post in threaded view
|

RFC: Place libs in Clang-dedicated directories (affects openmp, libcxx, libunwind, compiler-rt)

Bakhvalov, Denis via cfe-dev
Hi,

This RFC discusses placing Clang's libraries in Clang-dedicated
directories (somewhere within `lib/clang` instead of `lib`) in both
build trees and install trees.  While fixing broken linking with
openmp libraries was the original motivation, subprojects potentially
affected by that problem or this change also include libcxx,
libunwind, and compiler-rt.

Motivation: Broken Default Linking
----------------------------------

Clang by default prefers system openmp libraries over the ones built
alongside Clang. The effect is that Clang-compiled openmp applications
sometimes misbehave if the user doesn't adjust the library search
path.

For example, where `/home/jdenny/llvm` is built and installed from git
master, but where my system openmp libraries under
`/usr/lib/x86_64-linux-gnu` are from LLVM 6.0:

```
$ cd /home/jdenny/llvm
$ cat test.c
#include <stdio.h>
int main() {
  #pragma omp target teams
  printf("hello\n");
  return 0;
}
$ ./bin/clang -fopenmp -fopenmp-targets=nvptx64 test.c
$ ./a.out
./a.out: error while loading shared libraries: libomptarget.so: cannot
open shared object file: No such file or directory
$ LD_LIBRARY_PATH=lib ./a.out
Segmentation fault (core dumped)
$ LD_LIBRARY_PATH=lib ldd a.out | grep libomp
        libomp.so.5 => /usr/lib/x86_64-linux-gnu/libomp.so.5
(0x00007fab59748000)
        libomptarget.so => lib/libomptarget.so (0x00007fab59515000)
```

The problem here is a little subtle.  `-v` reveals that Clang
specifies `-L/usr/lib/x86_64-linux-gnu` before
`-L/home/jdenny/llvm/lib` when linking, and I have:

```
$ ls -l /usr/lib/x86_64-linux-gnu/libomp.so
lrwxrwxrwx 1 root root 11 Jan  7  2018
/usr/lib/x86_64-linux-gnu/libomp.so -> libomp.so.5
```

As a result, Clang links the executable as follows:

```
$ readelf -d a.out | grep libomp.so
 0x0000000000000001 (NEEDED)             Shared library: [libomp.so.5]
```

Now `LD_LIBRARY_PATH` cannot force `a.out` to find Clang's `libomp.so`
because `a.out` wants `libomp.so.5` instead.

As far as I know, there's nothing unusual about my system, which is a
typical Ubuntu 18.04.2 installation.  However, I have learned from
others that `libomp.so.5` is installed by Debian's packages not by
LLVM's cmake files [1], so many people might not see this particular
problem.

Even if `/usr/lib/x86_64-linux-gnu/libomp.so` weren't a symbolic link,
it's still the wrong library, and incompatibilities could have caused
a link failure instead of a run-time failure.

Either way, it seems surprising and confusing that Clang doesn't
always prefer its own libraries.

Solution: Early `-L` for Clang-Dedicated Directory
--------------------------------------------------

The easiest solution is probably to assume users will manually specify
`-L` when necessary to link against the right `libomp.so`.  However,
it seems like Clang should just do that automatically to help
unsuspecting users avoid surprises like the one above.

I wrote a patch to make that happen [2].  The patch's basic approach
is to place `libomp .so` in a directory for which, when linking, Clang
automatically specifies a `-L` earlier than any `-L` for a system
library directory.  It's important that the directory is a
Clang-dedicated directory (somewhere under `lib/clang`) so that other
libraries unrelated to Clang are not accidentally elevated in the
library search path.  When possible, the patch makes this change for
both the build tree and install tree so users can work out of either
tree.

This solution seems straight-forward, but it raises many questions.

Q1: What is the right Clang-dedicated directory?
------------------------------------------------

There are many possibilities.  On my system:

* `lib`: This directory is obviously not Clang-dedicated, but it's
where libcxx, libunwind, and openmp currently place their libraries by
default.

* `lib/clang/9.0.0/lib/linux`: This directory is where compiler-rt
libraries are placed by default, and their names are like
`libclang_rt.asan-x86_64.so`.

* `lib/clang/9.0.0/x86_64-unknown-linux-gnu/lib`: When the cmake
option `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` is set, this
directory is where compiler-rt, libcxx, and libunwind libraries are
placed, and the compiler-rt libraries then drop the redundant
`-x86_64` from their names.

* `lib/clang/9.0.0/lib/linux/x86_64`: This directory has the nice
advantage that `-frtlib-add-rpath` tells Clang to add a `-rpath` for
this directory so you don't have to set `LD_LIBRARY_PATH` when
executing `a.out`.  However, it turns out that `-rpath` doesn't work
correctly for me in the case of openmp offloading [3].  As far as I
can tell, this directory is currently populated only by some Android
tool chains [3].

* For libcxx and libunwind libraries, it's been suggested that a
Clang-dedicated directory should not be specific to the Clang version
(shouldn't contain `9.0.0`) because the libraries themselves are not
locked to the Clang version, and a possible directory layout has been
proposed [4].  My understanding is that this approach generally makes
sense when the shared libraries have version numbers, like
`libomp.so.5`.  In that case, it needs to be possible to find older
shared libraries that older software has already become dependent upon
while using older Clang installations.  It's easier to find them, with
`LD_LIBRARY_PATH` for example, if every Clang installation places such
libraries in the same Clang-version-independent directory, where the
libraries are still distinct due to the version numbers in their
names, instead of placing them in a bunch of different
Clang-version-specific directories.  Please correct me if I've
misunderstood this situation.

Which choice is right for openmp libraries?  Debian is giving version
numbers to `libomp.so`, but is that right?  If it is, the last choice
might make sense.  If not, perhaps one of the previous choices is
better.

Q2: Should `lib` also contain the libraries?
--------------------------------------------

As mentioned above, `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` moves
libcxx, libunwind, and compiler-rt libraries to their new
Clang-dedicated directory.  However, the patch I've proposed for
openmp instead duplicates libraries into their new Clang-dedicated
directory.  I figured duplicating would be better for backward
compatibility than moving.  Is it necessary?

(Actually, I should probably be creating symlinks instead of duplicates.)

Q3: Is it really OK to elevate Clang's libraries?
-------------------------------------------------

For me, this is the biggest unknown about this change.  That is, does
any user depend on being able to drop his own `libomp.so` in a system
library directory so that Clang will prefer it over Clang's own
`libomp.so`?  If so, is that an important enough use case that we
should continue to risk subtly breaking the compiles of other users by
default?

Q4: Should libcxx, libunwind, or compiler-rt be changed too?
------------------------------------------------------------

An early reaction to my patch was that we need to either handle the
various Clang libraries consistently or show why they should be
handled differently.  My patch so far only changes openmp, and it does
not make it consistent with other subprojects' libraries.

Keep in mind that, while compiler-rt libraries are already placed in a
Clang-dedicated directory by default, libcxx and libunwind libraries
are not and so could potentially be affected by the motivating example
I gave.  Must users specify
`-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` (which adds the problem of
Clang-version locking) to avoid that problem?  Shouldn't the default
configuration be the most user-friendly?

Conclusion
----------

So far, my estimate at the best path forward is as follows:

* openmp and compiler-rt libraries should always be placed in a
Clang-dedicated, Clang-version-specific directory.

* libcxx and libunwind libraries should always be placed in a
Clang-dedicated, Clang-version-independent directory.

* For each of those directories, Clang should specify `-L` earlier
than any system library directory.

* openmp, libcxx, and libunwind libraries should also be symlinked
directly in `lib`, for backward compatibility at least.

* `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` should be dropped and
dependent projects updated.  Alternatively, for backward
compatibility, this option could revert the Clang-dedicated directory
layout to the layout it currently produces (in particular, it places
libcxx and libunwind libraries in Clang-version-specific directories).

There are details like exact directory names to discuss, but does this
high-level plan make sense?

Thanks.

Joel

[1]: https://reviews.llvm.org/D55725#1370823
[2]: https://reviews.llvm.org/D55725
[3]: https://reviews.llvm.org/D55725#1371807
[4]: https://reviews.llvm.org/D30733#697781
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Place libs in Clang-dedicated directories (affects openmp, libcxx, libunwind, compiler-rt)

Bakhvalov, Denis via cfe-dev
Hi Joel,

Am Donnerstag, den 14.02.2019, 17:29 -0500 schrieb Joel E. Denny:

> Hi,
>
> This RFC discusses placing Clang's libraries in Clang-dedicated
> directories (somewhere within `lib/clang` instead of `lib`) in both
> build trees and install trees.  While fixing broken linking with
> openmp libraries was the original motivation, subprojects potentially
> affected by that problem or this change also include libcxx,
> libunwind, and compiler-rt.
>
> Motivation: Broken Default Linking
> ----------------------------------
>
> Clang by default prefers system openmp libraries over the ones built
> alongside Clang. The effect is that Clang-compiled openmp applications
> sometimes misbehave if the user doesn't adjust the library search
> path.
>
> For example, where `/home/jdenny/llvm` is built and installed from git
> master, but where my system openmp libraries under
> `/usr/lib/x86_64-linux-gnu` are from LLVM 6.0:
>
> ```
> $ cd /home/jdenny/llvm
> $ cat test.c
> #include <stdio.h>
> int main() {
>   #pragma omp target teams
>   printf("hello\n");
>   return 0;
> }
> $ ./bin/clang -fopenmp -fopenmp-targets=nvptx64 test.c
> $ ./a.out
> ./a.out: error while loading shared libraries: libomptarget.so: cannot
> open shared object file: No such file or directory
> $ LD_LIBRARY_PATH=lib ./a.out
> Segmentation fault (core dumped)
> $ LD_LIBRARY_PATH=lib ldd a.out | grep libomp
>         libomp.so.5 => /usr/lib/x86_64-linux-gnu/libomp.so.5
> (0x00007fab59748000)
>         libomptarget.so => lib/libomptarget.so (0x00007fab59515000)
> ```
>
> The problem here is a little subtle.  `-v` reveals that Clang
> specifies `-L/usr/lib/x86_64-linux-gnu` before
> `-L/home/jdenny/llvm/lib` when linking, and I have:
>
> ```
> $ ls -l /usr/lib/x86_64-linux-gnu/libomp.so
> lrwxrwxrwx 1 root root 11 Jan  7  2018
> /usr/lib/x86_64-linux-gnu/libomp.so -> libomp.so.5
> ```
>
> As a result, Clang links the executable as follows:
>
> ```
> $ readelf -d a.out | grep libomp.so
>  0x0000000000000001 (NEEDED)             Shared library: [libomp.so.5]
> ```
>
> Now `LD_LIBRARY_PATH` cannot force `a.out` to find Clang's `libomp.so`
> because `a.out` wants `libomp.so.5` instead.
>
> As far as I know, there's nothing unusual about my system, which is a
> typical Ubuntu 18.04.2 installation.  However, I have learned from
> others that `libomp.so.5` is installed by Debian's packages not by
> LLVM's cmake files [1], so many people might not see this particular
> problem.
>
> Even if `/usr/lib/x86_64-linux-gnu/libomp.so` weren't a symbolic link,
> it's still the wrong library, and incompatibilities could have caused
> a link failure instead of a run-time failure.
>
> Either way, it seems surprising and confusing that Clang doesn't
> always prefer its own libraries.
>
> Solution: Early `-L` for Clang-Dedicated Directory
> --------------------------------------------------
>
> The easiest solution is probably to assume users will manually specify
> `-L` when necessary to link against the right `libomp.so`.  However,
> it seems like Clang should just do that automatically to help
> unsuspecting users avoid surprises like the one above.
+1 that Clang should get this right in most cases.

> I wrote a patch to make that happen [2].  The patch's basic approach
> is to place `libomp .so` in a directory for which, when linking, Clang
> automatically specifies a `-L` earlier than any `-L` for a system
> library directory.  It's important that the directory is a
> Clang-dedicated directory (somewhere under `lib/clang`) so that other
> libraries unrelated to Clang are not accidentally elevated in the
> library search path.  When possible, the patch makes this change for
> both the build tree and install tree so users can work out of either
> tree.

The drawback is that this directory is added before LIBRARY_PATH, see
below.

> This solution seems straight-forward, but it raises many questions.
>
> Q1: What is the right Clang-dedicated directory?
> ------------------------------------------------
>
> There are many possibilities.  On my system:
>
> * `lib`: This directory is obviously not Clang-dedicated, but it's
> where libcxx, libunwind, and openmp currently place their libraries by
> default.
>
> * `lib/clang/9.0.0/lib/linux`: This directory is where compiler-rt
> libraries are placed by default, and their names are like
> `libclang_rt.asan-x86_64.so`.
>
> * `lib/clang/9.0.0/x86_64-unknown-linux-gnu/lib`: When the cmake
> option `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` is set, this
> directory is where compiler-rt, libcxx, and libunwind libraries are
> placed, and the compiler-rt libraries then drop the redundant
> `-x86_64` from their names.
>
> * `lib/clang/9.0.0/lib/linux/x86_64`: This directory has the nice
> advantage that `-frtlib-add-rpath` tells Clang to add a `-rpath` for
> this directory so you don't have to set `LD_LIBRARY_PATH` when
> executing `a.out`.  However, it turns out that `-rpath` doesn't work
> correctly for me in the case of openmp offloading [3].  As far as I
> can tell, this directory is currently populated only by some Android
> tool chains [3].
>
> * For libcxx and libunwind libraries, it's been suggested that a
> Clang-dedicated directory should not be specific to the Clang version
> (shouldn't contain `9.0.0`) because the libraries themselves are not
> locked to the Clang version, and a possible directory layout has been
> proposed [4].  My understanding is that this approach generally makes
> sense when the shared libraries have version numbers, like
> `libomp.so.5`.  In that case, it needs to be possible to find older
> shared libraries that older software has already become dependent upon
> while using older Clang installations.  It's easier to find them, with
> `LD_LIBRARY_PATH` for example, if every Clang installation places such
> libraries in the same Clang-version-independent directory, where the
> libraries are still distinct due to the version numbers in their
> names, instead of placing them in a bunch of different
> Clang-version-specific directories.  Please correct me if I've
> misunderstood this situation.
AFAIK libc++ and libunwind are using the same ABI version .1 since the
beginning. That's probably kind of the same guarantee that libomp has:
You can run an old binary with a newer version of the library.
However, that defeats the point of being "distinct" when installing the
libraries in a single directory that is independent of the Clang
version.

> Which choice is right for openmp libraries?  Debian is giving version
> numbers to `libomp.so`, but is that right?  If it is, the last choice
> might make sense.  If not, perhaps one of the previous choices is
> better.
>
> Q2: Should `lib` also contain the libraries?
> --------------------------------------------
>
> As mentioned above, `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` moves
> libcxx, libunwind, and compiler-rt libraries to their new
> Clang-dedicated directory.  However, the patch I've proposed for
> openmp instead duplicates libraries into their new Clang-dedicated
> directory.  I figured duplicating would be better for backward
> compatibility than moving.  Is it necessary?
>
> (Actually, I should probably be creating symlinks instead of duplicates.)
>
> Q3: Is it really OK to elevate Clang's libraries?
> -------------------------------------------------
>
> For me, this is the biggest unknown about this change.  That is, does
> any user depend on being able to drop his own `libomp.so` in a system
> library directory so that Clang will prefer it over Clang's own
> `libomp.so`?  If so, is that an important enough use case that we
> should continue to risk subtly breaking the compiles of other users by
> default?
As I said in the reviews we are building and using custom versions of
libomp with a globally installed (release) version of Clang (like we
can do with the Intel Compiler). For my usage I'm currently relying on
a combination of LIBRARY_PATH and LD_LIBRARY_PATH which many people
think is fragile, but it has worked so far. I guess we could switch to
using `-L` (and LD_LIBRARY_PATH), but setting an environment variable
is easier than telling all kind of build systems to modify their
compiler invocations...

I'm not sure if that qualifies as "important enough", but I think
that's a valid use case that would break with the proposed changes.
Maybe we can have a directory that is added after LIBRARY_PATH but
before all "default" system directories?

> Q4: Should libcxx, libunwind, or compiler-rt be changed too?
> ------------------------------------------------------------
>
> An early reaction to my patch was that we need to either handle the
> various Clang libraries consistently or show why they should be
> handled differently.  My patch so far only changes openmp, and it does
> not make it consistent with other subprojects' libraries.
>
> Keep in mind that, while compiler-rt libraries are already placed in a
> Clang-dedicated directory by default, libcxx and libunwind libraries
> are not and so could potentially be affected by the motivating example
> I gave.  Must users specify
> `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` (which adds the problem of
> Clang-version locking) to avoid that problem?  Shouldn't the default
> configuration be the most user-friendly?
>
> Conclusion
> ----------
>
> So far, my estimate at the best path forward is as follows:
>
> * openmp and compiler-rt libraries should always be placed in a
> Clang-dedicated, Clang-version-specific directory.
Based on the reasons given above I don't understand why openmp needs to
be version-locked with Clang. I think the guarantee is that you can run
old binaries with a newer version of the library, just as you can with
libc++ and friends. So why should openmp be treated differently?

Regards,
Jonas

> * libcxx and libunwind libraries should always be placed in a
> Clang-dedicated, Clang-version-independent directory.
>
> * For each of those directories, Clang should specify `-L` earlier
> than any system library directory.
>
> * openmp, libcxx, and libunwind libraries should also be symlinked
> directly in `lib`, for backward compatibility at least.
>
> * `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` should be dropped and
> dependent projects updated.  Alternatively, for backward
> compatibility, this option could revert the Clang-dedicated directory
> layout to the layout it currently produces (in particular, it places
> libcxx and libunwind libraries in Clang-version-specific directories).
>
> There are details like exact directory names to discuss, but does this
> high-level plan make sense?
>
> Thanks.
>
> Joel
>
> [1]:
https://reviews.llvm.org/D55725#1370823>
> [2]:
https://reviews.llvm.org/D55725>
> [3]:
https://reviews.llvm.org/D55725#1371807>
> [4]:
https://reviews.llvm.org/D30733#697781>

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

signature.asc (499 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Place libs in Clang-dedicated directories (affects openmp, libcxx, libunwind, compiler-rt)

Bakhvalov, Denis via cfe-dev
In reply to this post by Bakhvalov, Denis via cfe-dev
On Thu, Feb 14, 2019 at 2:29 PM Joel E. Denny via cfe-dev
<[hidden email]> wrote:

>
> Hi,
>
> This RFC discusses placing Clang's libraries in Clang-dedicated
> directories (somewhere within `lib/clang` instead of `lib`) in both
> build trees and install trees.  While fixing broken linking with
> openmp libraries was the original motivation, subprojects potentially
> affected by that problem or this change also include libcxx,
> libunwind, and compiler-rt.
>
> Motivation: Broken Default Linking
> ----------------------------------
>
> Clang by default prefers system openmp libraries over the ones built
> alongside Clang. The effect is that Clang-compiled openmp applications
> sometimes misbehave if the user doesn't adjust the library search
> path.
>
> For example, where `/home/jdenny/llvm` is built and installed from git
> master, but where my system openmp libraries under
> `/usr/lib/x86_64-linux-gnu` are from LLVM 6.0:
>
> ```
> $ cd /home/jdenny/llvm
> $ cat test.c
> #include <stdio.h>
> int main() {
>   #pragma omp target teams
>   printf("hello\n");
>   return 0;
> }
> $ ./bin/clang -fopenmp -fopenmp-targets=nvptx64 test.c
> $ ./a.out
> ./a.out: error while loading shared libraries: libomptarget.so: cannot
> open shared object file: No such file or directory
> $ LD_LIBRARY_PATH=lib ./a.out
> Segmentation fault (core dumped)
> $ LD_LIBRARY_PATH=lib ldd a.out | grep libomp
>         libomp.so.5 => /usr/lib/x86_64-linux-gnu/libomp.so.5
> (0x00007fab59748000)
>         libomptarget.so => lib/libomptarget.so (0x00007fab59515000)
> ```
>
> The problem here is a little subtle.  `-v` reveals that Clang
> specifies `-L/usr/lib/x86_64-linux-gnu` before
> `-L/home/jdenny/llvm/lib` when linking, and I have:
>
> ```
> $ ls -l /usr/lib/x86_64-linux-gnu/libomp.so
> lrwxrwxrwx 1 root root 11 Jan  7  2018
> /usr/lib/x86_64-linux-gnu/libomp.so -> libomp.so.5
> ```
>
> As a result, Clang links the executable as follows:
>
> ```
> $ readelf -d a.out | grep libomp.so
>  0x0000000000000001 (NEEDED)             Shared library: [libomp.so.5]
> ```
>
> Now `LD_LIBRARY_PATH` cannot force `a.out` to find Clang's `libomp.so`
> because `a.out` wants `libomp.so.5` instead.
>
> As far as I know, there's nothing unusual about my system, which is a
> typical Ubuntu 18.04.2 installation.  However, I have learned from
> others that `libomp.so.5` is installed by Debian's packages not by
> LLVM's cmake files [1], so many people might not see this particular
> problem.
>
> Even if `/usr/lib/x86_64-linux-gnu/libomp.so` weren't a symbolic link,
> it's still the wrong library, and incompatibilities could have caused
> a link failure instead of a run-time failure.
>
> Either way, it seems surprising and confusing that Clang doesn't
> always prefer its own libraries.
>
> Solution: Early `-L` for Clang-Dedicated Directory
> --------------------------------------------------
>
> The easiest solution is probably to assume users will manually specify
> `-L` when necessary to link against the right `libomp.so`.  However,
> it seems like Clang should just do that automatically to help
> unsuspecting users avoid surprises like the one above.
>
> I wrote a patch to make that happen [2].  The patch's basic approach
> is to place `libomp .so` in a directory for which, when linking, Clang
> automatically specifies a `-L` earlier than any `-L` for a system
> library directory.  It's important that the directory is a
> Clang-dedicated directory (somewhere under `lib/clang`) so that other
> libraries unrelated to Clang are not accidentally elevated in the
> library search path.  When possible, the patch makes this change for
> both the build tree and install tree so users can work out of either
> tree.
>
> This solution seems straight-forward, but it raises many questions.
>
> Q1: What is the right Clang-dedicated directory?
> ------------------------------------------------
>
> There are many possibilities.  On my system:
>
> * `lib`: This directory is obviously not Clang-dedicated, but it's
> where libcxx, libunwind, and openmp currently place their libraries by
> default.
>
> * `lib/clang/9.0.0/lib/linux`: This directory is where compiler-rt
> libraries are placed by default, and their names are like
> `libclang_rt.asan-x86_64.so`.
>
> * `lib/clang/9.0.0/x86_64-unknown-linux-gnu/lib`: When the cmake
> option `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` is set, this
> directory is where compiler-rt, libcxx, and libunwind libraries are
> placed, and the compiler-rt libraries then drop the redundant
> `-x86_64` from their names.

That's not the motivation behind LLVM_ENABLE_PER_TARGET_RUNTIME_DIR,
it's just a side-effect of that change. The motivation behind this
change is to support distributing and using runtimes for different
targets in a single Clang toolchain.

> * `lib/clang/9.0.0/lib/linux/x86_64`: This directory has the nice
> advantage that `-frtlib-add-rpath` tells Clang to add a `-rpath` for
> this directory so you don't have to set `LD_LIBRARY_PATH` when
> executing `a.out`.  However, it turns out that `-rpath` doesn't work
> correctly for me in the case of openmp offloading [3].  As far as I
> can tell, this directory is currently populated only by some Android
> tool chains [3].

This layout has one major downside, which is the fact that
architecture and operating system is insufficient. For example, say I
want to distribute runtimes built for x86_64-linux-gnu and
x86_64-linux-musl, where would these go? You cannot place them both in
lib/clang/9.0.0/lib/linux/x86_64 because they're incompatible. You
could consider adding another component to differentiate environment,
but what about targets that don't use it? Target triples already solve
all that which is why LLVM_ENABLE_PER_TARGET_RUNTIME_DIR uses those.

I talked to pirama who originally introduced this layout about
migrating to the LLVM_ENABLE_PER_TARGET_RUNTIME_DIR layout and he was
fine with it, I just haven't had time to do it (it's a bit more
involved than just making LLVM side changes because we would also need
to make sure that nothing is using it on the Android side). However, I
think it would be the right thing to do to reduce the number of
layouts we support.

> * For libcxx and libunwind libraries, it's been suggested that a
> Clang-dedicated directory should not be specific to the Clang version
> (shouldn't contain `9.0.0`) because the libraries themselves are not
> locked to the Clang version, and a possible directory layout has been
> proposed [4].  My understanding is that this approach generally makes
> sense when the shared libraries have version numbers, like
> `libomp.so.5`.  In that case, it needs to be possible to find older
> shared libraries that older software has already become dependent upon
> while using older Clang installations.  It's easier to find them, with
> `LD_LIBRARY_PATH` for example, if every Clang installation places such
> libraries in the same Clang-version-independent directory, where the
> libraries are still distinct due to the version numbers in their
> names, instead of placing them in a bunch of different
> Clang-version-specific directories.  Please correct me if I've
> misunderstood this situation.
>
> Which choice is right for openmp libraries?  Debian is giving version
> numbers to `libomp.so`, but is that right?  If it is, the last choice
> might make sense.  If not, perhaps one of the previous choices is
> better.
>
> Q2: Should `lib` also contain the libraries?
> --------------------------------------------
>
> As mentioned above, `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` moves
> libcxx, libunwind, and compiler-rt libraries to their new
> Clang-dedicated directory.  However, the patch I've proposed for
> openmp instead duplicates libraries into their new Clang-dedicated
> directory.  I figured duplicating would be better for backward
> compatibility than moving.  Is it necessary?
>
> (Actually, I should probably be creating symlinks instead of duplicates.)
>
> Q3: Is it really OK to elevate Clang's libraries?
> -------------------------------------------------
>
> For me, this is the biggest unknown about this change.  That is, does
> any user depend on being able to drop his own `libomp.so` in a system
> library directory so that Clang will prefer it over Clang's own
> `libomp.so`?  If so, is that an important enough use case that we
> should continue to risk subtly breaking the compiles of other users by
> default?
>
> Q4: Should libcxx, libunwind, or compiler-rt be changed too?
> ------------------------------------------------------------
>
> An early reaction to my patch was that we need to either handle the
> various Clang libraries consistently or show why they should be
> handled differently.  My patch so far only changes openmp, and it does
> not make it consistent with other subprojects' libraries.
>
> Keep in mind that, while compiler-rt libraries are already placed in a
> Clang-dedicated directory by default, libcxx and libunwind libraries
> are not and so could potentially be affected by the motivating example
> I gave.  Must users specify
> `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` (which adds the problem of
> Clang-version locking) to avoid that problem?  Shouldn't the default
> configuration be the most user-friendly?
>
> Conclusion
> ----------
>
> So far, my estimate at the best path forward is as follows:
>
> * openmp and compiler-rt libraries should always be placed in a
> Clang-dedicated, Clang-version-specific directory.
>
> * libcxx and libunwind libraries should always be placed in a
> Clang-dedicated, Clang-version-independent directory.
>
> * For each of those directories, Clang should specify `-L` earlier
> than any system library directory.
>
> * openmp, libcxx, and libunwind libraries should also be symlinked
> directly in `lib`, for backward compatibility at least.
>
> * `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` should be dropped and
> dependent projects updated.  Alternatively, for backward
> compatibility, this option could revert the Clang-dedicated directory
> layout to the layout it currently produces (in particular, it places
> libcxx and libunwind libraries in Clang-version-specific directories).

What's your proposed alternative? Having support for per-target
runtime directories has been proposed several times in the past. First
proposal I found was from chandlerc in 2011
(http://lists.llvm.org/pipermail/llvm-dev/2011-November/045565.html),
another one from mgorny
(http://lists.llvm.org/pipermail/cfe-dev/2017-January/052512.html) and
the last one from myself
(http://lists.llvm.org/pipermail/cfe-dev/2017-December/056442.html). I
implemented my proposal (which is basically the same as chandlerc's)
which is LLVM_ENABLE_PER_TARGET_RUNTIME_DIR. This is currently
supported for all the platforms except for Darwin which is
more-complicated due to use of fat binaries, but beanz has been
working on that.

We currently distribute our Clang toolchain for multiple different
host platforms (x86_64-linux-gnu, aarch64-linux-gnu,
x86_64-apple-darwin) and support multiple different target platforms
(i386-linux-gnu, armhf-linux-gnueabi, x86_64-linux-gnu,
aarch64-linux-gnu, x86_64-fuchsia, aarch-fuchsia). This what the
resource directory currently looks like:

$prefix/lib/clang/9.0.0/include
$prefix/lib/clang/9.0.0/i386-linux-gnu
$prefix/lib/clang/9.0.0/aarch64-fuchsia
$prefix/lib/clang/9.0.0/aarch64-linux-gnu
$prefix/lib/clang/9.0.0/armhf-linux-gnueabi
$prefix/lib/clang/9.0.0/x86_64-fuchsia
$prefix/lib/clang/9.0.0/x86_64-linux-gnu

I'm also trying to build runtimes for Windows. This toolchain is being
used by several projects (Fuchsia, Flutter, Dart and probably some
others I'm not aware of). The idea is that you shouldn't need one
toolchain per target, a single toolchain should be able to target
multiple different targets, but for that you need runtimes for each
target platform (plus sysroot).

Alternative that has been suggested in the past is to ship runtimes
inside sysroot and not with Clang. That would work but IMHO it's
inconvenient. It's true that runtimes (e.g. libc++) aren't in theory
version locked to Clang, but there have been and will be cases when a
new flag is implemented in Clang and immediately used in libc++. Very
often these changes address various issues that users run into. We
want to consume those improvements as soon as they're available.
That's why we're building all the runtimes with the toolchain and with
LLVM_ENABLE_PER_TARGET_RUNTIME_DIR we can do so with a single
CMake+Ninja invocation. If we were to ship these runtimes with the
sysroot, it would mean re-building and packaging those runtimes
separately which would require significantly more effort.

If you want to drop LLVM_ENABLE_PER_TARGET_RUNTIME_DIR, then you
really need to provide some alternative that's going to support the
same use cases and ideally would be as convenient to use.

> There are details like exact directory names to discuss, but does this
> high-level plan make sense?
>
> Thanks.
>
> Joel
>
> [1]: https://reviews.llvm.org/D55725#1370823
> [2]: https://reviews.llvm.org/D55725
> [3]: https://reviews.llvm.org/D55725#1371807
> [4]: https://reviews.llvm.org/D30733#697781
> _______________________________________________
> cfe-dev mailing list
> [hidden email]
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Place libs in Clang-dedicated directories (affects openmp, libcxx, libunwind, compiler-rt)

Bakhvalov, Denis via cfe-dev
In reply to this post by Bakhvalov, Denis via cfe-dev
Hi Jonas,

On Sun, Feb 17, 2019 at 4:05 AM Jonas Hahnfeld <[hidden email]> wrote:

>
> Hi Joel,
>
> Am Donnerstag, den 14.02.2019, 17:29 -0500 schrieb Joel E. Denny:
> > Hi,
> >
> > This RFC discusses placing Clang's libraries in Clang-dedicated
> > directories (somewhere within `lib/clang` instead of `lib`) in both
> > build trees and install trees.  While fixing broken linking with
> > openmp libraries was the original motivation, subprojects potentially
> > affected by that problem or this change also include libcxx,
> > libunwind, and compiler-rt.
> >
> > Motivation: Broken Default Linking
> > ----------------------------------
> >
> > Clang by default prefers system openmp libraries over the ones built
> > alongside Clang. The effect is that Clang-compiled openmp applications
> > sometimes misbehave if the user doesn't adjust the library search
> > path.
> >
> > For example, where `/home/jdenny/llvm` is built and installed from git
> > master, but where my system openmp libraries under
> > `/usr/lib/x86_64-linux-gnu` are from LLVM 6.0:
> >
> > ```
> > $ cd /home/jdenny/llvm
> > $ cat test.c
> > #include <stdio.h>
> > int main() {
> >   #pragma omp target teams
> >   printf("hello\n");
> >   return 0;
> > }
> > $ ./bin/clang -fopenmp -fopenmp-targets=nvptx64 test.c
> > $ ./a.out
> > ./a.out: error while loading shared libraries: libomptarget.so: cannot
> > open shared object file: No such file or directory
> > $ LD_LIBRARY_PATH=lib ./a.out
> > Segmentation fault (core dumped)
> > $ LD_LIBRARY_PATH=lib ldd a.out | grep libomp
> >         libomp.so.5 => /usr/lib/x86_64-linux-gnu/libomp.so.5
> > (0x00007fab59748000)
> >         libomptarget.so => lib/libomptarget.so (0x00007fab59515000)
> > ```
> >
> > The problem here is a little subtle.  `-v` reveals that Clang
> > specifies `-L/usr/lib/x86_64-linux-gnu` before
> > `-L/home/jdenny/llvm/lib` when linking, and I have:
> >
> > ```
> > $ ls -l /usr/lib/x86_64-linux-gnu/libomp.so
> > lrwxrwxrwx 1 root root 11 Jan  7  2018
> > /usr/lib/x86_64-linux-gnu/libomp.so -> libomp.so.5
> > ```
> >
> > As a result, Clang links the executable as follows:
> >
> > ```
> > $ readelf -d a.out | grep libomp.so
> >  0x0000000000000001 (NEEDED)             Shared library: [libomp.so.5]
> > ```
> >
> > Now `LD_LIBRARY_PATH` cannot force `a.out` to find Clang's `libomp.so`
> > because `a.out` wants `libomp.so.5` instead.
> >
> > As far as I know, there's nothing unusual about my system, which is a
> > typical Ubuntu 18.04.2 installation.  However, I have learned from
> > others that `libomp.so.5` is installed by Debian's packages not by
> > LLVM's cmake files [1], so many people might not see this particular
> > problem.
> >
> > Even if `/usr/lib/x86_64-linux-gnu/libomp.so` weren't a symbolic link,
> > it's still the wrong library, and incompatibilities could have caused
> > a link failure instead of a run-time failure.
> >
> > Either way, it seems surprising and confusing that Clang doesn't
> > always prefer its own libraries.
> >
> > Solution: Early `-L` for Clang-Dedicated Directory
> > --------------------------------------------------
> >
> > The easiest solution is probably to assume users will manually specify
> > `-L` when necessary to link against the right `libomp.so`.  However,
> > it seems like Clang should just do that automatically to help
> > unsuspecting users avoid surprises like the one above.
>
> +1 that Clang should get this right in most cases.
>
> > I wrote a patch to make that happen [2].  The patch's basic approach
> > is to place `libomp .so` in a directory for which, when linking, Clang
> > automatically specifies a `-L` earlier than any `-L` for a system
> > library directory.  It's important that the directory is a
> > Clang-dedicated directory (somewhere under `lib/clang`) so that other
> > libraries unrelated to Clang are not accidentally elevated in the
> > library search path.  When possible, the patch makes this change for
> > both the build tree and install tree so users can work out of either
> > tree.
>
> The drawback is that this directory is added before LIBRARY_PATH, see
> below.
>
> > This solution seems straight-forward, but it raises many questions.
> >
> > Q1: What is the right Clang-dedicated directory?
> > ------------------------------------------------
> >
> > There are many possibilities.  On my system:
> >
> > * `lib`: This directory is obviously not Clang-dedicated, but it's
> > where libcxx, libunwind, and openmp currently place their libraries by
> > default.
> >
> > * `lib/clang/9.0.0/lib/linux`: This directory is where compiler-rt
> > libraries are placed by default, and their names are like
> > `libclang_rt.asan-x86_64.so`.
> >
> > * `lib/clang/9.0.0/x86_64-unknown-linux-gnu/lib`: When the cmake
> > option `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` is set, this
> > directory is where compiler-rt, libcxx, and libunwind libraries are
> > placed, and the compiler-rt libraries then drop the redundant
> > `-x86_64` from their names.
> >
> > * `lib/clang/9.0.0/lib/linux/x86_64`: This directory has the nice
> > advantage that `-frtlib-add-rpath` tells Clang to add a `-rpath` for
> > this directory so you don't have to set `LD_LIBRARY_PATH` when
> > executing `a.out`.  However, it turns out that `-rpath` doesn't work
> > correctly for me in the case of openmp offloading [3].  As far as I
> > can tell, this directory is currently populated only by some Android
> > tool chains [3].
> >
> > * For libcxx and libunwind libraries, it's been suggested that a
> > Clang-dedicated directory should not be specific to the Clang version
> > (shouldn't contain `9.0.0`) because the libraries themselves are not
> > locked to the Clang version, and a possible directory layout has been
> > proposed [4].  My understanding is that this approach generally makes
> > sense when the shared libraries have version numbers, like
> > `libomp.so.5`.  In that case, it needs to be possible to find older
> > shared libraries that older software has already become dependent upon
> > while using older Clang installations.  It's easier to find them, with
> > `LD_LIBRARY_PATH` for example, if every Clang installation places such
> > libraries in the same Clang-version-independent directory, where the
> > libraries are still distinct due to the version numbers in their
> > names, instead of placing them in a bunch of different
> > Clang-version-specific directories.  Please correct me if I've
> > misunderstood this situation.
>
> AFAIK libc++ and libunwind are using the same ABI version .1 since the
> beginning. That's probably kind of the same guarantee that libomp has:
> You can run an old binary with a newer version of the library.
> However, that defeats the point of being "distinct" when installing the
> libraries in a single directory that is independent of the Clang
> version.

I'm not close enough to these libraries to make a decision here.  It
sounds like libomp, libc++, and libunwind are not handled
consistently, and so far we don't have any solid justification for
that inconsistency.

>
> > Which choice is right for openmp libraries?  Debian is giving version
> > numbers to `libomp.so`, but is that right?  If it is, the last choice
> > might make sense.  If not, perhaps one of the previous choices is
> > better.
> >
> > Q2: Should `lib` also contain the libraries?
> > --------------------------------------------
> >
> > As mentioned above, `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` moves
> > libcxx, libunwind, and compiler-rt libraries to their new
> > Clang-dedicated directory.  However, the patch I've proposed for
> > openmp instead duplicates libraries into their new Clang-dedicated
> > directory.  I figured duplicating would be better for backward
> > compatibility than moving.  Is it necessary?
> >
> > (Actually, I should probably be creating symlinks instead of duplicates.)
> >
> > Q3: Is it really OK to elevate Clang's libraries?
> > -------------------------------------------------
> >
> > For me, this is the biggest unknown about this change.  That is, does
> > any user depend on being able to drop his own `libomp.so` in a system
> > library directory so that Clang will prefer it over Clang's own
> > `libomp.so`?  If so, is that an important enough use case that we
> > should continue to risk subtly breaking the compiles of other users by
> > default?
>
> As I said in the reviews we are building and using custom versions of
> libomp with a globally installed (release) version of Clang (like we
> can do with the Intel Compiler). For my usage I'm currently relying on
> a combination of LIBRARY_PATH and LD_LIBRARY_PATH which many people
> think is fragile, but it has worked so far. I guess we could switch to
> using `-L` (and LD_LIBRARY_PATH), but setting an environment variable
> is easier than telling all kind of build systems to modify their
> compiler invocations...
>
> I'm not sure if that qualifies as "important enough", but I think
> that's a valid use case that would break with the proposed changes.

I don't know if it's important enough either.  However, it suffers
from the problem I mentioned in the example in the motivation of the
RFC.  That is, system directories that cause the trouble in that
example have precedence over Clang's lib directory, which has
precedence over LIBRARY_PATH.  Thus, libomp.so in LIBRARY_PATH will
not be seen if such a system directory has a libomp.so.

> Maybe we can have a directory that is added after LIBRARY_PATH but
> before all "default" system directories?

That's not possible because the system directories are before LIBRARY_PATH.

Thanks.


Joel

>
> > Q4: Should libcxx, libunwind, or compiler-rt be changed too?
> > ------------------------------------------------------------
> >
> > An early reaction to my patch was that we need to either handle the
> > various Clang libraries consistently or show why they should be
> > handled differently.  My patch so far only changes openmp, and it does
> > not make it consistent with other subprojects' libraries.
> >
> > Keep in mind that, while compiler-rt libraries are already placed in a
> > Clang-dedicated directory by default, libcxx and libunwind libraries
> > are not and so could potentially be affected by the motivating example
> > I gave.  Must users specify
> > `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` (which adds the problem of
> > Clang-version locking) to avoid that problem?  Shouldn't the default
> > configuration be the most user-friendly?
> >
> > Conclusion
> > ----------
> >
> > So far, my estimate at the best path forward is as follows:
> >
> > * openmp and compiler-rt libraries should always be placed in a
> > Clang-dedicated, Clang-version-specific directory.
>
> Based on the reasons given above I don't understand why openmp needs to
> be version-locked with Clang. I think the guarantee is that you can run
> old binaries with a newer version of the library, just as you can with
> libc++ and friends. So why should openmp be treated differently?
>
> Regards,
> Jonas
>
> > * libcxx and libunwind libraries should always be placed in a
> > Clang-dedicated, Clang-version-independent directory.
> >
> > * For each of those directories, Clang should specify `-L` earlier
> > than any system library directory.
> >
> > * openmp, libcxx, and libunwind libraries should also be symlinked
> > directly in `lib`, for backward compatibility at least.
> >
> > * `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` should be dropped and
> > dependent projects updated.  Alternatively, for backward
> > compatibility, this option could revert the Clang-dedicated directory
> > layout to the layout it currently produces (in particular, it places
> > libcxx and libunwind libraries in Clang-version-specific directories).
> >
> > There are details like exact directory names to discuss, but does this
> > high-level plan make sense?
> >
> > Thanks.
> >
> > Joel
> >
> > [1]:
> https://reviews.llvm.org/D55725#1370823>
> > [2]:
> https://reviews.llvm.org/D55725>
> > [3]:
> https://reviews.llvm.org/D55725#1371807>
> > [4]:
> https://reviews.llvm.org/D30733#697781>
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Place libs in Clang-dedicated directories (affects openmp, libcxx, libunwind, compiler-rt)

Bakhvalov, Denis via cfe-dev
In reply to this post by Bakhvalov, Denis via cfe-dev
Hi Petr,

On Sun, Feb 17, 2019 at 1:39 PM Petr Hosek <[hidden email]> wrote:

>
> On Thu, Feb 14, 2019 at 2:29 PM Joel E. Denny via cfe-dev
> <[hidden email]> wrote:
> >
> > Hi,
> >
> > This RFC discusses placing Clang's libraries in Clang-dedicated
> > directories (somewhere within `lib/clang` instead of `lib`) in both
> > build trees and install trees.  While fixing broken linking with
> > openmp libraries was the original motivation, subprojects potentially
> > affected by that problem or this change also include libcxx,
> > libunwind, and compiler-rt.
> >
> > Motivation: Broken Default Linking
> > ----------------------------------
> >
> > Clang by default prefers system openmp libraries over the ones built
> > alongside Clang. The effect is that Clang-compiled openmp applications
> > sometimes misbehave if the user doesn't adjust the library search
> > path.
> >
> > For example, where `/home/jdenny/llvm` is built and installed from git
> > master, but where my system openmp libraries under
> > `/usr/lib/x86_64-linux-gnu` are from LLVM 6.0:
> >
> > ```
> > $ cd /home/jdenny/llvm
> > $ cat test.c
> > #include <stdio.h>
> > int main() {
> >   #pragma omp target teams
> >   printf("hello\n");
> >   return 0;
> > }
> > $ ./bin/clang -fopenmp -fopenmp-targets=nvptx64 test.c
> > $ ./a.out
> > ./a.out: error while loading shared libraries: libomptarget.so: cannot
> > open shared object file: No such file or directory
> > $ LD_LIBRARY_PATH=lib ./a.out
> > Segmentation fault (core dumped)
> > $ LD_LIBRARY_PATH=lib ldd a.out | grep libomp
> >         libomp.so.5 => /usr/lib/x86_64-linux-gnu/libomp.so.5
> > (0x00007fab59748000)
> >         libomptarget.so => lib/libomptarget.so (0x00007fab59515000)
> > ```
> >
> > The problem here is a little subtle.  `-v` reveals that Clang
> > specifies `-L/usr/lib/x86_64-linux-gnu` before
> > `-L/home/jdenny/llvm/lib` when linking, and I have:
> >
> > ```
> > $ ls -l /usr/lib/x86_64-linux-gnu/libomp.so
> > lrwxrwxrwx 1 root root 11 Jan  7  2018
> > /usr/lib/x86_64-linux-gnu/libomp.so -> libomp.so.5
> > ```
> >
> > As a result, Clang links the executable as follows:
> >
> > ```
> > $ readelf -d a.out | grep libomp.so
> >  0x0000000000000001 (NEEDED)             Shared library: [libomp.so.5]
> > ```
> >
> > Now `LD_LIBRARY_PATH` cannot force `a.out` to find Clang's `libomp.so`
> > because `a.out` wants `libomp.so.5` instead.
> >
> > As far as I know, there's nothing unusual about my system, which is a
> > typical Ubuntu 18.04.2 installation.  However, I have learned from
> > others that `libomp.so.5` is installed by Debian's packages not by
> > LLVM's cmake files [1], so many people might not see this particular
> > problem.
> >
> > Even if `/usr/lib/x86_64-linux-gnu/libomp.so` weren't a symbolic link,
> > it's still the wrong library, and incompatibilities could have caused
> > a link failure instead of a run-time failure.
> >
> > Either way, it seems surprising and confusing that Clang doesn't
> > always prefer its own libraries.
> >
> > Solution: Early `-L` for Clang-Dedicated Directory
> > --------------------------------------------------
> >
> > The easiest solution is probably to assume users will manually specify
> > `-L` when necessary to link against the right `libomp.so`.  However,
> > it seems like Clang should just do that automatically to help
> > unsuspecting users avoid surprises like the one above.
> >
> > I wrote a patch to make that happen [2].  The patch's basic approach
> > is to place `libomp .so` in a directory for which, when linking, Clang
> > automatically specifies a `-L` earlier than any `-L` for a system
> > library directory.  It's important that the directory is a
> > Clang-dedicated directory (somewhere under `lib/clang`) so that other
> > libraries unrelated to Clang are not accidentally elevated in the
> > library search path.  When possible, the patch makes this change for
> > both the build tree and install tree so users can work out of either
> > tree.
> >
> > This solution seems straight-forward, but it raises many questions.
> >
> > Q1: What is the right Clang-dedicated directory?
> > ------------------------------------------------
> >
> > There are many possibilities.  On my system:
> >
> > * `lib`: This directory is obviously not Clang-dedicated, but it's
> > where libcxx, libunwind, and openmp currently place their libraries by
> > default.
> >
> > * `lib/clang/9.0.0/lib/linux`: This directory is where compiler-rt
> > libraries are placed by default, and their names are like
> > `libclang_rt.asan-x86_64.so`.
> >
> > * `lib/clang/9.0.0/x86_64-unknown-linux-gnu/lib`: When the cmake
> > option `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` is set, this
> > directory is where compiler-rt, libcxx, and libunwind libraries are
> > placed, and the compiler-rt libraries then drop the redundant
> > `-x86_64` from their names.
>
> That's not the motivation behind LLVM_ENABLE_PER_TARGET_RUNTIME_DIR,
> it's just a side-effect of that change. The motivation behind this
> change is to support distributing and using runtimes for different
> targets in a single Clang toolchain.
>
> > * `lib/clang/9.0.0/lib/linux/x86_64`: This directory has the nice
> > advantage that `-frtlib-add-rpath` tells Clang to add a `-rpath` for
> > this directory so you don't have to set `LD_LIBRARY_PATH` when
> > executing `a.out`.  However, it turns out that `-rpath` doesn't work
> > correctly for me in the case of openmp offloading [3].  As far as I
> > can tell, this directory is currently populated only by some Android
> > tool chains [3].
>
> This layout has one major downside, which is the fact that
> architecture and operating system is insufficient. For example, say I
> want to distribute runtimes built for x86_64-linux-gnu and
> x86_64-linux-musl, where would these go? You cannot place them both in
> lib/clang/9.0.0/lib/linux/x86_64 because they're incompatible. You
> could consider adding another component to differentiate environment,
> but what about targets that don't use it? Target triples already solve
> all that which is why LLVM_ENABLE_PER_TARGET_RUNTIME_DIR uses those.
>
> I talked to pirama who originally introduced this layout about
> migrating to the LLVM_ENABLE_PER_TARGET_RUNTIME_DIR layout and he was
> fine with it, I just haven't had time to do it (it's a bit more
> involved than just making LLVM side changes because we would also need
> to make sure that nothing is using it on the Android side). However, I
> think it would be the right thing to do to reduce the number of
> layouts we support.

Thanks for explaining.  No arguments here.

>
> > * For libcxx and libunwind libraries, it's been suggested that a
> > Clang-dedicated directory should not be specific to the Clang version
> > (shouldn't contain `9.0.0`) because the libraries themselves are not
> > locked to the Clang version, and a possible directory layout has been
> > proposed [4].  My understanding is that this approach generally makes
> > sense when the shared libraries have version numbers, like
> > `libomp.so.5`.  In that case, it needs to be possible to find older
> > shared libraries that older software has already become dependent upon
> > while using older Clang installations.  It's easier to find them, with
> > `LD_LIBRARY_PATH` for example, if every Clang installation places such
> > libraries in the same Clang-version-independent directory, where the
> > libraries are still distinct due to the version numbers in their
> > names, instead of placing them in a bunch of different
> > Clang-version-specific directories.  Please correct me if I've
> > misunderstood this situation.
> >
> > Which choice is right for openmp libraries?  Debian is giving version
> > numbers to `libomp.so`, but is that right?  If it is, the last choice
> > might make sense.  If not, perhaps one of the previous choices is
> > better.
> >
> > Q2: Should `lib` also contain the libraries?
> > --------------------------------------------
> >
> > As mentioned above, `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` moves
> > libcxx, libunwind, and compiler-rt libraries to their new
> > Clang-dedicated directory.  However, the patch I've proposed for
> > openmp instead duplicates libraries into their new Clang-dedicated
> > directory.  I figured duplicating would be better for backward
> > compatibility than moving.  Is it necessary?
> >
> > (Actually, I should probably be creating symlinks instead of duplicates.)
> >
> > Q3: Is it really OK to elevate Clang's libraries?
> > -------------------------------------------------
> >
> > For me, this is the biggest unknown about this change.  That is, does
> > any user depend on being able to drop his own `libomp.so` in a system
> > library directory so that Clang will prefer it over Clang's own
> > `libomp.so`?  If so, is that an important enough use case that we
> > should continue to risk subtly breaking the compiles of other users by
> > default?
> >
> > Q4: Should libcxx, libunwind, or compiler-rt be changed too?
> > ------------------------------------------------------------
> >
> > An early reaction to my patch was that we need to either handle the
> > various Clang libraries consistently or show why they should be
> > handled differently.  My patch so far only changes openmp, and it does
> > not make it consistent with other subprojects' libraries.
> >
> > Keep in mind that, while compiler-rt libraries are already placed in a
> > Clang-dedicated directory by default, libcxx and libunwind libraries
> > are not and so could potentially be affected by the motivating example
> > I gave.  Must users specify
> > `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` (which adds the problem of
> > Clang-version locking) to avoid that problem?  Shouldn't the default
> > configuration be the most user-friendly?
> >
> > Conclusion
> > ----------
> >
> > So far, my estimate at the best path forward is as follows:
> >
> > * openmp and compiler-rt libraries should always be placed in a
> > Clang-dedicated, Clang-version-specific directory.
> >
> > * libcxx and libunwind libraries should always be placed in a
> > Clang-dedicated, Clang-version-independent directory.
> >
> > * For each of those directories, Clang should specify `-L` earlier
> > than any system library directory.
> >
> > * openmp, libcxx, and libunwind libraries should also be symlinked
> > directly in `lib`, for backward compatibility at least.
> >
> > * `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` should be dropped and
> > dependent projects updated.  Alternatively, for backward
> > compatibility, this option could revert the Clang-dedicated directory
> > layout to the layout it currently produces (in particular, it places
> > libcxx and libunwind libraries in Clang-version-specific directories).
>
> What's your proposed alternative?

My alternative to LLVM_ENABLE_PER_TARGET_RUNTIME_DIR is the preceding
bullets.  In other words, you wouldn't need to specify
LLVM_ENABLE_PER_TARGET_RUNTIME_DIR because it would effectively be
always on (except the directories might be different than now if the
version locking issue is important, as noted above).  Is that what
you're asking?

> Having support for per-target
> runtime directories has been proposed several times in the past. First
> proposal I found was from chandlerc in 2011
> (http://lists.llvm.org/pipermail/llvm-dev/2011-November/045565.html),

I hadn't seen that one.  This has been discussed for quite a while.

Thanks.


Joel

> another one from mgorny
> (http://lists.llvm.org/pipermail/cfe-dev/2017-January/052512.html) and
> the last one from myself
> (http://lists.llvm.org/pipermail/cfe-dev/2017-December/056442.html). I
> implemented my proposal (which is basically the same as chandlerc's)
> which is LLVM_ENABLE_PER_TARGET_RUNTIME_DIR. This is currently
> supported for all the platforms except for Darwin which is
> more-complicated due to use of fat binaries, but beanz has been
> working on that.
>
> We currently distribute our Clang toolchain for multiple different
> host platforms (x86_64-linux-gnu, aarch64-linux-gnu,
> x86_64-apple-darwin) and support multiple different target platforms
> (i386-linux-gnu, armhf-linux-gnueabi, x86_64-linux-gnu,
> aarch64-linux-gnu, x86_64-fuchsia, aarch-fuchsia). This what the
> resource directory currently looks like:
>
> $prefix/lib/clang/9.0.0/include
> $prefix/lib/clang/9.0.0/i386-linux-gnu
> $prefix/lib/clang/9.0.0/aarch64-fuchsia
> $prefix/lib/clang/9.0.0/aarch64-linux-gnu
> $prefix/lib/clang/9.0.0/armhf-linux-gnueabi
> $prefix/lib/clang/9.0.0/x86_64-fuchsia
> $prefix/lib/clang/9.0.0/x86_64-linux-gnu
>
> I'm also trying to build runtimes for Windows. This toolchain is being
> used by several projects (Fuchsia, Flutter, Dart and probably some
> others I'm not aware of). The idea is that you shouldn't need one
> toolchain per target, a single toolchain should be able to target
> multiple different targets, but for that you need runtimes for each
> target platform (plus sysroot).
>
> Alternative that has been suggested in the past is to ship runtimes
> inside sysroot and not with Clang. That would work but IMHO it's
> inconvenient. It's true that runtimes (e.g. libc++) aren't in theory
> version locked to Clang, but there have been and will be cases when a
> new flag is implemented in Clang and immediately used in libc++. Very
> often these changes address various issues that users run into. We
> want to consume those improvements as soon as they're available.
> That's why we're building all the runtimes with the toolchain and with
> LLVM_ENABLE_PER_TARGET_RUNTIME_DIR we can do so with a single
> CMake+Ninja invocation. If we were to ship these runtimes with the
> sysroot, it would mean re-building and packaging those runtimes
> separately which would require significantly more effort.
>
> If you want to drop LLVM_ENABLE_PER_TARGET_RUNTIME_DIR, then you
> really need to provide some alternative that's going to support the
> same use cases and ideally would be as convenient to use.
>
> > There are details like exact directory names to discuss, but does this
> > high-level plan make sense?
> >
> > Thanks.
> >
> > Joel
> >
> > [1]: https://reviews.llvm.org/D55725#1370823
> > [2]: https://reviews.llvm.org/D55725
> > [3]: https://reviews.llvm.org/D55725#1371807
> > [4]: https://reviews.llvm.org/D30733#697781
> > _______________________________________________
> > cfe-dev mailing list
> > [hidden email]
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Place libs in Clang-dedicated directories (affects openmp, libcxx, libunwind, compiler-rt)

Bakhvalov, Denis via cfe-dev
In reply to this post by Bakhvalov, Denis via cfe-dev
On Wed, Feb 20, 2019 at 6:14 PM Joel E. Denny <[hidden email]> wrote:

>
> Hi Jonas,
>
> On Sun, Feb 17, 2019 at 4:05 AM Jonas Hahnfeld <[hidden email]> wrote:
> >
> > Hi Joel,
> >
> > Am Donnerstag, den 14.02.2019, 17:29 -0500 schrieb Joel E. Denny:
> > > Hi,
> > >
> > > This RFC discusses placing Clang's libraries in Clang-dedicated
> > > directories (somewhere within `lib/clang` instead of `lib`) in both
> > > build trees and install trees.  While fixing broken linking with
> > > openmp libraries was the original motivation, subprojects potentially
> > > affected by that problem or this change also include libcxx,
> > > libunwind, and compiler-rt.
> > >
> > > Motivation: Broken Default Linking
> > > ----------------------------------
> > >
> > > Clang by default prefers system openmp libraries over the ones built
> > > alongside Clang. The effect is that Clang-compiled openmp applications
> > > sometimes misbehave if the user doesn't adjust the library search
> > > path.
> > >
> > > For example, where `/home/jdenny/llvm` is built and installed from git
> > > master, but where my system openmp libraries under
> > > `/usr/lib/x86_64-linux-gnu` are from LLVM 6.0:
> > >
> > > ```
> > > $ cd /home/jdenny/llvm
> > > $ cat test.c
> > > #include <stdio.h>
> > > int main() {
> > >   #pragma omp target teams
> > >   printf("hello\n");
> > >   return 0;
> > > }
> > > $ ./bin/clang -fopenmp -fopenmp-targets=nvptx64 test.c
> > > $ ./a.out
> > > ./a.out: error while loading shared libraries: libomptarget.so: cannot
> > > open shared object file: No such file or directory
> > > $ LD_LIBRARY_PATH=lib ./a.out
> > > Segmentation fault (core dumped)
> > > $ LD_LIBRARY_PATH=lib ldd a.out | grep libomp
> > >         libomp.so.5 => /usr/lib/x86_64-linux-gnu/libomp.so.5
> > > (0x00007fab59748000)
> > >         libomptarget.so => lib/libomptarget.so (0x00007fab59515000)
> > > ```
> > >
> > > The problem here is a little subtle.  `-v` reveals that Clang
> > > specifies `-L/usr/lib/x86_64-linux-gnu` before
> > > `-L/home/jdenny/llvm/lib` when linking, and I have:
> > >
> > > ```
> > > $ ls -l /usr/lib/x86_64-linux-gnu/libomp.so
> > > lrwxrwxrwx 1 root root 11 Jan  7  2018
> > > /usr/lib/x86_64-linux-gnu/libomp.so -> libomp.so.5
> > > ```
> > >
> > > As a result, Clang links the executable as follows:
> > >
> > > ```
> > > $ readelf -d a.out | grep libomp.so
> > >  0x0000000000000001 (NEEDED)             Shared library: [libomp.so.5]
> > > ```
> > >
> > > Now `LD_LIBRARY_PATH` cannot force `a.out` to find Clang's `libomp.so`
> > > because `a.out` wants `libomp.so.5` instead.
> > >
> > > As far as I know, there's nothing unusual about my system, which is a
> > > typical Ubuntu 18.04.2 installation.  However, I have learned from
> > > others that `libomp.so.5` is installed by Debian's packages not by
> > > LLVM's cmake files [1], so many people might not see this particular
> > > problem.
> > >
> > > Even if `/usr/lib/x86_64-linux-gnu/libomp.so` weren't a symbolic link,
> > > it's still the wrong library, and incompatibilities could have caused
> > > a link failure instead of a run-time failure.
> > >
> > > Either way, it seems surprising and confusing that Clang doesn't
> > > always prefer its own libraries.
> > >
> > > Solution: Early `-L` for Clang-Dedicated Directory
> > > --------------------------------------------------
> > >
> > > The easiest solution is probably to assume users will manually specify
> > > `-L` when necessary to link against the right `libomp.so`.  However,
> > > it seems like Clang should just do that automatically to help
> > > unsuspecting users avoid surprises like the one above.
> >
> > +1 that Clang should get this right in most cases.
> >
> > > I wrote a patch to make that happen [2].  The patch's basic approach
> > > is to place `libomp .so` in a directory for which, when linking, Clang
> > > automatically specifies a `-L` earlier than any `-L` for a system
> > > library directory.  It's important that the directory is a
> > > Clang-dedicated directory (somewhere under `lib/clang`) so that other
> > > libraries unrelated to Clang are not accidentally elevated in the
> > > library search path.  When possible, the patch makes this change for
> > > both the build tree and install tree so users can work out of either
> > > tree.
> >
> > The drawback is that this directory is added before LIBRARY_PATH, see
> > below.
> >
> > > This solution seems straight-forward, but it raises many questions.
> > >
> > > Q1: What is the right Clang-dedicated directory?
> > > ------------------------------------------------
> > >
> > > There are many possibilities.  On my system:
> > >
> > > * `lib`: This directory is obviously not Clang-dedicated, but it's
> > > where libcxx, libunwind, and openmp currently place their libraries by
> > > default.
> > >
> > > * `lib/clang/9.0.0/lib/linux`: This directory is where compiler-rt
> > > libraries are placed by default, and their names are like
> > > `libclang_rt.asan-x86_64.so`.
> > >
> > > * `lib/clang/9.0.0/x86_64-unknown-linux-gnu/lib`: When the cmake
> > > option `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` is set, this
> > > directory is where compiler-rt, libcxx, and libunwind libraries are
> > > placed, and the compiler-rt libraries then drop the redundant
> > > `-x86_64` from their names.
> > >
> > > * `lib/clang/9.0.0/lib/linux/x86_64`: This directory has the nice
> > > advantage that `-frtlib-add-rpath` tells Clang to add a `-rpath` for
> > > this directory so you don't have to set `LD_LIBRARY_PATH` when
> > > executing `a.out`.  However, it turns out that `-rpath` doesn't work
> > > correctly for me in the case of openmp offloading [3].  As far as I
> > > can tell, this directory is currently populated only by some Android
> > > tool chains [3].
> > >
> > > * For libcxx and libunwind libraries, it's been suggested that a
> > > Clang-dedicated directory should not be specific to the Clang version
> > > (shouldn't contain `9.0.0`) because the libraries themselves are not
> > > locked to the Clang version, and a possible directory layout has been
> > > proposed [4].  My understanding is that this approach generally makes
> > > sense when the shared libraries have version numbers, like
> > > `libomp.so.5`.  In that case, it needs to be possible to find older
> > > shared libraries that older software has already become dependent upon
> > > while using older Clang installations.  It's easier to find them, with
> > > `LD_LIBRARY_PATH` for example, if every Clang installation places such
> > > libraries in the same Clang-version-independent directory, where the
> > > libraries are still distinct due to the version numbers in their
> > > names, instead of placing them in a bunch of different
> > > Clang-version-specific directories.  Please correct me if I've
> > > misunderstood this situation.
> >
> > AFAIK libc++ and libunwind are using the same ABI version .1 since the
> > beginning. That's probably kind of the same guarantee that libomp has:
> > You can run an old binary with a newer version of the library.
> > However, that defeats the point of being "distinct" when installing the
> > libraries in a single directory that is independent of the Clang
> > version.
>
> I'm not close enough to these libraries to make a decision here.  It
> sounds like libomp, libc++, and libunwind are not handled
> consistently, and so far we don't have any solid justification for
> that inconsistency.
>
> >
> > > Which choice is right for openmp libraries?  Debian is giving version
> > > numbers to `libomp.so`, but is that right?  If it is, the last choice
> > > might make sense.  If not, perhaps one of the previous choices is
> > > better.
> > >
> > > Q2: Should `lib` also contain the libraries?
> > > --------------------------------------------
> > >
> > > As mentioned above, `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` moves
> > > libcxx, libunwind, and compiler-rt libraries to their new
> > > Clang-dedicated directory.  However, the patch I've proposed for
> > > openmp instead duplicates libraries into their new Clang-dedicated
> > > directory.  I figured duplicating would be better for backward
> > > compatibility than moving.  Is it necessary?
> > >
> > > (Actually, I should probably be creating symlinks instead of duplicates.)
> > >
> > > Q3: Is it really OK to elevate Clang's libraries?
> > > -------------------------------------------------
> > >
> > > For me, this is the biggest unknown about this change.  That is, does
> > > any user depend on being able to drop his own `libomp.so` in a system
> > > library directory so that Clang will prefer it over Clang's own
> > > `libomp.so`?  If so, is that an important enough use case that we
> > > should continue to risk subtly breaking the compiles of other users by
> > > default?
> >
> > As I said in the reviews we are building and using custom versions of
> > libomp with a globally installed (release) version of Clang (like we
> > can do with the Intel Compiler). For my usage I'm currently relying on
> > a combination of LIBRARY_PATH and LD_LIBRARY_PATH which many people
> > think is fragile, but it has worked so far. I guess we could switch to
> > using `-L` (and LD_LIBRARY_PATH), but setting an environment variable
> > is easier than telling all kind of build systems to modify their
> > compiler invocations...
> >
> > I'm not sure if that qualifies as "important enough", but I think
> > that's a valid use case that would break with the proposed changes.
>
> I don't know if it's important enough either.  However, it suffers
> from the problem I mentioned in the example in the motivation of the
> RFC.  That is, system directories that cause the trouble in that
> example have precedence over Clang's lib directory, which has
> precedence over LIBRARY_PATH.  Thus, libomp.so in LIBRARY_PATH will
> not be seen if such a system directory has a libomp.so.
>
> > Maybe we can have a directory that is added after LIBRARY_PATH but
> > before all "default" system directories?
>
> That's not possible because the system directories are before LIBRARY_PATH.

Would this change even affect your use case?  Given that LIBRARY_PATH
comes after Clang's lib directory, if Clang's lib directory contains
libomp.so, then LIBRARY_PATH will not affect which libomp.so Clang
finds.  That leads me to assume your Clang lib directory must not have
a libomp.so at all, so this whole discussion of where Clang places
libomp.so seems irrelevant to your use case.  Does that make sense?

Thanks.

Joel

>
> Thanks.
>
>
> Joel
>
> >
> > > Q4: Should libcxx, libunwind, or compiler-rt be changed too?
> > > ------------------------------------------------------------
> > >
> > > An early reaction to my patch was that we need to either handle the
> > > various Clang libraries consistently or show why they should be
> > > handled differently.  My patch so far only changes openmp, and it does
> > > not make it consistent with other subprojects' libraries.
> > >
> > > Keep in mind that, while compiler-rt libraries are already placed in a
> > > Clang-dedicated directory by default, libcxx and libunwind libraries
> > > are not and so could potentially be affected by the motivating example
> > > I gave.  Must users specify
> > > `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` (which adds the problem of
> > > Clang-version locking) to avoid that problem?  Shouldn't the default
> > > configuration be the most user-friendly?
> > >
> > > Conclusion
> > > ----------
> > >
> > > So far, my estimate at the best path forward is as follows:
> > >
> > > * openmp and compiler-rt libraries should always be placed in a
> > > Clang-dedicated, Clang-version-specific directory.
> >
> > Based on the reasons given above I don't understand why openmp needs to
> > be version-locked with Clang. I think the guarantee is that you can run
> > old binaries with a newer version of the library, just as you can with
> > libc++ and friends. So why should openmp be treated differently?
> >
> > Regards,
> > Jonas
> >
> > > * libcxx and libunwind libraries should always be placed in a
> > > Clang-dedicated, Clang-version-independent directory.
> > >
> > > * For each of those directories, Clang should specify `-L` earlier
> > > than any system library directory.
> > >
> > > * openmp, libcxx, and libunwind libraries should also be symlinked
> > > directly in `lib`, for backward compatibility at least.
> > >
> > > * `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` should be dropped and
> > > dependent projects updated.  Alternatively, for backward
> > > compatibility, this option could revert the Clang-dedicated directory
> > > layout to the layout it currently produces (in particular, it places
> > > libcxx and libunwind libraries in Clang-version-specific directories).
> > >
> > > There are details like exact directory names to discuss, but does this
> > > high-level plan make sense?
> > >
> > > Thanks.
> > >
> > > Joel
> > >
> > > [1]:
> > https://reviews.llvm.org/D55725#1370823>
> > > [2]:
> > https://reviews.llvm.org/D55725>
> > > [3]:
> > https://reviews.llvm.org/D55725#1371807>
> > > [4]:
> > https://reviews.llvm.org/D30733#697781>
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Place libs in Clang-dedicated directories (affects openmp, libcxx, libunwind, compiler-rt)

Bakhvalov, Denis via cfe-dev
In reply to this post by Bakhvalov, Denis via cfe-dev
On Wed, Feb 20, 2019 at 6:14 PM Joel E. Denny <[hidden email]> wrote:
My alternative to LLVM_ENABLE_PER_TARGET_RUNTIME_DIR is the preceding
bullets.  In other words, you wouldn't need to specify
LLVM_ENABLE_PER_TARGET_RUNTIME_DIR because it would effectively be
always on (except the directories might be different than now if the
version locking issue is important, as noted above).  Is that what
you're asking?

That would be my preference. I always hoped that LLVM_ENABLE_PER_TARGET_RUNTIME_DIR would eventually become the default. It would be nice to finish the Darwin support so we can completely deprecate the old layout, but I don't know how far along beanz is in his effort. We should also update openmp to stop using the custom Android-specific runtime layout.

There's also the unresolved question of where should libc++ headers and libraries go. Currently, in LLVM_ENABLE_PER_TARGET_RUNTIME_DIR we use the resource dir, but some people expressed the opinion that we shouldn't be using these for libc++ et al. since they're not version-locked to Clang. This is different from what GCC does (e.g. GCC would use $prefix/lib/gcc/x86_64-linux-gnu/8/libstdc++.a) and it's one of the reasons why I used the resource dir for libc++ et al. when implementing LLVM_ENABLE_PER_TARGET_RUNTIME_DIR.

So concretely, today LLVM_ENABLE_PER_TARGET_RUNTIME_DIR uses the following layout:

headers: $prefix/lib/clang/$version/include(/$triple)(/c++/v1)
libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext

The alternative that doesn't use resource dir for libc++ would be the following:

compiler-rt:
  headers: $prefix/lib/clang/$version/include
  libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext

libc++, libc++abi, libunwind:
  headers: $prefix/include/c++/v1
  libraries: $prefix/lib/$triple/$name.$ext

Making this change should be trivial, it's the matter of changing three CMakeLists.txt files (libunwind, libc++abi and libc++) and Clang driver in one place. However, if we're going to make that change, I'd like to get a broader consensus. It'd be also useful to get feedback from libc++ maintainers on this.

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Place libs in Clang-dedicated directories (affects openmp, libcxx, libunwind, compiler-rt)

Bakhvalov, Denis via cfe-dev
Using the resource-dir in the header search paths would break tools that use compilation database, e.g. clang-tidy and clangd.
They override the resource-dir as it's very common for them to be built from a different revision than the used compiler. They rely on the fact that the same standard library can be found with the overridden resouce-dir.

From this point of view, what LLVM_ENABLE_PER_TARGET_RUNTIME_DIR breaks the tools and the proposed alternative fixes this problem.

On Mon, Feb 25, 2019 at 11:17 AM Petr Hosek via cfe-dev <[hidden email]> wrote:
On Wed, Feb 20, 2019 at 6:14 PM Joel E. Denny <[hidden email]> wrote:
My alternative to LLVM_ENABLE_PER_TARGET_RUNTIME_DIR is the preceding
bullets.  In other words, you wouldn't need to specify
LLVM_ENABLE_PER_TARGET_RUNTIME_DIR because it would effectively be
always on (except the directories might be different than now if the
version locking issue is important, as noted above).  Is that what
you're asking?

That would be my preference. I always hoped that LLVM_ENABLE_PER_TARGET_RUNTIME_DIR would eventually become the default. It would be nice to finish the Darwin support so we can completely deprecate the old layout, but I don't know how far along beanz is in his effort. We should also update openmp to stop using the custom Android-specific runtime layout.

There's also the unresolved question of where should libc++ headers and libraries go. Currently, in LLVM_ENABLE_PER_TARGET_RUNTIME_DIR we use the resource dir, but some people expressed the opinion that we shouldn't be using these for libc++ et al. since they're not version-locked to Clang. This is different from what GCC does (e.g. GCC would use $prefix/lib/gcc/x86_64-linux-gnu/8/libstdc++.a) and it's one of the reasons why I used the resource dir for libc++ et al. when implementing LLVM_ENABLE_PER_TARGET_RUNTIME_DIR.

So concretely, today LLVM_ENABLE_PER_TARGET_RUNTIME_DIR uses the following layout:

headers: $prefix/lib/clang/$version/include(/$triple)(/c++/v1)
libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext

The alternative that doesn't use resource dir for libc++ would be the following:

compiler-rt:
  headers: $prefix/lib/clang/$version/include
  libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext

libc++, libc++abi, libunwind:
  headers: $prefix/include/c++/v1
  libraries: $prefix/lib/$triple/$name.$ext

Making this change should be trivial, it's the matter of changing three CMakeLists.txt files (libunwind, libc++abi and libc++) and Clang driver in one place. However, if we're going to make that change, I'd like to get a broader consensus. It'd be also useful to get feedback from libc++ maintainers on this.
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


--
Regards,
Ilya Biryukov

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Place libs in Clang-dedicated directories (affects openmp, libcxx, libunwind, compiler-rt)

Bakhvalov, Denis via cfe-dev
> From this point of view, what LLVM_ENABLE_PER_TARGET_RUNTIME_DIR breaks the tools and the proposed alternative fixes this problem
what  LLVM_ENABLE_PER_TARGET_RUNTIME_DIR does now ...

On Mon, Feb 25, 2019 at 11:31 AM Ilya Biryukov <[hidden email]> wrote:
Using the resource-dir in the header search paths would break tools that use compilation database, e.g. clang-tidy and clangd.
They override the resource-dir as it's very common for them to be built from a different revision than the used compiler. They rely on the fact that the same standard library can be found with the overridden resouce-dir.

From this point of view, what LLVM_ENABLE_PER_TARGET_RUNTIME_DIR breaks the tools and the proposed alternative fixes this problem.

On Mon, Feb 25, 2019 at 11:17 AM Petr Hosek via cfe-dev <[hidden email]> wrote:
On Wed, Feb 20, 2019 at 6:14 PM Joel E. Denny <[hidden email]> wrote:
My alternative to LLVM_ENABLE_PER_TARGET_RUNTIME_DIR is the preceding
bullets.  In other words, you wouldn't need to specify
LLVM_ENABLE_PER_TARGET_RUNTIME_DIR because it would effectively be
always on (except the directories might be different than now if the
version locking issue is important, as noted above).  Is that what
you're asking?

That would be my preference. I always hoped that LLVM_ENABLE_PER_TARGET_RUNTIME_DIR would eventually become the default. It would be nice to finish the Darwin support so we can completely deprecate the old layout, but I don't know how far along beanz is in his effort. We should also update openmp to stop using the custom Android-specific runtime layout.

There's also the unresolved question of where should libc++ headers and libraries go. Currently, in LLVM_ENABLE_PER_TARGET_RUNTIME_DIR we use the resource dir, but some people expressed the opinion that we shouldn't be using these for libc++ et al. since they're not version-locked to Clang. This is different from what GCC does (e.g. GCC would use $prefix/lib/gcc/x86_64-linux-gnu/8/libstdc++.a) and it's one of the reasons why I used the resource dir for libc++ et al. when implementing LLVM_ENABLE_PER_TARGET_RUNTIME_DIR.

So concretely, today LLVM_ENABLE_PER_TARGET_RUNTIME_DIR uses the following layout:

headers: $prefix/lib/clang/$version/include(/$triple)(/c++/v1)
libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext

The alternative that doesn't use resource dir for libc++ would be the following:

compiler-rt:
  headers: $prefix/lib/clang/$version/include
  libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext

libc++, libc++abi, libunwind:
  headers: $prefix/include/c++/v1
  libraries: $prefix/lib/$triple/$name.$ext

Making this change should be trivial, it's the matter of changing three CMakeLists.txt files (libunwind, libc++abi and libc++) and Clang driver in one place. However, if we're going to make that change, I'd like to get a broader consensus. It'd be also useful to get feedback from libc++ maintainers on this.
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


--
Regards,
Ilya Biryukov


--
Regards,
Ilya Biryukov

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Place libs in Clang-dedicated directories (affects openmp, libcxx, libunwind, compiler-rt)

Bakhvalov, Denis via cfe-dev
That's useful to know and not something I was aware of, this might be a sufficient reason to alter the LLVM_ENABLE_PER_TARGET_RUNTIME_DIR layout and move libc++ out of the resource directory.

On Mon, Feb 25, 2019 at 2:32 AM Ilya Biryukov <[hidden email]> wrote:
> From this point of view, what LLVM_ENABLE_PER_TARGET_RUNTIME_DIR breaks the tools and the proposed alternative fixes this problem
what  LLVM_ENABLE_PER_TARGET_RUNTIME_DIR does now ...

On Mon, Feb 25, 2019 at 11:31 AM Ilya Biryukov <[hidden email]> wrote:
Using the resource-dir in the header search paths would break tools that use compilation database, e.g. clang-tidy and clangd.
They override the resource-dir as it's very common for them to be built from a different revision than the used compiler. They rely on the fact that the same standard library can be found with the overridden resouce-dir.

From this point of view, what LLVM_ENABLE_PER_TARGET_RUNTIME_DIR breaks the tools and the proposed alternative fixes this problem.

On Mon, Feb 25, 2019 at 11:17 AM Petr Hosek via cfe-dev <[hidden email]> wrote:
On Wed, Feb 20, 2019 at 6:14 PM Joel E. Denny <[hidden email]> wrote:
My alternative to LLVM_ENABLE_PER_TARGET_RUNTIME_DIR is the preceding
bullets.  In other words, you wouldn't need to specify
LLVM_ENABLE_PER_TARGET_RUNTIME_DIR because it would effectively be
always on (except the directories might be different than now if the
version locking issue is important, as noted above).  Is that what
you're asking?

That would be my preference. I always hoped that LLVM_ENABLE_PER_TARGET_RUNTIME_DIR would eventually become the default. It would be nice to finish the Darwin support so we can completely deprecate the old layout, but I don't know how far along beanz is in his effort. We should also update openmp to stop using the custom Android-specific runtime layout.

There's also the unresolved question of where should libc++ headers and libraries go. Currently, in LLVM_ENABLE_PER_TARGET_RUNTIME_DIR we use the resource dir, but some people expressed the opinion that we shouldn't be using these for libc++ et al. since they're not version-locked to Clang. This is different from what GCC does (e.g. GCC would use $prefix/lib/gcc/x86_64-linux-gnu/8/libstdc++.a) and it's one of the reasons why I used the resource dir for libc++ et al. when implementing LLVM_ENABLE_PER_TARGET_RUNTIME_DIR.

So concretely, today LLVM_ENABLE_PER_TARGET_RUNTIME_DIR uses the following layout:

headers: $prefix/lib/clang/$version/include(/$triple)(/c++/v1)
libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext

The alternative that doesn't use resource dir for libc++ would be the following:

compiler-rt:
  headers: $prefix/lib/clang/$version/include
  libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext

libc++, libc++abi, libunwind:
  headers: $prefix/include/c++/v1
  libraries: $prefix/lib/$triple/$name.$ext

Making this change should be trivial, it's the matter of changing three CMakeLists.txt files (libunwind, libc++abi and libc++) and Clang driver in one place. However, if we're going to make that change, I'd like to get a broader consensus. It'd be also useful to get feedback from libc++ maintainers on this.
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


--
Regards,
Ilya Biryukov


--
Regards,
Ilya Biryukov

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Place libs in Clang-dedicated directories (affects openmp, libcxx, libunwind, compiler-rt)

Bakhvalov, Denis via cfe-dev
In reply to this post by Bakhvalov, Denis via cfe-dev
Hi Joel,

On 2019-02-21 03:27, Joel E. Denny wrote:

> On Wed, Feb 20, 2019 at 6:14 PM Joel E. Denny <[hidden email]>
> wrote:
>>
>> Hi Jonas,
>>
>> On Sun, Feb 17, 2019 at 4:05 AM Jonas Hahnfeld <[hidden email]>
>> wrote:
>> >
>> > Hi Joel,
>> >
>> > Am Donnerstag, den 14.02.2019, 17:29 -0500 schrieb Joel E. Denny:
>> > > Hi,
>> > >
>> > > This RFC discusses placing Clang's libraries in Clang-dedicated
>> > > directories (somewhere within `lib/clang` instead of `lib`) in both
>> > > build trees and install trees.  While fixing broken linking with
>> > > openmp libraries was the original motivation, subprojects potentially
>> > > affected by that problem or this change also include libcxx,
>> > > libunwind, and compiler-rt.
>> > >
>> > > Motivation: Broken Default Linking
>> > > ----------------------------------
>> > >
>> > > Clang by default prefers system openmp libraries over the ones built
>> > > alongside Clang. The effect is that Clang-compiled openmp applications
>> > > sometimes misbehave if the user doesn't adjust the library search
>> > > path.
>> > >
>> > > For example, where `/home/jdenny/llvm` is built and installed from git
>> > > master, but where my system openmp libraries under
>> > > `/usr/lib/x86_64-linux-gnu` are from LLVM 6.0:
>> > >
>> > > ```
>> > > $ cd /home/jdenny/llvm
>> > > $ cat test.c
>> > > #include <stdio.h>
>> > > int main() {
>> > >   #pragma omp target teams
>> > >   printf("hello\n");
>> > >   return 0;
>> > > }
>> > > $ ./bin/clang -fopenmp -fopenmp-targets=nvptx64 test.c
>> > > $ ./a.out
>> > > ./a.out: error while loading shared libraries: libomptarget.so: cannot
>> > > open shared object file: No such file or directory
>> > > $ LD_LIBRARY_PATH=lib ./a.out
>> > > Segmentation fault (core dumped)
>> > > $ LD_LIBRARY_PATH=lib ldd a.out | grep libomp
>> > >         libomp.so.5 => /usr/lib/x86_64-linux-gnu/libomp.so.5
>> > > (0x00007fab59748000)
>> > >         libomptarget.so => lib/libomptarget.so (0x00007fab59515000)
>> > > ```
>> > >
>> > > The problem here is a little subtle.  `-v` reveals that Clang
>> > > specifies `-L/usr/lib/x86_64-linux-gnu` before
>> > > `-L/home/jdenny/llvm/lib` when linking, and I have:
>> > >
>> > > ```
>> > > $ ls -l /usr/lib/x86_64-linux-gnu/libomp.so
>> > > lrwxrwxrwx 1 root root 11 Jan  7  2018
>> > > /usr/lib/x86_64-linux-gnu/libomp.so -> libomp.so.5
>> > > ```
>> > >
>> > > As a result, Clang links the executable as follows:
>> > >
>> > > ```
>> > > $ readelf -d a.out | grep libomp.so
>> > >  0x0000000000000001 (NEEDED)             Shared library: [libomp.so.5]
>> > > ```
>> > >
>> > > Now `LD_LIBRARY_PATH` cannot force `a.out` to find Clang's `libomp.so`
>> > > because `a.out` wants `libomp.so.5` instead.
>> > >
>> > > As far as I know, there's nothing unusual about my system, which is a
>> > > typical Ubuntu 18.04.2 installation.  However, I have learned from
>> > > others that `libomp.so.5` is installed by Debian's packages not by
>> > > LLVM's cmake files [1], so many people might not see this particular
>> > > problem.
>> > >
>> > > Even if `/usr/lib/x86_64-linux-gnu/libomp.so` weren't a symbolic link,
>> > > it's still the wrong library, and incompatibilities could have caused
>> > > a link failure instead of a run-time failure.
>> > >
>> > > Either way, it seems surprising and confusing that Clang doesn't
>> > > always prefer its own libraries.
>> > >
>> > > Solution: Early `-L` for Clang-Dedicated Directory
>> > > --------------------------------------------------
>> > >
>> > > The easiest solution is probably to assume users will manually specify
>> > > `-L` when necessary to link against the right `libomp.so`.  However,
>> > > it seems like Clang should just do that automatically to help
>> > > unsuspecting users avoid surprises like the one above.
>> >
>> > +1 that Clang should get this right in most cases.
>> >
>> > > I wrote a patch to make that happen [2].  The patch's basic approach
>> > > is to place `libomp .so` in a directory for which, when linking, Clang
>> > > automatically specifies a `-L` earlier than any `-L` for a system
>> > > library directory.  It's important that the directory is a
>> > > Clang-dedicated directory (somewhere under `lib/clang`) so that other
>> > > libraries unrelated to Clang are not accidentally elevated in the
>> > > library search path.  When possible, the patch makes this change for
>> > > both the build tree and install tree so users can work out of either
>> > > tree.
>> >
>> > The drawback is that this directory is added before LIBRARY_PATH, see
>> > below.
>> >
>> > > This solution seems straight-forward, but it raises many questions.
>> > >
>> > > Q1: What is the right Clang-dedicated directory?
>> > > ------------------------------------------------
>> > >
>> > > There are many possibilities.  On my system:
>> > >
>> > > * `lib`: This directory is obviously not Clang-dedicated, but it's
>> > > where libcxx, libunwind, and openmp currently place their libraries by
>> > > default.
>> > >
>> > > * `lib/clang/9.0.0/lib/linux`: This directory is where compiler-rt
>> > > libraries are placed by default, and their names are like
>> > > `libclang_rt.asan-x86_64.so`.
>> > >
>> > > * `lib/clang/9.0.0/x86_64-unknown-linux-gnu/lib`: When the cmake
>> > > option `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` is set, this
>> > > directory is where compiler-rt, libcxx, and libunwind libraries are
>> > > placed, and the compiler-rt libraries then drop the redundant
>> > > `-x86_64` from their names.
>> > >
>> > > * `lib/clang/9.0.0/lib/linux/x86_64`: This directory has the nice
>> > > advantage that `-frtlib-add-rpath` tells Clang to add a `-rpath` for
>> > > this directory so you don't have to set `LD_LIBRARY_PATH` when
>> > > executing `a.out`.  However, it turns out that `-rpath` doesn't work
>> > > correctly for me in the case of openmp offloading [3].  As far as I
>> > > can tell, this directory is currently populated only by some Android
>> > > tool chains [3].
>> > >
>> > > * For libcxx and libunwind libraries, it's been suggested that a
>> > > Clang-dedicated directory should not be specific to the Clang version
>> > > (shouldn't contain `9.0.0`) because the libraries themselves are not
>> > > locked to the Clang version, and a possible directory layout has been
>> > > proposed [4].  My understanding is that this approach generally makes
>> > > sense when the shared libraries have version numbers, like
>> > > `libomp.so.5`.  In that case, it needs to be possible to find older
>> > > shared libraries that older software has already become dependent upon
>> > > while using older Clang installations.  It's easier to find them, with
>> > > `LD_LIBRARY_PATH` for example, if every Clang installation places such
>> > > libraries in the same Clang-version-independent directory, where the
>> > > libraries are still distinct due to the version numbers in their
>> > > names, instead of placing them in a bunch of different
>> > > Clang-version-specific directories.  Please correct me if I've
>> > > misunderstood this situation.
>> >
>> > AFAIK libc++ and libunwind are using the same ABI version .1 since the
>> > beginning. That's probably kind of the same guarantee that libomp has:
>> > You can run an old binary with a newer version of the library.
>> > However, that defeats the point of being "distinct" when installing the
>> > libraries in a single directory that is independent of the Clang
>> > version.
>>
>> I'm not close enough to these libraries to make a decision here.  It
>> sounds like libomp, libc++, and libunwind are not handled
>> consistently, and so far we don't have any solid justification for
>> that inconsistency.
>>
>> >
>> > > Which choice is right for openmp libraries?  Debian is giving version
>> > > numbers to `libomp.so`, but is that right?  If it is, the last choice
>> > > might make sense.  If not, perhaps one of the previous choices is
>> > > better.
>> > >
>> > > Q2: Should `lib` also contain the libraries?
>> > > --------------------------------------------
>> > >
>> > > As mentioned above, `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` moves
>> > > libcxx, libunwind, and compiler-rt libraries to their new
>> > > Clang-dedicated directory.  However, the patch I've proposed for
>> > > openmp instead duplicates libraries into their new Clang-dedicated
>> > > directory.  I figured duplicating would be better for backward
>> > > compatibility than moving.  Is it necessary?
>> > >
>> > > (Actually, I should probably be creating symlinks instead of duplicates.)
>> > >
>> > > Q3: Is it really OK to elevate Clang's libraries?
>> > > -------------------------------------------------
>> > >
>> > > For me, this is the biggest unknown about this change.  That is, does
>> > > any user depend on being able to drop his own `libomp.so` in a system
>> > > library directory so that Clang will prefer it over Clang's own
>> > > `libomp.so`?  If so, is that an important enough use case that we
>> > > should continue to risk subtly breaking the compiles of other users by
>> > > default?
>> >
>> > As I said in the reviews we are building and using custom versions of
>> > libomp with a globally installed (release) version of Clang (like we
>> > can do with the Intel Compiler). For my usage I'm currently relying on
>> > a combination of LIBRARY_PATH and LD_LIBRARY_PATH which many people
>> > think is fragile, but it has worked so far. I guess we could switch to
>> > using `-L` (and LD_LIBRARY_PATH), but setting an environment variable
>> > is easier than telling all kind of build systems to modify their
>> > compiler invocations...
>> >
>> > I'm not sure if that qualifies as "important enough", but I think
>> > that's a valid use case that would break with the proposed changes.
>>
>> I don't know if it's important enough either.  However, it suffers
>> from the problem I mentioned in the example in the motivation of the
>> RFC.  That is, system directories that cause the trouble in that
>> example have precedence over Clang's lib directory, which has
>> precedence over LIBRARY_PATH.  Thus, libomp.so in LIBRARY_PATH will
>> not be seen if such a system directory has a libomp.so.
>>
>> > Maybe we can have a directory that is added after LIBRARY_PATH but
>> > before all "default" system directories?
>>
>> That's not possible because the system directories are before
>> LIBRARY_PATH.
>
> Would this change even affect your use case?  Given that LIBRARY_PATH
> comes after Clang's lib directory, if Clang's lib directory contains
> libomp.so, then LIBRARY_PATH will not affect which libomp.so Clang
> finds. That leads me to assume your Clang lib directory must not have
> a libomp.so at all, so this whole discussion of where Clang places
> libomp.so seems irrelevant to your use case.  Does that make sense?

I guess you are absolutely right and I'm indeed linking against
libomp.so from Clang's lib directory (according to -Wl,-verbose). What's
saving me is that during runtime I get the "right" library because I'm
setting LD_LIBRARY_PATH, so I'm seeing new behavior that I implement in
my custom installation.
So yes, please ignore this use case because it's not affected as you
say.

Regards,
Jonas
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Place libs in Clang-dedicated directories (affects openmp, libcxx, libunwind, compiler-rt)

Bakhvalov, Denis via cfe-dev
In reply to this post by Bakhvalov, Denis via cfe-dev
Hi Petr,

On Mon, Feb 25, 2019 at 5:17 AM Petr Hosek <[hidden email]> wrote:

>
> On Wed, Feb 20, 2019 at 6:14 PM Joel E. Denny <[hidden email]> wrote:
>>
>> My alternative to LLVM_ENABLE_PER_TARGET_RUNTIME_DIR is the preceding
>> bullets.  In other words, you wouldn't need to specify
>> LLVM_ENABLE_PER_TARGET_RUNTIME_DIR because it would effectively be
>> always on (except the directories might be different than now if the
>> version locking issue is important, as noted above).  Is that what
>> you're asking?
>
>
> That would be my preference. I always hoped that LLVM_ENABLE_PER_TARGET_RUNTIME_DIR would eventually become the default. It would be nice to finish the Darwin support so we can completely deprecate the old layout, but I don't know how far along beanz is in his effort.

In the meantime, would it be helpful to duplicate (or probably
symlink) libraries in both locations by default?  I mean (1) the
current default directory, usually lib, and (2) the directory
LLVM_ENABLE_PER_TARGET_RUNTIME_DIR selects.  That strategy seems like
it should support users of either layout, but I don't know Darwin
support well enough to be sure.

> We should also update openmp to stop using the custom Android-specific runtime layout.

How does one activate that layout?

Thanks.




Joel

> There's also the unresolved question of where should libc++ headers and libraries go. Currently, in LLVM_ENABLE_PER_TARGET_RUNTIME_DIR we use the resource dir, but some people expressed the opinion that we shouldn't be using these for libc++ et al. since they're not version-locked to Clang. This is different from what GCC does (e.g. GCC would use $prefix/lib/gcc/x86_64-linux-gnu/8/libstdc++.a) and it's one of the reasons why I used the resource dir for libc++ et al. when implementing LLVM_ENABLE_PER_TARGET_RUNTIME_DIR.
>
> So concretely, today LLVM_ENABLE_PER_TARGET_RUNTIME_DIR uses the following layout:
>
> headers: $prefix/lib/clang/$version/include(/$triple)(/c++/v1)
> libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext
>
> The alternative that doesn't use resource dir for libc++ would be the following:
>
> compiler-rt:
>   headers: $prefix/lib/clang/$version/include
>   libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext
>
> libc++, libc++abi, libunwind:
>   headers: $prefix/include/c++/v1
>   libraries: $prefix/lib/$triple/$name.$ext
>
> Making this change should be trivial, it's the matter of changing three CMakeLists.txt files (libunwind, libc++abi and libc++) and Clang driver in one place. However, if we're going to make that change, I'd like to get a broader consensus. It'd be also useful to get feedback from libc++ maintainers on this.
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Place libs in Clang-dedicated directories (affects openmp, libcxx, libunwind, compiler-rt)

Bakhvalov, Denis via cfe-dev
In reply to this post by Bakhvalov, Denis via cfe-dev
Hi Ilya,

On Mon, Feb 25, 2019 at 5:32 AM Ilya Biryukov <[hidden email]> wrote:

>
> > From this point of view, what LLVM_ENABLE_PER_TARGET_RUNTIME_DIR breaks the tools and the proposed alternative fixes this problem
> what  LLVM_ENABLE_PER_TARGET_RUNTIME_DIR does now ...
>
> On Mon, Feb 25, 2019 at 11:31 AM Ilya Biryukov <[hidden email]> wrote:
>>
>> Using the resource-dir in the header search paths would break tools that use compilation database, e.g. clang-tidy and clangd.
>> They override the resource-dir as it's very common for them to be built from a different revision than the used compiler. They rely on the fact that the same standard library can be found with the overridden resouce-dir.
>>
>> From this point of view, what LLVM_ENABLE_PER_TARGET_RUNTIME_DIR breaks the tools and the proposed alternative fixes this problem.

Thanks for pointing this out.  I'd like to better understand, but I'm
only familiar with clang-tidy and clangd from a high level.  Can you
explain a bit more about how they interact with the used compiler and
how they use the overridden resource directory?

Thanks.

Joel

>>
>> On Mon, Feb 25, 2019 at 11:17 AM Petr Hosek via cfe-dev <[hidden email]> wrote:
>>>
>>> On Wed, Feb 20, 2019 at 6:14 PM Joel E. Denny <[hidden email]> wrote:
>>>>
>>>> My alternative to LLVM_ENABLE_PER_TARGET_RUNTIME_DIR is the preceding
>>>> bullets.  In other words, you wouldn't need to specify
>>>> LLVM_ENABLE_PER_TARGET_RUNTIME_DIR because it would effectively be
>>>> always on (except the directories might be different than now if the
>>>> version locking issue is important, as noted above).  Is that what
>>>> you're asking?
>>>
>>>
>>> That would be my preference. I always hoped that LLVM_ENABLE_PER_TARGET_RUNTIME_DIR would eventually become the default. It would be nice to finish the Darwin support so we can completely deprecate the old layout, but I don't know how far along beanz is in his effort. We should also update openmp to stop using the custom Android-specific runtime layout.
>>>
>>> There's also the unresolved question of where should libc++ headers and libraries go. Currently, in LLVM_ENABLE_PER_TARGET_RUNTIME_DIR we use the resource dir, but some people expressed the opinion that we shouldn't be using these for libc++ et al. since they're not version-locked to Clang. This is different from what GCC does (e.g. GCC would use $prefix/lib/gcc/x86_64-linux-gnu/8/libstdc++.a) and it's one of the reasons why I used the resource dir for libc++ et al. when implementing LLVM_ENABLE_PER_TARGET_RUNTIME_DIR.
>>>
>>> So concretely, today LLVM_ENABLE_PER_TARGET_RUNTIME_DIR uses the following layout:
>>>
>>> headers: $prefix/lib/clang/$version/include(/$triple)(/c++/v1)
>>> libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext
>>>
>>> The alternative that doesn't use resource dir for libc++ would be the following:
>>>
>>> compiler-rt:
>>>   headers: $prefix/lib/clang/$version/include
>>>   libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext
>>>
>>> libc++, libc++abi, libunwind:
>>>   headers: $prefix/include/c++/v1
>>>   libraries: $prefix/lib/$triple/$name.$ext
>>>
>>> Making this change should be trivial, it's the matter of changing three CMakeLists.txt files (libunwind, libc++abi and libc++) and Clang driver in one place. However, if we're going to make that change, I'd like to get a broader consensus. It'd be also useful to get feedback from libc++ maintainers on this.
>>> _______________________________________________
>>> cfe-dev mailing list
>>> [hidden email]
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>
>>
>>
>> --
>> Regards,
>> Ilya Biryukov
>
>
>
> --
> Regards,
> Ilya Biryukov
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Place libs in Clang-dedicated directories (affects openmp, libcxx, libunwind, compiler-rt)

Bakhvalov, Denis via cfe-dev
In reply to this post by Bakhvalov, Denis via cfe-dev
Hi Jonas,

On Tue, Feb 26, 2019 at 8:59 AM Jonas Hahnfeld <[hidden email]> wrote:

>
> Hi Joel,
>
> On 2019-02-21 03:27, Joel E. Denny wrote:
> > On Wed, Feb 20, 2019 at 6:14 PM Joel E. Denny <[hidden email]>
> > wrote:
> >>
> >> Hi Jonas,
> >>
> >> On Sun, Feb 17, 2019 at 4:05 AM Jonas Hahnfeld <[hidden email]>
> >> wrote:
> >> >
> >> > Hi Joel,
> >> >
> >> > Am Donnerstag, den 14.02.2019, 17:29 -0500 schrieb Joel E. Denny:
> >> > > Hi,
> >> > >
> >> > > This RFC discusses placing Clang's libraries in Clang-dedicated
> >> > > directories (somewhere within `lib/clang` instead of `lib`) in both
> >> > > build trees and install trees.  While fixing broken linking with
> >> > > openmp libraries was the original motivation, subprojects potentially
> >> > > affected by that problem or this change also include libcxx,
> >> > > libunwind, and compiler-rt.
> >> > >
> >> > > Motivation: Broken Default Linking
> >> > > ----------------------------------
> >> > >
> >> > > Clang by default prefers system openmp libraries over the ones built
> >> > > alongside Clang. The effect is that Clang-compiled openmp applications
> >> > > sometimes misbehave if the user doesn't adjust the library search
> >> > > path.
> >> > >
> >> > > For example, where `/home/jdenny/llvm` is built and installed from git
> >> > > master, but where my system openmp libraries under
> >> > > `/usr/lib/x86_64-linux-gnu` are from LLVM 6.0:
> >> > >
> >> > > ```
> >> > > $ cd /home/jdenny/llvm
> >> > > $ cat test.c
> >> > > #include <stdio.h>
> >> > > int main() {
> >> > >   #pragma omp target teams
> >> > >   printf("hello\n");
> >> > >   return 0;
> >> > > }
> >> > > $ ./bin/clang -fopenmp -fopenmp-targets=nvptx64 test.c
> >> > > $ ./a.out
> >> > > ./a.out: error while loading shared libraries: libomptarget.so: cannot
> >> > > open shared object file: No such file or directory
> >> > > $ LD_LIBRARY_PATH=lib ./a.out
> >> > > Segmentation fault (core dumped)
> >> > > $ LD_LIBRARY_PATH=lib ldd a.out | grep libomp
> >> > >         libomp.so.5 => /usr/lib/x86_64-linux-gnu/libomp.so.5
> >> > > (0x00007fab59748000)
> >> > >         libomptarget.so => lib/libomptarget.so (0x00007fab59515000)
> >> > > ```
> >> > >
> >> > > The problem here is a little subtle.  `-v` reveals that Clang
> >> > > specifies `-L/usr/lib/x86_64-linux-gnu` before
> >> > > `-L/home/jdenny/llvm/lib` when linking, and I have:
> >> > >
> >> > > ```
> >> > > $ ls -l /usr/lib/x86_64-linux-gnu/libomp.so
> >> > > lrwxrwxrwx 1 root root 11 Jan  7  2018
> >> > > /usr/lib/x86_64-linux-gnu/libomp.so -> libomp.so.5
> >> > > ```
> >> > >
> >> > > As a result, Clang links the executable as follows:
> >> > >
> >> > > ```
> >> > > $ readelf -d a.out | grep libomp.so
> >> > >  0x0000000000000001 (NEEDED)             Shared library: [libomp.so.5]
> >> > > ```
> >> > >
> >> > > Now `LD_LIBRARY_PATH` cannot force `a.out` to find Clang's `libomp.so`
> >> > > because `a.out` wants `libomp.so.5` instead.
> >> > >
> >> > > As far as I know, there's nothing unusual about my system, which is a
> >> > > typical Ubuntu 18.04.2 installation.  However, I have learned from
> >> > > others that `libomp.so.5` is installed by Debian's packages not by
> >> > > LLVM's cmake files [1], so many people might not see this particular
> >> > > problem.
> >> > >
> >> > > Even if `/usr/lib/x86_64-linux-gnu/libomp.so` weren't a symbolic link,
> >> > > it's still the wrong library, and incompatibilities could have caused
> >> > > a link failure instead of a run-time failure.
> >> > >
> >> > > Either way, it seems surprising and confusing that Clang doesn't
> >> > > always prefer its own libraries.
> >> > >
> >> > > Solution: Early `-L` for Clang-Dedicated Directory
> >> > > --------------------------------------------------
> >> > >
> >> > > The easiest solution is probably to assume users will manually specify
> >> > > `-L` when necessary to link against the right `libomp.so`.  However,
> >> > > it seems like Clang should just do that automatically to help
> >> > > unsuspecting users avoid surprises like the one above.
> >> >
> >> > +1 that Clang should get this right in most cases.
> >> >
> >> > > I wrote a patch to make that happen [2].  The patch's basic approach
> >> > > is to place `libomp .so` in a directory for which, when linking, Clang
> >> > > automatically specifies a `-L` earlier than any `-L` for a system
> >> > > library directory.  It's important that the directory is a
> >> > > Clang-dedicated directory (somewhere under `lib/clang`) so that other
> >> > > libraries unrelated to Clang are not accidentally elevated in the
> >> > > library search path.  When possible, the patch makes this change for
> >> > > both the build tree and install tree so users can work out of either
> >> > > tree.
> >> >
> >> > The drawback is that this directory is added before LIBRARY_PATH, see
> >> > below.
> >> >
> >> > > This solution seems straight-forward, but it raises many questions.
> >> > >
> >> > > Q1: What is the right Clang-dedicated directory?
> >> > > ------------------------------------------------
> >> > >
> >> > > There are many possibilities.  On my system:
> >> > >
> >> > > * `lib`: This directory is obviously not Clang-dedicated, but it's
> >> > > where libcxx, libunwind, and openmp currently place their libraries by
> >> > > default.
> >> > >
> >> > > * `lib/clang/9.0.0/lib/linux`: This directory is where compiler-rt
> >> > > libraries are placed by default, and their names are like
> >> > > `libclang_rt.asan-x86_64.so`.
> >> > >
> >> > > * `lib/clang/9.0.0/x86_64-unknown-linux-gnu/lib`: When the cmake
> >> > > option `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` is set, this
> >> > > directory is where compiler-rt, libcxx, and libunwind libraries are
> >> > > placed, and the compiler-rt libraries then drop the redundant
> >> > > `-x86_64` from their names.
> >> > >
> >> > > * `lib/clang/9.0.0/lib/linux/x86_64`: This directory has the nice
> >> > > advantage that `-frtlib-add-rpath` tells Clang to add a `-rpath` for
> >> > > this directory so you don't have to set `LD_LIBRARY_PATH` when
> >> > > executing `a.out`.  However, it turns out that `-rpath` doesn't work
> >> > > correctly for me in the case of openmp offloading [3].  As far as I
> >> > > can tell, this directory is currently populated only by some Android
> >> > > tool chains [3].
> >> > >
> >> > > * For libcxx and libunwind libraries, it's been suggested that a
> >> > > Clang-dedicated directory should not be specific to the Clang version
> >> > > (shouldn't contain `9.0.0`) because the libraries themselves are not
> >> > > locked to the Clang version, and a possible directory layout has been
> >> > > proposed [4].  My understanding is that this approach generally makes
> >> > > sense when the shared libraries have version numbers, like
> >> > > `libomp.so.5`.  In that case, it needs to be possible to find older
> >> > > shared libraries that older software has already become dependent upon
> >> > > while using older Clang installations.  It's easier to find them, with
> >> > > `LD_LIBRARY_PATH` for example, if every Clang installation places such
> >> > > libraries in the same Clang-version-independent directory, where the
> >> > > libraries are still distinct due to the version numbers in their
> >> > > names, instead of placing them in a bunch of different
> >> > > Clang-version-specific directories.  Please correct me if I've
> >> > > misunderstood this situation.
> >> >
> >> > AFAIK libc++ and libunwind are using the same ABI version .1 since the
> >> > beginning. That's probably kind of the same guarantee that libomp has:
> >> > You can run an old binary with a newer version of the library.
> >> > However, that defeats the point of being "distinct" when installing the
> >> > libraries in a single directory that is independent of the Clang
> >> > version.
> >>
> >> I'm not close enough to these libraries to make a decision here.  It
> >> sounds like libomp, libc++, and libunwind are not handled
> >> consistently, and so far we don't have any solid justification for
> >> that inconsistency.
> >>
> >> >
> >> > > Which choice is right for openmp libraries?  Debian is giving version
> >> > > numbers to `libomp.so`, but is that right?  If it is, the last choice
> >> > > might make sense.  If not, perhaps one of the previous choices is
> >> > > better.
> >> > >
> >> > > Q2: Should `lib` also contain the libraries?
> >> > > --------------------------------------------
> >> > >
> >> > > As mentioned above, `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True` moves
> >> > > libcxx, libunwind, and compiler-rt libraries to their new
> >> > > Clang-dedicated directory.  However, the patch I've proposed for
> >> > > openmp instead duplicates libraries into their new Clang-dedicated
> >> > > directory.  I figured duplicating would be better for backward
> >> > > compatibility than moving.  Is it necessary?
> >> > >
> >> > > (Actually, I should probably be creating symlinks instead of duplicates.)
> >> > >
> >> > > Q3: Is it really OK to elevate Clang's libraries?
> >> > > -------------------------------------------------
> >> > >
> >> > > For me, this is the biggest unknown about this change.  That is, does
> >> > > any user depend on being able to drop his own `libomp.so` in a system
> >> > > library directory so that Clang will prefer it over Clang's own
> >> > > `libomp.so`?  If so, is that an important enough use case that we
> >> > > should continue to risk subtly breaking the compiles of other users by
> >> > > default?
> >> >
> >> > As I said in the reviews we are building and using custom versions of
> >> > libomp with a globally installed (release) version of Clang (like we
> >> > can do with the Intel Compiler). For my usage I'm currently relying on
> >> > a combination of LIBRARY_PATH and LD_LIBRARY_PATH which many people
> >> > think is fragile, but it has worked so far. I guess we could switch to
> >> > using `-L` (and LD_LIBRARY_PATH), but setting an environment variable
> >> > is easier than telling all kind of build systems to modify their
> >> > compiler invocations...
> >> >
> >> > I'm not sure if that qualifies as "important enough", but I think
> >> > that's a valid use case that would break with the proposed changes.
> >>
> >> I don't know if it's important enough either.  However, it suffers
> >> from the problem I mentioned in the example in the motivation of the
> >> RFC.  That is, system directories that cause the trouble in that
> >> example have precedence over Clang's lib directory, which has
> >> precedence over LIBRARY_PATH.  Thus, libomp.so in LIBRARY_PATH will
> >> not be seen if such a system directory has a libomp.so.
> >>
> >> > Maybe we can have a directory that is added after LIBRARY_PATH but
> >> > before all "default" system directories?
> >>
> >> That's not possible because the system directories are before
> >> LIBRARY_PATH.
> >
> > Would this change even affect your use case?  Given that LIBRARY_PATH
> > comes after Clang's lib directory, if Clang's lib directory contains
> > libomp.so, then LIBRARY_PATH will not affect which libomp.so Clang
> > finds. That leads me to assume your Clang lib directory must not have
> > a libomp.so at all,

I forgot that your Clang's lib directory might indeed have a libomp.so
because linking often (or always so far for me) manages to succeed
when the wrong libomp.so is found.

> > so this whole discussion of where Clang places
> > libomp.so seems irrelevant to your use case.  Does that make sense?
>
> I guess you are absolutely right and I'm indeed linking against
> libomp.so from Clang's lib directory (according to -Wl,-verbose). What's
> saving me is that during runtime I get the "right" library because I'm
> setting LD_LIBRARY_PATH, so I'm seeing new behavior that I implement in
> my custom installation.
> So yes, please ignore this use case because it's not affected as you
> say.

Thanks for confirming.

It sounds like we so far have no use case that answers no to "Q3: Is
it really OK to elevate Clang's libraries?".

Joel

>
> Regards,
> Jonas
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Place libs in Clang-dedicated directories (affects openmp, libcxx, libunwind, compiler-rt)

Bakhvalov, Denis via cfe-dev
In reply to this post by Bakhvalov, Denis via cfe-dev
Hi Joel,

clangd, clang-tidy and other tools do not require to be built from the same revision as the host compiler that the project uses to build code. In fact, the compiler is not necessarily clang, it can be gcc or MSVC.
However, the internal clang headers (the ones -resource-dir points at) must correspond to the same version of the code that the clang frontend is built from. 
So the aforementioned tools ship their own version of clang's internal headers and pass -resource-dir to the clang frontend to make sure the frontend picks them up. I.e. if the host compiler is also clang, the tools must not pick the host clang's internal headers.
The tools take other compilation arguments from a compilation database (compile_commands.json).

Note that the internal headers is the only thing that the tools need to override, e.g. this should not affect the C++ standard library found by the tools.
For that to work, we have to make sure the internal headers is the only thing affected by "-resource-dir", we don't want the tools to see a different standard library (or not find a standard library at all).

So far in cases where -resource-dir was used for finding libc++, replacing it with "compiler install dir" ("Driver.InstalledDir")  seemed to do the job.

On Wed, Feb 27, 2019 at 12:09 AM Joel E. Denny <[hidden email]> wrote:
Hi Ilya,

On Mon, Feb 25, 2019 at 5:32 AM Ilya Biryukov <[hidden email]> wrote:
>
> > From this point of view, what LLVM_ENABLE_PER_TARGET_RUNTIME_DIR breaks the tools and the proposed alternative fixes this problem
> what  LLVM_ENABLE_PER_TARGET_RUNTIME_DIR does now ...
>
> On Mon, Feb 25, 2019 at 11:31 AM Ilya Biryukov <[hidden email]> wrote:
>>
>> Using the resource-dir in the header search paths would break tools that use compilation database, e.g. clang-tidy and clangd.
>> They override the resource-dir as it's very common for them to be built from a different revision than the used compiler. They rely on the fact that the same standard library can be found with the overridden resouce-dir.
>>
>> From this point of view, what LLVM_ENABLE_PER_TARGET_RUNTIME_DIR breaks the tools and the proposed alternative fixes this problem.

Thanks for pointing this out.  I'd like to better understand, but I'm
only familiar with clang-tidy and clangd from a high level.  Can you
explain a bit more about how they interact with the used compiler and
how they use the overridden resource directory?

Thanks.

Joel

>>
>> On Mon, Feb 25, 2019 at 11:17 AM Petr Hosek via cfe-dev <[hidden email]> wrote:
>>>
>>> On Wed, Feb 20, 2019 at 6:14 PM Joel E. Denny <[hidden email]> wrote:
>>>>
>>>> My alternative to LLVM_ENABLE_PER_TARGET_RUNTIME_DIR is the preceding
>>>> bullets.  In other words, you wouldn't need to specify
>>>> LLVM_ENABLE_PER_TARGET_RUNTIME_DIR because it would effectively be
>>>> always on (except the directories might be different than now if the
>>>> version locking issue is important, as noted above).  Is that what
>>>> you're asking?
>>>
>>>
>>> That would be my preference. I always hoped that LLVM_ENABLE_PER_TARGET_RUNTIME_DIR would eventually become the default. It would be nice to finish the Darwin support so we can completely deprecate the old layout, but I don't know how far along beanz is in his effort. We should also update openmp to stop using the custom Android-specific runtime layout.
>>>
>>> There's also the unresolved question of where should libc++ headers and libraries go. Currently, in LLVM_ENABLE_PER_TARGET_RUNTIME_DIR we use the resource dir, but some people expressed the opinion that we shouldn't be using these for libc++ et al. since they're not version-locked to Clang. This is different from what GCC does (e.g. GCC would use $prefix/lib/gcc/x86_64-linux-gnu/8/libstdc++.a) and it's one of the reasons why I used the resource dir for libc++ et al. when implementing LLVM_ENABLE_PER_TARGET_RUNTIME_DIR.
>>>
>>> So concretely, today LLVM_ENABLE_PER_TARGET_RUNTIME_DIR uses the following layout:
>>>
>>> headers: $prefix/lib/clang/$version/include(/$triple)(/c++/v1)
>>> libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext
>>>
>>> The alternative that doesn't use resource dir for libc++ would be the following:
>>>
>>> compiler-rt:
>>>   headers: $prefix/lib/clang/$version/include
>>>   libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext
>>>
>>> libc++, libc++abi, libunwind:
>>>   headers: $prefix/include/c++/v1
>>>   libraries: $prefix/lib/$triple/$name.$ext
>>>
>>> Making this change should be trivial, it's the matter of changing three CMakeLists.txt files (libunwind, libc++abi and libc++) and Clang driver in one place. However, if we're going to make that change, I'd like to get a broader consensus. It'd be also useful to get feedback from libc++ maintainers on this.
>>> _______________________________________________
>>> cfe-dev mailing list
>>> [hidden email]
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>
>>
>>
>> --
>> Regards,
>> Ilya Biryukov
>
>
>
> --
> Regards,
> Ilya Biryukov


--
Regards,
Ilya Biryukov

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Place libs in Clang-dedicated directories (affects openmp, libcxx, libunwind, compiler-rt)

Bakhvalov, Denis via cfe-dev
Hi Ilya,

On Wed, Feb 27, 2019 at 6:08 AM Ilya Biryukov <[hidden email]> wrote:
>
> Hi Joel,
>
> clangd, clang-tidy and other tools do not require to be built from the same revision as the host compiler that the project uses to build code. In fact, the compiler is not necessarily clang, it can be gcc or MSVC.
> However, the internal clang headers (the ones -resource-dir points at) must correspond to the same version of the code that the clang frontend is built from.
> So the aforementioned tools ship their own version of clang's internal headers and pass -resource-dir to the clang frontend to make sure the frontend picks them up. I.e. if the host compiler is also clang, the tools must not pick the host clang's internal headers.
> The tools take other compilation arguments from a compilation database (compile_commands.json).

Thanks.  That clears up a lot for me.

Naively, I would've thought any project (clangd, clang-tidy, or any
project external to LLVM) would specify -I to tell the compiler
(clang, gcc, or MSVC) where to find the project's required headers
when building the project.  Using -resource-dir sounds like a special
shortcut for the case where the project is clang-based (clangd or
clang-tidy) and the compiler building the project is clang.  Is that
right?  What do clangd and clang-tidy specify to the compiler if the
compiler is instead gcc or MSVC?  Do those have an option equivalent
to -resource-dir?

I don't mean to be arguing against the usage of -resource-dir for this
purpose.  I'm just trying to understand it.

>
> Note that the internal headers is the only thing that the tools need to override, e.g. this should not affect the C++ standard library found by the tools.
> For that to work, we have to make sure the internal headers is the only thing affected by "-resource-dir", we don't want the tools to see a different standard library (or not find a standard library at all).

Makes sense.

>
> So far in cases where -resource-dir was used for finding libc++, replacing it with "compiler install dir" ("Driver.InstalledDir")  seemed to do the job.

Does "resource-dir used for finding libc++" imply libc++ is in the
resource directory and so LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True?

Thanks.

Joel

>
> On Wed, Feb 27, 2019 at 12:09 AM Joel E. Denny <[hidden email]> wrote:
>>
>> Hi Ilya,
>>
>> On Mon, Feb 25, 2019 at 5:32 AM Ilya Biryukov <[hidden email]> wrote:
>> >
>> > > From this point of view, what LLVM_ENABLE_PER_TARGET_RUNTIME_DIR breaks the tools and the proposed alternative fixes this problem
>> > what  LLVM_ENABLE_PER_TARGET_RUNTIME_DIR does now ...
>> >
>> > On Mon, Feb 25, 2019 at 11:31 AM Ilya Biryukov <[hidden email]> wrote:
>> >>
>> >> Using the resource-dir in the header search paths would break tools that use compilation database, e.g. clang-tidy and clangd.
>> >> They override the resource-dir as it's very common for them to be built from a different revision than the used compiler. They rely on the fact that the same standard library can be found with the overridden resouce-dir.
>> >>
>> >> From this point of view, what LLVM_ENABLE_PER_TARGET_RUNTIME_DIR breaks the tools and the proposed alternative fixes this problem.
>>
>> Thanks for pointing this out.  I'd like to better understand, but I'm
>> only familiar with clang-tidy and clangd from a high level.  Can you
>> explain a bit more about how they interact with the used compiler and
>> how they use the overridden resource directory?
>>
>> Thanks.
>>
>> Joel
>>
>> >>
>> >> On Mon, Feb 25, 2019 at 11:17 AM Petr Hosek via cfe-dev <[hidden email]> wrote:
>> >>>
>> >>> On Wed, Feb 20, 2019 at 6:14 PM Joel E. Denny <[hidden email]> wrote:
>> >>>>
>> >>>> My alternative to LLVM_ENABLE_PER_TARGET_RUNTIME_DIR is the preceding
>> >>>> bullets.  In other words, you wouldn't need to specify
>> >>>> LLVM_ENABLE_PER_TARGET_RUNTIME_DIR because it would effectively be
>> >>>> always on (except the directories might be different than now if the
>> >>>> version locking issue is important, as noted above).  Is that what
>> >>>> you're asking?
>> >>>
>> >>>
>> >>> That would be my preference. I always hoped that LLVM_ENABLE_PER_TARGET_RUNTIME_DIR would eventually become the default. It would be nice to finish the Darwin support so we can completely deprecate the old layout, but I don't know how far along beanz is in his effort. We should also update openmp to stop using the custom Android-specific runtime layout.
>> >>>
>> >>> There's also the unresolved question of where should libc++ headers and libraries go. Currently, in LLVM_ENABLE_PER_TARGET_RUNTIME_DIR we use the resource dir, but some people expressed the opinion that we shouldn't be using these for libc++ et al. since they're not version-locked to Clang. This is different from what GCC does (e.g. GCC would use $prefix/lib/gcc/x86_64-linux-gnu/8/libstdc++.a) and it's one of the reasons why I used the resource dir for libc++ et al. when implementing LLVM_ENABLE_PER_TARGET_RUNTIME_DIR.
>> >>>
>> >>> So concretely, today LLVM_ENABLE_PER_TARGET_RUNTIME_DIR uses the following layout:
>> >>>
>> >>> headers: $prefix/lib/clang/$version/include(/$triple)(/c++/v1)
>> >>> libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext
>> >>>
>> >>> The alternative that doesn't use resource dir for libc++ would be the following:
>> >>>
>> >>> compiler-rt:
>> >>>   headers: $prefix/lib/clang/$version/include
>> >>>   libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext
>> >>>
>> >>> libc++, libc++abi, libunwind:
>> >>>   headers: $prefix/include/c++/v1
>> >>>   libraries: $prefix/lib/$triple/$name.$ext
>> >>>
>> >>> Making this change should be trivial, it's the matter of changing three CMakeLists.txt files (libunwind, libc++abi and libc++) and Clang driver in one place. However, if we're going to make that change, I'd like to get a broader consensus. It'd be also useful to get feedback from libc++ maintainers on this.
>> >>> _______________________________________________
>> >>> cfe-dev mailing list
>> >>> [hidden email]
>> >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>> >>
>> >>
>> >>
>> >> --
>> >> Regards,
>> >> Ilya Biryukov
>> >
>> >
>> >
>> > --
>> > Regards,
>> > Ilya Biryukov
>
>
>
> --
> Regards,
> Ilya Biryukov
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Place libs in Clang-dedicated directories (affects openmp, libcxx, libunwind, compiler-rt)

Bakhvalov, Denis via cfe-dev
I've implemented https://reviews.llvm.org/D59013 which moves libunwind, libc++abi and libc++ output to lib/<target> and include/ in LLVM_ENABLE_PER_TARGET_RUNTIME_DIR build, feedback is welcome.

On Wed, Feb 27, 2019 at 10:28 AM Joel E. Denny <[hidden email]> wrote:
Hi Ilya,

On Wed, Feb 27, 2019 at 6:08 AM Ilya Biryukov <[hidden email]> wrote:
>
> Hi Joel,
>
> clangd, clang-tidy and other tools do not require to be built from the same revision as the host compiler that the project uses to build code. In fact, the compiler is not necessarily clang, it can be gcc or MSVC.
> However, the internal clang headers (the ones -resource-dir points at) must correspond to the same version of the code that the clang frontend is built from.
> So the aforementioned tools ship their own version of clang's internal headers and pass -resource-dir to the clang frontend to make sure the frontend picks them up. I.e. if the host compiler is also clang, the tools must not pick the host clang's internal headers.
> The tools take other compilation arguments from a compilation database (compile_commands.json).

Thanks.  That clears up a lot for me.

Naively, I would've thought any project (clangd, clang-tidy, or any
project external to LLVM) would specify -I to tell the compiler
(clang, gcc, or MSVC) where to find the project's required headers
when building the project.  Using -resource-dir sounds like a special
shortcut for the case where the project is clang-based (clangd or
clang-tidy) and the compiler building the project is clang.  Is that
right?  What do clangd and clang-tidy specify to the compiler if the
compiler is instead gcc or MSVC?  Do those have an option equivalent
to -resource-dir?

I don't mean to be arguing against the usage of -resource-dir for this
purpose.  I'm just trying to understand it.

>
> Note that the internal headers is the only thing that the tools need to override, e.g. this should not affect the C++ standard library found by the tools.
> For that to work, we have to make sure the internal headers is the only thing affected by "-resource-dir", we don't want the tools to see a different standard library (or not find a standard library at all).

Makes sense.

>
> So far in cases where -resource-dir was used for finding libc++, replacing it with "compiler install dir" ("Driver.InstalledDir")  seemed to do the job.

Does "resource-dir used for finding libc++" imply libc++ is in the
resource directory and so LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True?

Thanks.

Joel

>
> On Wed, Feb 27, 2019 at 12:09 AM Joel E. Denny <[hidden email]> wrote:
>>
>> Hi Ilya,
>>
>> On Mon, Feb 25, 2019 at 5:32 AM Ilya Biryukov <[hidden email]> wrote:
>> >
>> > > From this point of view, what LLVM_ENABLE_PER_TARGET_RUNTIME_DIR breaks the tools and the proposed alternative fixes this problem
>> > what  LLVM_ENABLE_PER_TARGET_RUNTIME_DIR does now ...
>> >
>> > On Mon, Feb 25, 2019 at 11:31 AM Ilya Biryukov <[hidden email]> wrote:
>> >>
>> >> Using the resource-dir in the header search paths would break tools that use compilation database, e.g. clang-tidy and clangd.
>> >> They override the resource-dir as it's very common for them to be built from a different revision than the used compiler. They rely on the fact that the same standard library can be found with the overridden resouce-dir.
>> >>
>> >> From this point of view, what LLVM_ENABLE_PER_TARGET_RUNTIME_DIR breaks the tools and the proposed alternative fixes this problem.
>>
>> Thanks for pointing this out.  I'd like to better understand, but I'm
>> only familiar with clang-tidy and clangd from a high level.  Can you
>> explain a bit more about how they interact with the used compiler and
>> how they use the overridden resource directory?
>>
>> Thanks.
>>
>> Joel
>>
>> >>
>> >> On Mon, Feb 25, 2019 at 11:17 AM Petr Hosek via cfe-dev <[hidden email]> wrote:
>> >>>
>> >>> On Wed, Feb 20, 2019 at 6:14 PM Joel E. Denny <[hidden email]> wrote:
>> >>>>
>> >>>> My alternative to LLVM_ENABLE_PER_TARGET_RUNTIME_DIR is the preceding
>> >>>> bullets.  In other words, you wouldn't need to specify
>> >>>> LLVM_ENABLE_PER_TARGET_RUNTIME_DIR because it would effectively be
>> >>>> always on (except the directories might be different than now if the
>> >>>> version locking issue is important, as noted above).  Is that what
>> >>>> you're asking?
>> >>>
>> >>>
>> >>> That would be my preference. I always hoped that LLVM_ENABLE_PER_TARGET_RUNTIME_DIR would eventually become the default. It would be nice to finish the Darwin support so we can completely deprecate the old layout, but I don't know how far along beanz is in his effort. We should also update openmp to stop using the custom Android-specific runtime layout.
>> >>>
>> >>> There's also the unresolved question of where should libc++ headers and libraries go. Currently, in LLVM_ENABLE_PER_TARGET_RUNTIME_DIR we use the resource dir, but some people expressed the opinion that we shouldn't be using these for libc++ et al. since they're not version-locked to Clang. This is different from what GCC does (e.g. GCC would use $prefix/lib/gcc/x86_64-linux-gnu/8/libstdc++.a) and it's one of the reasons why I used the resource dir for libc++ et al. when implementing LLVM_ENABLE_PER_TARGET_RUNTIME_DIR.
>> >>>
>> >>> So concretely, today LLVM_ENABLE_PER_TARGET_RUNTIME_DIR uses the following layout:
>> >>>
>> >>> headers: $prefix/lib/clang/$version/include(/$triple)(/c++/v1)
>> >>> libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext
>> >>>
>> >>> The alternative that doesn't use resource dir for libc++ would be the following:
>> >>>
>> >>> compiler-rt:
>> >>>   headers: $prefix/lib/clang/$version/include
>> >>>   libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext
>> >>>
>> >>> libc++, libc++abi, libunwind:
>> >>>   headers: $prefix/include/c++/v1
>> >>>   libraries: $prefix/lib/$triple/$name.$ext
>> >>>
>> >>> Making this change should be trivial, it's the matter of changing three CMakeLists.txt files (libunwind, libc++abi and libc++) and Clang driver in one place. However, if we're going to make that change, I'd like to get a broader consensus. It'd be also useful to get feedback from libc++ maintainers on this.
>> >>> _______________________________________________
>> >>> cfe-dev mailing list
>> >>> [hidden email]
>> >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>> >>
>> >>
>> >>
>> >> --
>> >> Regards,
>> >> Ilya Biryukov
>> >
>> >
>> >
>> > --
>> > Regards,
>> > Ilya Biryukov
>
>
>
> --
> Regards,
> Ilya Biryukov

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Place libs in Clang-dedicated directories (affects openmp, libcxx, libunwind, compiler-rt)

Bakhvalov, Denis via cfe-dev
Hi Petr,

On Wed, Mar 6, 2019 at 3:11 AM Petr Hosek <[hidden email]> wrote:
>
> I've implemented https://reviews.llvm.org/D59013 which moves libunwind, libc++abi and libc++ output to lib/<target> and include/ in LLVM_ENABLE_PER_TARGET_RUNTIME_DIR build, feedback is welcome.

Thanks for working on that.  Sorry, I was traveling and didn't have a
chance to take a look before you pushed.

So, if LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True, then all libraries in
lib/x86_64-unknown-linux-gnu are now early in my library search path.
Is that right?

One of the objections to previous patches I've seen is that promoting
non-Clang-dedicated directories (that is, their names don't contain
"/clang/") is not safe for backward compatibility because that can
affect library search order for system libraries not installed by
Clang.  Doesn't your patch have that issue?


Thanks.

Joel

>
> On Wed, Feb 27, 2019 at 10:28 AM Joel E. Denny <[hidden email]> wrote:
>>
>> Hi Ilya,
>>
>> On Wed, Feb 27, 2019 at 6:08 AM Ilya Biryukov <[hidden email]> wrote:
>> >
>> > Hi Joel,
>> >
>> > clangd, clang-tidy and other tools do not require to be built from the same revision as the host compiler that the project uses to build code. In fact, the compiler is not necessarily clang, it can be gcc or MSVC.
>> > However, the internal clang headers (the ones -resource-dir points at) must correspond to the same version of the code that the clang frontend is built from.
>> > So the aforementioned tools ship their own version of clang's internal headers and pass -resource-dir to the clang frontend to make sure the frontend picks them up. I.e. if the host compiler is also clang, the tools must not pick the host clang's internal headers.
>> > The tools take other compilation arguments from a compilation database (compile_commands.json).
>>
>> Thanks.  That clears up a lot for me.
>>
>> Naively, I would've thought any project (clangd, clang-tidy, or any
>> project external to LLVM) would specify -I to tell the compiler
>> (clang, gcc, or MSVC) where to find the project's required headers
>> when building the project.  Using -resource-dir sounds like a special
>> shortcut for the case where the project is clang-based (clangd or
>> clang-tidy) and the compiler building the project is clang.  Is that
>> right?  What do clangd and clang-tidy specify to the compiler if the
>> compiler is instead gcc or MSVC?  Do those have an option equivalent
>> to -resource-dir?
>>
>> I don't mean to be arguing against the usage of -resource-dir for this
>> purpose.  I'm just trying to understand it.
>>
>> >
>> > Note that the internal headers is the only thing that the tools need to override, e.g. this should not affect the C++ standard library found by the tools.
>> > For that to work, we have to make sure the internal headers is the only thing affected by "-resource-dir", we don't want the tools to see a different standard library (or not find a standard library at all).
>>
>> Makes sense.
>>
>> >
>> > So far in cases where -resource-dir was used for finding libc++, replacing it with "compiler install dir" ("Driver.InstalledDir")  seemed to do the job.
>>
>> Does "resource-dir used for finding libc++" imply libc++ is in the
>> resource directory and so LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True?
>>
>> Thanks.
>>
>> Joel
>>
>> >
>> > On Wed, Feb 27, 2019 at 12:09 AM Joel E. Denny <[hidden email]> wrote:
>> >>
>> >> Hi Ilya,
>> >>
>> >> On Mon, Feb 25, 2019 at 5:32 AM Ilya Biryukov <[hidden email]> wrote:
>> >> >
>> >> > > From this point of view, what LLVM_ENABLE_PER_TARGET_RUNTIME_DIR breaks the tools and the proposed alternative fixes this problem
>> >> > what  LLVM_ENABLE_PER_TARGET_RUNTIME_DIR does now ...
>> >> >
>> >> > On Mon, Feb 25, 2019 at 11:31 AM Ilya Biryukov <[hidden email]> wrote:
>> >> >>
>> >> >> Using the resource-dir in the header search paths would break tools that use compilation database, e.g. clang-tidy and clangd.
>> >> >> They override the resource-dir as it's very common for them to be built from a different revision than the used compiler. They rely on the fact that the same standard library can be found with the overridden resouce-dir.
>> >> >>
>> >> >> From this point of view, what LLVM_ENABLE_PER_TARGET_RUNTIME_DIR breaks the tools and the proposed alternative fixes this problem.
>> >>
>> >> Thanks for pointing this out.  I'd like to better understand, but I'm
>> >> only familiar with clang-tidy and clangd from a high level.  Can you
>> >> explain a bit more about how they interact with the used compiler and
>> >> how they use the overridden resource directory?
>> >>
>> >> Thanks.
>> >>
>> >> Joel
>> >>
>> >> >>
>> >> >> On Mon, Feb 25, 2019 at 11:17 AM Petr Hosek via cfe-dev <[hidden email]> wrote:
>> >> >>>
>> >> >>> On Wed, Feb 20, 2019 at 6:14 PM Joel E. Denny <[hidden email]> wrote:
>> >> >>>>
>> >> >>>> My alternative to LLVM_ENABLE_PER_TARGET_RUNTIME_DIR is the preceding
>> >> >>>> bullets.  In other words, you wouldn't need to specify
>> >> >>>> LLVM_ENABLE_PER_TARGET_RUNTIME_DIR because it would effectively be
>> >> >>>> always on (except the directories might be different than now if the
>> >> >>>> version locking issue is important, as noted above).  Is that what
>> >> >>>> you're asking?
>> >> >>>
>> >> >>>
>> >> >>> That would be my preference. I always hoped that LLVM_ENABLE_PER_TARGET_RUNTIME_DIR would eventually become the default. It would be nice to finish the Darwin support so we can completely deprecate the old layout, but I don't know how far along beanz is in his effort. We should also update openmp to stop using the custom Android-specific runtime layout.
>> >> >>>
>> >> >>> There's also the unresolved question of where should libc++ headers and libraries go. Currently, in LLVM_ENABLE_PER_TARGET_RUNTIME_DIR we use the resource dir, but some people expressed the opinion that we shouldn't be using these for libc++ et al. since they're not version-locked to Clang. This is different from what GCC does (e.g. GCC would use $prefix/lib/gcc/x86_64-linux-gnu/8/libstdc++.a) and it's one of the reasons why I used the resource dir for libc++ et al. when implementing LLVM_ENABLE_PER_TARGET_RUNTIME_DIR.
>> >> >>>
>> >> >>> So concretely, today LLVM_ENABLE_PER_TARGET_RUNTIME_DIR uses the following layout:
>> >> >>>
>> >> >>> headers: $prefix/lib/clang/$version/include(/$triple)(/c++/v1)
>> >> >>> libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext
>> >> >>>
>> >> >>> The alternative that doesn't use resource dir for libc++ would be the following:
>> >> >>>
>> >> >>> compiler-rt:
>> >> >>>   headers: $prefix/lib/clang/$version/include
>> >> >>>   libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext
>> >> >>>
>> >> >>> libc++, libc++abi, libunwind:
>> >> >>>   headers: $prefix/include/c++/v1
>> >> >>>   libraries: $prefix/lib/$triple/$name.$ext
>> >> >>>
>> >> >>> Making this change should be trivial, it's the matter of changing three CMakeLists.txt files (libunwind, libc++abi and libc++) and Clang driver in one place. However, if we're going to make that change, I'd like to get a broader consensus. It'd be also useful to get feedback from libc++ maintainers on this.
>> >> >>> _______________________________________________
>> >> >>> cfe-dev mailing list
>> >> >>> [hidden email]
>> >> >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>> >> >>
>> >> >>
>> >> >>
>> >> >> --
>> >> >> Regards,
>> >> >> Ilya Biryukov
>> >> >
>> >> >
>> >> >
>> >> > --
>> >> > Regards,
>> >> > Ilya Biryukov
>> >
>> >
>> >
>> > --
>> > Regards,
>> > Ilya Biryukov


Joel

>
> On Wed, Feb 27, 2019 at 10:28 AM Joel E. Denny <[hidden email]> wrote:
>>
>> Hi Ilya,
>>
>> On Wed, Feb 27, 2019 at 6:08 AM Ilya Biryukov <[hidden email]> wrote:
>> >
>> > Hi Joel,
>> >
>> > clangd, clang-tidy and other tools do not require to be built from the same revision as the host compiler that the project uses to build code. In fact, the compiler is not necessarily clang, it can be gcc or MSVC.
>> > However, the internal clang headers (the ones -resource-dir points at) must correspond to the same version of the code that the clang frontend is built from.
>> > So the aforementioned tools ship their own version of clang's internal headers and pass -resource-dir to the clang frontend to make sure the frontend picks them up. I.e. if the host compiler is also clang, the tools must not pick the host clang's internal headers.
>> > The tools take other compilation arguments from a compilation database (compile_commands.json).
>>
>> Thanks.  That clears up a lot for me.
>>
>> Naively, I would've thought any project (clangd, clang-tidy, or any
>> project external to LLVM) would specify -I to tell the compiler
>> (clang, gcc, or MSVC) where to find the project's required headers
>> when building the project.  Using -resource-dir sounds like a special
>> shortcut for the case where the project is clang-based (clangd or
>> clang-tidy) and the compiler building the project is clang.  Is that
>> right?  What do clangd and clang-tidy specify to the compiler if the
>> compiler is instead gcc or MSVC?  Do those have an option equivalent
>> to -resource-dir?
>>
>> I don't mean to be arguing against the usage of -resource-dir for this
>> purpose.  I'm just trying to understand it.
>>
>> >
>> > Note that the internal headers is the only thing that the tools need to override, e.g. this should not affect the C++ standard library found by the tools.
>> > For that to work, we have to make sure the internal headers is the only thing affected by "-resource-dir", we don't want the tools to see a different standard library (or not find a standard library at all).
>>
>> Makes sense.
>>
>> >
>> > So far in cases where -resource-dir was used for finding libc++, replacing it with "compiler install dir" ("Driver.InstalledDir")  seemed to do the job.
>>
>> Does "resource-dir used for finding libc++" imply libc++ is in the
>> resource directory and so LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True?
>>
>> Thanks.
>>
>> Joel
>>
>> >
>> > On Wed, Feb 27, 2019 at 12:09 AM Joel E. Denny <[hidden email]> wrote:
>> >>
>> >> Hi Ilya,
>> >>
>> >> On Mon, Feb 25, 2019 at 5:32 AM Ilya Biryukov <[hidden email]> wrote:
>> >> >
>> >> > > From this point of view, what LLVM_ENABLE_PER_TARGET_RUNTIME_DIR breaks the tools and the proposed alternative fixes this problem
>> >> > what  LLVM_ENABLE_PER_TARGET_RUNTIME_DIR does now ...
>> >> >
>> >> > On Mon, Feb 25, 2019 at 11:31 AM Ilya Biryukov <[hidden email]> wrote:
>> >> >>
>> >> >> Using the resource-dir in the header search paths would break tools that use compilation database, e.g. clang-tidy and clangd.
>> >> >> They override the resource-dir as it's very common for them to be built from a different revision than the used compiler. They rely on the fact that the same standard library can be found with the overridden resouce-dir.
>> >> >>
>> >> >> From this point of view, what LLVM_ENABLE_PER_TARGET_RUNTIME_DIR breaks the tools and the proposed alternative fixes this problem.
>> >>
>> >> Thanks for pointing this out.  I'd like to better understand, but I'm
>> >> only familiar with clang-tidy and clangd from a high level.  Can you
>> >> explain a bit more about how they interact with the used compiler and
>> >> how they use the overridden resource directory?
>> >>
>> >> Thanks.
>> >>
>> >> Joel
>> >>
>> >> >>
>> >> >> On Mon, Feb 25, 2019 at 11:17 AM Petr Hosek via cfe-dev <[hidden email]> wrote:
>> >> >>>
>> >> >>> On Wed, Feb 20, 2019 at 6:14 PM Joel E. Denny <[hidden email]> wrote:
>> >> >>>>
>> >> >>>> My alternative to LLVM_ENABLE_PER_TARGET_RUNTIME_DIR is the preceding
>> >> >>>> bullets.  In other words, you wouldn't need to specify
>> >> >>>> LLVM_ENABLE_PER_TARGET_RUNTIME_DIR because it would effectively be
>> >> >>>> always on (except the directories might be different than now if the
>> >> >>>> version locking issue is important, as noted above).  Is that what
>> >> >>>> you're asking?
>> >> >>>
>> >> >>>
>> >> >>> That would be my preference. I always hoped that LLVM_ENABLE_PER_TARGET_RUNTIME_DIR would eventually become the default. It would be nice to finish the Darwin support so we can completely deprecate the old layout, but I don't know how far along beanz is in his effort. We should also update openmp to stop using the custom Android-specific runtime layout.
>> >> >>>
>> >> >>> There's also the unresolved question of where should libc++ headers and libraries go. Currently, in LLVM_ENABLE_PER_TARGET_RUNTIME_DIR we use the resource dir, but some people expressed the opinion that we shouldn't be using these for libc++ et al. since they're not version-locked to Clang. This is different from what GCC does (e.g. GCC would use $prefix/lib/gcc/x86_64-linux-gnu/8/libstdc++.a) and it's one of the reasons why I used the resource dir for libc++ et al. when implementing LLVM_ENABLE_PER_TARGET_RUNTIME_DIR.
>> >> >>>
>> >> >>> So concretely, today LLVM_ENABLE_PER_TARGET_RUNTIME_DIR uses the following layout:
>> >> >>>
>> >> >>> headers: $prefix/lib/clang/$version/include(/$triple)(/c++/v1)
>> >> >>> libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext
>> >> >>>
>> >> >>> The alternative that doesn't use resource dir for libc++ would be the following:
>> >> >>>
>> >> >>> compiler-rt:
>> >> >>>   headers: $prefix/lib/clang/$version/include
>> >> >>>   libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext
>> >> >>>
>> >> >>> libc++, libc++abi, libunwind:
>> >> >>>   headers: $prefix/include/c++/v1
>> >> >>>   libraries: $prefix/lib/$triple/$name.$ext
>> >> >>>
>> >> >>> Making this change should be trivial, it's the matter of changing three CMakeLists.txt files (libunwind, libc++abi and libc++) and Clang driver in one place. However, if we're going to make that change, I'd like to get a broader consensus. It'd be also useful to get feedback from libc++ maintainers on this.
>> >> >>> _______________________________________________
>> >> >>> cfe-dev mailing list
>> >> >>> [hidden email]
>> >> >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>> >> >>
>> >> >>
>> >> >>
>> >> >> --
>> >> >> Regards,
>> >> >> Ilya Biryukov
>> >> >
>> >> >
>> >> >
>> >> > --
>> >> > Regards,
>> >> > Ilya Biryukov
>> >
>> >
>> >
>> > --
>> > Regards,
>> > Ilya Biryukov
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Place libs in Clang-dedicated directories (affects openmp, libcxx, libunwind, compiler-rt)

Bakhvalov, Denis via cfe-dev
On Fri, Mar 8, 2019 at 2:55 PM Joel E. Denny <[hidden email]> wrote:
Hi Petr,

On Wed, Mar 6, 2019 at 3:11 AM Petr Hosek <[hidden email]> wrote:
>
> I've implemented https://reviews.llvm.org/D59013 which moves libunwind, libc++abi and libc++ output to lib/<target> and include/ in LLVM_ENABLE_PER_TARGET_RUNTIME_DIR build, feedback is welcome.

Thanks for working on that.  Sorry, I was traveling and didn't have a
chance to take a look before you pushed.

So, if LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True, then all libraries in
lib/x86_64-unknown-linux-gnu are now early in my library search path.
Is that right?

One of the objections to previous patches I've seen is that promoting
non-Clang-dedicated directories (that is, their names don't contain
"/clang/") is not safe for backward compatibility because that can
affect library search order for system libraries not installed by
Clang.  Doesn't your patch have that issue?

I believe so. Would the alternative be inserting a clang/ component, i.e. lib/clang/x86_64-linux-gnu? I could make that change, my patch has been reverted because it broke Windows bots so I can address this in the reland.

Another alternative would be to avoid relying on -L for libraries like libunwind, libc++abi and libc++ which will always have the issue of being a subject to collisions and instead teach the driver to use a full path just like it does for compiler-rt runtimes, but that's a dramatic change.

> On Wed, Feb 27, 2019 at 10:28 AM Joel E. Denny <[hidden email]> wrote:
>>
>> Hi Ilya,
>>
>> On Wed, Feb 27, 2019 at 6:08 AM Ilya Biryukov <[hidden email]> wrote:
>> >
>> > Hi Joel,
>> >
>> > clangd, clang-tidy and other tools do not require to be built from the same revision as the host compiler that the project uses to build code. In fact, the compiler is not necessarily clang, it can be gcc or MSVC.
>> > However, the internal clang headers (the ones -resource-dir points at) must correspond to the same version of the code that the clang frontend is built from.
>> > So the aforementioned tools ship their own version of clang's internal headers and pass -resource-dir to the clang frontend to make sure the frontend picks them up. I.e. if the host compiler is also clang, the tools must not pick the host clang's internal headers.
>> > The tools take other compilation arguments from a compilation database (compile_commands.json).
>>
>> Thanks.  That clears up a lot for me.
>>
>> Naively, I would've thought any project (clangd, clang-tidy, or any
>> project external to LLVM) would specify -I to tell the compiler
>> (clang, gcc, or MSVC) where to find the project's required headers
>> when building the project.  Using -resource-dir sounds like a special
>> shortcut for the case where the project is clang-based (clangd or
>> clang-tidy) and the compiler building the project is clang.  Is that
>> right?  What do clangd and clang-tidy specify to the compiler if the
>> compiler is instead gcc or MSVC?  Do those have an option equivalent
>> to -resource-dir?
>>
>> I don't mean to be arguing against the usage of -resource-dir for this
>> purpose.  I'm just trying to understand it.
>>
>> >
>> > Note that the internal headers is the only thing that the tools need to override, e.g. this should not affect the C++ standard library found by the tools.
>> > For that to work, we have to make sure the internal headers is the only thing affected by "-resource-dir", we don't want the tools to see a different standard library (or not find a standard library at all).
>>
>> Makes sense.
>>
>> >
>> > So far in cases where -resource-dir was used for finding libc++, replacing it with "compiler install dir" ("Driver.InstalledDir")  seemed to do the job.
>>
>> Does "resource-dir used for finding libc++" imply libc++ is in the
>> resource directory and so LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True?
>>
>> Thanks.
>>
>> Joel
>>
>> >
>> > On Wed, Feb 27, 2019 at 12:09 AM Joel E. Denny <[hidden email]> wrote:
>> >>
>> >> Hi Ilya,
>> >>
>> >> On Mon, Feb 25, 2019 at 5:32 AM Ilya Biryukov <[hidden email]> wrote:
>> >> >
>> >> > > From this point of view, what LLVM_ENABLE_PER_TARGET_RUNTIME_DIR breaks the tools and the proposed alternative fixes this problem
>> >> > what  LLVM_ENABLE_PER_TARGET_RUNTIME_DIR does now ...
>> >> >
>> >> > On Mon, Feb 25, 2019 at 11:31 AM Ilya Biryukov <[hidden email]> wrote:
>> >> >>
>> >> >> Using the resource-dir in the header search paths would break tools that use compilation database, e.g. clang-tidy and clangd.
>> >> >> They override the resource-dir as it's very common for them to be built from a different revision than the used compiler. They rely on the fact that the same standard library can be found with the overridden resouce-dir.
>> >> >>
>> >> >> From this point of view, what LLVM_ENABLE_PER_TARGET_RUNTIME_DIR breaks the tools and the proposed alternative fixes this problem.
>> >>
>> >> Thanks for pointing this out.  I'd like to better understand, but I'm
>> >> only familiar with clang-tidy and clangd from a high level.  Can you
>> >> explain a bit more about how they interact with the used compiler and
>> >> how they use the overridden resource directory?
>> >>
>> >> Thanks.
>> >>
>> >> Joel
>> >>
>> >> >>
>> >> >> On Mon, Feb 25, 2019 at 11:17 AM Petr Hosek via cfe-dev <[hidden email]> wrote:
>> >> >>>
>> >> >>> On Wed, Feb 20, 2019 at 6:14 PM Joel E. Denny <[hidden email]> wrote:
>> >> >>>>
>> >> >>>> My alternative to LLVM_ENABLE_PER_TARGET_RUNTIME_DIR is the preceding
>> >> >>>> bullets.  In other words, you wouldn't need to specify
>> >> >>>> LLVM_ENABLE_PER_TARGET_RUNTIME_DIR because it would effectively be
>> >> >>>> always on (except the directories might be different than now if the
>> >> >>>> version locking issue is important, as noted above).  Is that what
>> >> >>>> you're asking?
>> >> >>>
>> >> >>>
>> >> >>> That would be my preference. I always hoped that LLVM_ENABLE_PER_TARGET_RUNTIME_DIR would eventually become the default. It would be nice to finish the Darwin support so we can completely deprecate the old layout, but I don't know how far along beanz is in his effort. We should also update openmp to stop using the custom Android-specific runtime layout.
>> >> >>>
>> >> >>> There's also the unresolved question of where should libc++ headers and libraries go. Currently, in LLVM_ENABLE_PER_TARGET_RUNTIME_DIR we use the resource dir, but some people expressed the opinion that we shouldn't be using these for libc++ et al. since they're not version-locked to Clang. This is different from what GCC does (e.g. GCC would use $prefix/lib/gcc/x86_64-linux-gnu/8/libstdc++.a) and it's one of the reasons why I used the resource dir for libc++ et al. when implementing LLVM_ENABLE_PER_TARGET_RUNTIME_DIR.
>> >> >>>
>> >> >>> So concretely, today LLVM_ENABLE_PER_TARGET_RUNTIME_DIR uses the following layout:
>> >> >>>
>> >> >>> headers: $prefix/lib/clang/$version/include(/$triple)(/c++/v1)
>> >> >>> libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext
>> >> >>>
>> >> >>> The alternative that doesn't use resource dir for libc++ would be the following:
>> >> >>>
>> >> >>> compiler-rt:
>> >> >>>   headers: $prefix/lib/clang/$version/include
>> >> >>>   libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext
>> >> >>>
>> >> >>> libc++, libc++abi, libunwind:
>> >> >>>   headers: $prefix/include/c++/v1
>> >> >>>   libraries: $prefix/lib/$triple/$name.$ext
>> >> >>>
>> >> >>> Making this change should be trivial, it's the matter of changing three CMakeLists.txt files (libunwind, libc++abi and libc++) and Clang driver in one place. However, if we're going to make that change, I'd like to get a broader consensus. It'd be also useful to get feedback from libc++ maintainers on this.
>> >> >>> _______________________________________________
>> >> >>> cfe-dev mailing list
>> >> >>> [hidden email]
>> >> >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>> >> >>
>> >> >>
>> >> >>
>> >> >> --
>> >> >> Regards,
>> >> >> Ilya Biryukov
>> >> >
>> >> >
>> >> >
>> >> > --
>> >> > Regards,
>> >> > Ilya Biryukov
>> >
>> >
>> >
>> > --
>> > Regards,
>> > Ilya Biryukov


Joel

>
> On Wed, Feb 27, 2019 at 10:28 AM Joel E. Denny <[hidden email]> wrote:
>>
>> Hi Ilya,
>>
>> On Wed, Feb 27, 2019 at 6:08 AM Ilya Biryukov <[hidden email]> wrote:
>> >
>> > Hi Joel,
>> >
>> > clangd, clang-tidy and other tools do not require to be built from the same revision as the host compiler that the project uses to build code. In fact, the compiler is not necessarily clang, it can be gcc or MSVC.
>> > However, the internal clang headers (the ones -resource-dir points at) must correspond to the same version of the code that the clang frontend is built from.
>> > So the aforementioned tools ship their own version of clang's internal headers and pass -resource-dir to the clang frontend to make sure the frontend picks them up. I.e. if the host compiler is also clang, the tools must not pick the host clang's internal headers.
>> > The tools take other compilation arguments from a compilation database (compile_commands.json).
>>
>> Thanks.  That clears up a lot for me.
>>
>> Naively, I would've thought any project (clangd, clang-tidy, or any
>> project external to LLVM) would specify -I to tell the compiler
>> (clang, gcc, or MSVC) where to find the project's required headers
>> when building the project.  Using -resource-dir sounds like a special
>> shortcut for the case where the project is clang-based (clangd or
>> clang-tidy) and the compiler building the project is clang.  Is that
>> right?  What do clangd and clang-tidy specify to the compiler if the
>> compiler is instead gcc or MSVC?  Do those have an option equivalent
>> to -resource-dir?
>>
>> I don't mean to be arguing against the usage of -resource-dir for this
>> purpose.  I'm just trying to understand it.
>>
>> >
>> > Note that the internal headers is the only thing that the tools need to override, e.g. this should not affect the C++ standard library found by the tools.
>> > For that to work, we have to make sure the internal headers is the only thing affected by "-resource-dir", we don't want the tools to see a different standard library (or not find a standard library at all).
>>
>> Makes sense.
>>
>> >
>> > So far in cases where -resource-dir was used for finding libc++, replacing it with "compiler install dir" ("Driver.InstalledDir")  seemed to do the job.
>>
>> Does "resource-dir used for finding libc++" imply libc++ is in the
>> resource directory and so LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True?
>>
>> Thanks.
>>
>> Joel
>>
>> >
>> > On Wed, Feb 27, 2019 at 12:09 AM Joel E. Denny <[hidden email]> wrote:
>> >>
>> >> Hi Ilya,
>> >>
>> >> On Mon, Feb 25, 2019 at 5:32 AM Ilya Biryukov <[hidden email]> wrote:
>> >> >
>> >> > > From this point of view, what LLVM_ENABLE_PER_TARGET_RUNTIME_DIR breaks the tools and the proposed alternative fixes this problem
>> >> > what  LLVM_ENABLE_PER_TARGET_RUNTIME_DIR does now ...
>> >> >
>> >> > On Mon, Feb 25, 2019 at 11:31 AM Ilya Biryukov <[hidden email]> wrote:
>> >> >>
>> >> >> Using the resource-dir in the header search paths would break tools that use compilation database, e.g. clang-tidy and clangd.
>> >> >> They override the resource-dir as it's very common for them to be built from a different revision than the used compiler. They rely on the fact that the same standard library can be found with the overridden resouce-dir.
>> >> >>
>> >> >> From this point of view, what LLVM_ENABLE_PER_TARGET_RUNTIME_DIR breaks the tools and the proposed alternative fixes this problem.
>> >>
>> >> Thanks for pointing this out.  I'd like to better understand, but I'm
>> >> only familiar with clang-tidy and clangd from a high level.  Can you
>> >> explain a bit more about how they interact with the used compiler and
>> >> how they use the overridden resource directory?
>> >>
>> >> Thanks.
>> >>
>> >> Joel
>> >>
>> >> >>
>> >> >> On Mon, Feb 25, 2019 at 11:17 AM Petr Hosek via cfe-dev <[hidden email]> wrote:
>> >> >>>
>> >> >>> On Wed, Feb 20, 2019 at 6:14 PM Joel E. Denny <[hidden email]> wrote:
>> >> >>>>
>> >> >>>> My alternative to LLVM_ENABLE_PER_TARGET_RUNTIME_DIR is the preceding
>> >> >>>> bullets.  In other words, you wouldn't need to specify
>> >> >>>> LLVM_ENABLE_PER_TARGET_RUNTIME_DIR because it would effectively be
>> >> >>>> always on (except the directories might be different than now if the
>> >> >>>> version locking issue is important, as noted above).  Is that what
>> >> >>>> you're asking?
>> >> >>>
>> >> >>>
>> >> >>> That would be my preference. I always hoped that LLVM_ENABLE_PER_TARGET_RUNTIME_DIR would eventually become the default. It would be nice to finish the Darwin support so we can completely deprecate the old layout, but I don't know how far along beanz is in his effort. We should also update openmp to stop using the custom Android-specific runtime layout.
>> >> >>>
>> >> >>> There's also the unresolved question of where should libc++ headers and libraries go. Currently, in LLVM_ENABLE_PER_TARGET_RUNTIME_DIR we use the resource dir, but some people expressed the opinion that we shouldn't be using these for libc++ et al. since they're not version-locked to Clang. This is different from what GCC does (e.g. GCC would use $prefix/lib/gcc/x86_64-linux-gnu/8/libstdc++.a) and it's one of the reasons why I used the resource dir for libc++ et al. when implementing LLVM_ENABLE_PER_TARGET_RUNTIME_DIR.
>> >> >>>
>> >> >>> So concretely, today LLVM_ENABLE_PER_TARGET_RUNTIME_DIR uses the following layout:
>> >> >>>
>> >> >>> headers: $prefix/lib/clang/$version/include(/$triple)(/c++/v1)
>> >> >>> libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext
>> >> >>>
>> >> >>> The alternative that doesn't use resource dir for libc++ would be the following:
>> >> >>>
>> >> >>> compiler-rt:
>> >> >>>   headers: $prefix/lib/clang/$version/include
>> >> >>>   libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext
>> >> >>>
>> >> >>> libc++, libc++abi, libunwind:
>> >> >>>   headers: $prefix/include/c++/v1
>> >> >>>   libraries: $prefix/lib/$triple/$name.$ext
>> >> >>>
>> >> >>> Making this change should be trivial, it's the matter of changing three CMakeLists.txt files (libunwind, libc++abi and libc++) and Clang driver in one place. However, if we're going to make that change, I'd like to get a broader consensus. It'd be also useful to get feedback from libc++ maintainers on this.
>> >> >>> _______________________________________________
>> >> >>> cfe-dev mailing list
>> >> >>> [hidden email]
>> >> >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>> >> >>
>> >> >>
>> >> >>
>> >> >> --
>> >> >> Regards,
>> >> >> Ilya Biryukov
>> >> >
>> >> >
>> >> >
>> >> > --
>> >> > Regards,
>> >> > Ilya Biryukov
>> >
>> >
>> >
>> > --
>> > Regards,
>> > Ilya Biryukov

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Place libs in Clang-dedicated directories (affects openmp, libcxx, libunwind, compiler-rt)

Bakhvalov, Denis via cfe-dev
On Fri, Mar 8, 2019, 6:21 PM Petr Hosek <[hidden email]> wrote:
On Fri, Mar 8, 2019 at 2:55 PM Joel E. Denny <[hidden email]> wrote:
Hi Petr,

On Wed, Mar 6, 2019 at 3:11 AM Petr Hosek <[hidden email]> wrote:
>
> I've implemented https://reviews.llvm.org/D59013 which moves libunwind, libc++abi and libc++ output to lib/<target> and include/ in LLVM_ENABLE_PER_TARGET_RUNTIME_DIR build, feedback is welcome.

Thanks for working on that.  Sorry, I was traveling and didn't have a
chance to take a look before you pushed.

So, if LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True, then all libraries in
lib/x86_64-unknown-linux-gnu are now early in my library search path.
Is that right?

One of the objections to previous patches I've seen is that promoting
non-Clang-dedicated directories (that is, their names don't contain
"/clang/") is not safe for backward compatibility because that can
affect library search order for system libraries not installed by
Clang.  Doesn't your patch have that issue?

I believe so. Would the alternative be inserting a clang/ component, i.e. lib/clang/x86_64-linux-gnu?

It sounds sufficient to me.  Just to have something to compare to, there was also an example layout here: 


I'm not sure whether that's any better.

I could make that change, my patch has been reverted because it broke Windows bots so I can address this in the reland.

Another alternative would be to avoid relying on -L for libraries like libunwind, libc++abi and libc++ which will always have the issue of being a subject to collisions and instead teach the driver to use a full path just like it does for compiler-rt runtimes, but that's a dramatic change.

What's the advantage over a Clang- dedicated directory that's early in the search path?

Thanks.

Joel


> On Wed, Feb 27, 2019 at 10:28 AM Joel E. Denny <[hidden email]> wrote:
>>
>> Hi Ilya,
>>
>> On Wed, Feb 27, 2019 at 6:08 AM Ilya Biryukov <[hidden email]> wrote:
>> >
>> > Hi Joel,
>> >
>> > clangd, clang-tidy and other tools do not require to be built from the same revision as the host compiler that the project uses to build code. In fact, the compiler is not necessarily clang, it can be gcc or MSVC.
>> > However, the internal clang headers (the ones -resource-dir points at) must correspond to the same version of the code that the clang frontend is built from.
>> > So the aforementioned tools ship their own version of clang's internal headers and pass -resource-dir to the clang frontend to make sure the frontend picks them up. I.e. if the host compiler is also clang, the tools must not pick the host clang's internal headers.
>> > The tools take other compilation arguments from a compilation database (compile_commands.json).
>>
>> Thanks.  That clears up a lot for me.
>>
>> Naively, I would've thought any project (clangd, clang-tidy, or any
>> project external to LLVM) would specify -I to tell the compiler
>> (clang, gcc, or MSVC) where to find the project's required headers
>> when building the project.  Using -resource-dir sounds like a special
>> shortcut for the case where the project is clang-based (clangd or
>> clang-tidy) and the compiler building the project is clang.  Is that
>> right?  What do clangd and clang-tidy specify to the compiler if the
>> compiler is instead gcc or MSVC?  Do those have an option equivalent
>> to -resource-dir?
>>
>> I don't mean to be arguing against the usage of -resource-dir for this
>> purpose.  I'm just trying to understand it.
>>
>> >
>> > Note that the internal headers is the only thing that the tools need to override, e.g. this should not affect the C++ standard library found by the tools.
>> > For that to work, we have to make sure the internal headers is the only thing affected by "-resource-dir", we don't want the tools to see a different standard library (or not find a standard library at all).
>>
>> Makes sense.
>>
>> >
>> > So far in cases where -resource-dir was used for finding libc++, replacing it with "compiler install dir" ("Driver.InstalledDir")  seemed to do the job.
>>
>> Does "resource-dir used for finding libc++" imply libc++ is in the
>> resource directory and so LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True?
>>
>> Thanks.
>>
>> Joel
>>
>> >
>> > On Wed, Feb 27, 2019 at 12:09 AM Joel E. Denny <[hidden email]> wrote:
>> >>
>> >> Hi Ilya,
>> >>
>> >> On Mon, Feb 25, 2019 at 5:32 AM Ilya Biryukov <[hidden email]> wrote:
>> >> >
>> >> > > From this point of view, what LLVM_ENABLE_PER_TARGET_RUNTIME_DIR breaks the tools and the proposed alternative fixes this problem
>> >> > what  LLVM_ENABLE_PER_TARGET_RUNTIME_DIR does now ...
>> >> >
>> >> > On Mon, Feb 25, 2019 at 11:31 AM Ilya Biryukov <[hidden email]> wrote:
>> >> >>
>> >> >> Using the resource-dir in the header search paths would break tools that use compilation database, e.g. clang-tidy and clangd.
>> >> >> They override the resource-dir as it's very common for them to be built from a different revision than the used compiler. They rely on the fact that the same standard library can be found with the overridden resouce-dir.
>> >> >>
>> >> >> From this point of view, what LLVM_ENABLE_PER_TARGET_RUNTIME_DIR breaks the tools and the proposed alternative fixes this problem.
>> >>
>> >> Thanks for pointing this out.  I'd like to better understand, but I'm
>> >> only familiar with clang-tidy and clangd from a high level.  Can you
>> >> explain a bit more about how they interact with the used compiler and
>> >> how they use the overridden resource directory?
>> >>
>> >> Thanks.
>> >>
>> >> Joel
>> >>
>> >> >>
>> >> >> On Mon, Feb 25, 2019 at 11:17 AM Petr Hosek via cfe-dev <[hidden email]> wrote:
>> >> >>>
>> >> >>> On Wed, Feb 20, 2019 at 6:14 PM Joel E. Denny <[hidden email]> wrote:
>> >> >>>>
>> >> >>>> My alternative to LLVM_ENABLE_PER_TARGET_RUNTIME_DIR is the preceding
>> >> >>>> bullets.  In other words, you wouldn't need to specify
>> >> >>>> LLVM_ENABLE_PER_TARGET_RUNTIME_DIR because it would effectively be
>> >> >>>> always on (except the directories might be different than now if the
>> >> >>>> version locking issue is important, as noted above).  Is that what
>> >> >>>> you're asking?
>> >> >>>
>> >> >>>
>> >> >>> That would be my preference. I always hoped that LLVM_ENABLE_PER_TARGET_RUNTIME_DIR would eventually become the default. It would be nice to finish the Darwin support so we can completely deprecate the old layout, but I don't know how far along beanz is in his effort. We should also update openmp to stop using the custom Android-specific runtime layout.
>> >> >>>
>> >> >>> There's also the unresolved question of where should libc++ headers and libraries go. Currently, in LLVM_ENABLE_PER_TARGET_RUNTIME_DIR we use the resource dir, but some people expressed the opinion that we shouldn't be using these for libc++ et al. since they're not version-locked to Clang. This is different from what GCC does (e.g. GCC would use $prefix/lib/gcc/x86_64-linux-gnu/8/libstdc++.a) and it's one of the reasons why I used the resource dir for libc++ et al. when implementing LLVM_ENABLE_PER_TARGET_RUNTIME_DIR.
>> >> >>>
>> >> >>> So concretely, today LLVM_ENABLE_PER_TARGET_RUNTIME_DIR uses the following layout:
>> >> >>>
>> >> >>> headers: $prefix/lib/clang/$version/include(/$triple)(/c++/v1)
>> >> >>> libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext
>> >> >>>
>> >> >>> The alternative that doesn't use resource dir for libc++ would be the following:
>> >> >>>
>> >> >>> compiler-rt:
>> >> >>>   headers: $prefix/lib/clang/$version/include
>> >> >>>   libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext
>> >> >>>
>> >> >>> libc++, libc++abi, libunwind:
>> >> >>>   headers: $prefix/include/c++/v1
>> >> >>>   libraries: $prefix/lib/$triple/$name.$ext
>> >> >>>
>> >> >>> Making this change should be trivial, it's the matter of changing three CMakeLists.txt files (libunwind, libc++abi and libc++) and Clang driver in one place. However, if we're going to make that change, I'd like to get a broader consensus. It'd be also useful to get feedback from libc++ maintainers on this.
>> >> >>> _______________________________________________
>> >> >>> cfe-dev mailing list
>> >> >>> [hidden email]
>> >> >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>> >> >>
>> >> >>
>> >> >>
>> >> >> --
>> >> >> Regards,
>> >> >> Ilya Biryukov
>> >> >
>> >> >
>> >> >
>> >> > --
>> >> > Regards,
>> >> > Ilya Biryukov
>> >
>> >
>> >
>> > --
>> > Regards,
>> > Ilya Biryukov


Joel

>
> On Wed, Feb 27, 2019 at 10:28 AM Joel E. Denny <[hidden email]> wrote:
>>
>> Hi Ilya,
>>
>> On Wed, Feb 27, 2019 at 6:08 AM Ilya Biryukov <[hidden email]> wrote:
>> >
>> > Hi Joel,
>> >
>> > clangd, clang-tidy and other tools do not require to be built from the same revision as the host compiler that the project uses to build code. In fact, the compiler is not necessarily clang, it can be gcc or MSVC.
>> > However, the internal clang headers (the ones -resource-dir points at) must correspond to the same version of the code that the clang frontend is built from.
>> > So the aforementioned tools ship their own version of clang's internal headers and pass -resource-dir to the clang frontend to make sure the frontend picks them up. I.e. if the host compiler is also clang, the tools must not pick the host clang's internal headers.
>> > The tools take other compilation arguments from a compilation database (compile_commands.json).
>>
>> Thanks.  That clears up a lot for me.
>>
>> Naively, I would've thought any project (clangd, clang-tidy, or any
>> project external to LLVM) would specify -I to tell the compiler
>> (clang, gcc, or MSVC) where to find the project's required headers
>> when building the project.  Using -resource-dir sounds like a special
>> shortcut for the case where the project is clang-based (clangd or
>> clang-tidy) and the compiler building the project is clang.  Is that
>> right?  What do clangd and clang-tidy specify to the compiler if the
>> compiler is instead gcc or MSVC?  Do those have an option equivalent
>> to -resource-dir?
>>
>> I don't mean to be arguing against the usage of -resource-dir for this
>> purpose.  I'm just trying to understand it.
>>
>> >
>> > Note that the internal headers is the only thing that the tools need to override, e.g. this should not affect the C++ standard library found by the tools.
>> > For that to work, we have to make sure the internal headers is the only thing affected by "-resource-dir", we don't want the tools to see a different standard library (or not find a standard library at all).
>>
>> Makes sense.
>>
>> >
>> > So far in cases where -resource-dir was used for finding libc++, replacing it with "compiler install dir" ("Driver.InstalledDir")  seemed to do the job.
>>
>> Does "resource-dir used for finding libc++" imply libc++ is in the
>> resource directory and so LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=True?
>>
>> Thanks.
>>
>> Joel
>>
>> >
>> > On Wed, Feb 27, 2019 at 12:09 AM Joel E. Denny <[hidden email]> wrote:
>> >>
>> >> Hi Ilya,
>> >>
>> >> On Mon, Feb 25, 2019 at 5:32 AM Ilya Biryukov <[hidden email]> wrote:
>> >> >
>> >> > > From this point of view, what LLVM_ENABLE_PER_TARGET_RUNTIME_DIR breaks the tools and the proposed alternative fixes this problem
>> >> > what  LLVM_ENABLE_PER_TARGET_RUNTIME_DIR does now ...
>> >> >
>> >> > On Mon, Feb 25, 2019 at 11:31 AM Ilya Biryukov <[hidden email]> wrote:
>> >> >>
>> >> >> Using the resource-dir in the header search paths would break tools that use compilation database, e.g. clang-tidy and clangd.
>> >> >> They override the resource-dir as it's very common for them to be built from a different revision than the used compiler. They rely on the fact that the same standard library can be found with the overridden resouce-dir.
>> >> >>
>> >> >> From this point of view, what LLVM_ENABLE_PER_TARGET_RUNTIME_DIR breaks the tools and the proposed alternative fixes this problem.
>> >>
>> >> Thanks for pointing this out.  I'd like to better understand, but I'm
>> >> only familiar with clang-tidy and clangd from a high level.  Can you
>> >> explain a bit more about how they interact with the used compiler and
>> >> how they use the overridden resource directory?
>> >>
>> >> Thanks.
>> >>
>> >> Joel
>> >>
>> >> >>
>> >> >> On Mon, Feb 25, 2019 at 11:17 AM Petr Hosek via cfe-dev <[hidden email]> wrote:
>> >> >>>
>> >> >>> On Wed, Feb 20, 2019 at 6:14 PM Joel E. Denny <[hidden email]> wrote:
>> >> >>>>
>> >> >>>> My alternative to LLVM_ENABLE_PER_TARGET_RUNTIME_DIR is the preceding
>> >> >>>> bullets.  In other words, you wouldn't need to specify
>> >> >>>> LLVM_ENABLE_PER_TARGET_RUNTIME_DIR because it would effectively be
>> >> >>>> always on (except the directories might be different than now if the
>> >> >>>> version locking issue is important, as noted above).  Is that what
>> >> >>>> you're asking?
>> >> >>>
>> >> >>>
>> >> >>> That would be my preference. I always hoped that LLVM_ENABLE_PER_TARGET_RUNTIME_DIR would eventually become the default. It would be nice to finish the Darwin support so we can completely deprecate the old layout, but I don't know how far along beanz is in his effort. We should also update openmp to stop using the custom Android-specific runtime layout.
>> >> >>>
>> >> >>> There's also the unresolved question of where should libc++ headers and libraries go. Currently, in LLVM_ENABLE_PER_TARGET_RUNTIME_DIR we use the resource dir, but some people expressed the opinion that we shouldn't be using these for libc++ et al. since they're not version-locked to Clang. This is different from what GCC does (e.g. GCC would use $prefix/lib/gcc/x86_64-linux-gnu/8/libstdc++.a) and it's one of the reasons why I used the resource dir for libc++ et al. when implementing LLVM_ENABLE_PER_TARGET_RUNTIME_DIR.
>> >> >>>
>> >> >>> So concretely, today LLVM_ENABLE_PER_TARGET_RUNTIME_DIR uses the following layout:
>> >> >>>
>> >> >>> headers: $prefix/lib/clang/$version/include(/$triple)(/c++/v1)
>> >> >>> libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext
>> >> >>>
>> >> >>> The alternative that doesn't use resource dir for libc++ would be the following:
>> >> >>>
>> >> >>> compiler-rt:
>> >> >>>   headers: $prefix/lib/clang/$version/include
>> >> >>>   libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext
>> >> >>>
>> >> >>> libc++, libc++abi, libunwind:
>> >> >>>   headers: $prefix/include/c++/v1
>> >> >>>   libraries: $prefix/lib/$triple/$name.$ext
>> >> >>>
>> >> >>> Making this change should be trivial, it's the matter of changing three CMakeLists.txt files (libunwind, libc++abi and libc++) and Clang driver in one place. However, if we're going to make that change, I'd like to get a broader consensus. It'd be also useful to get feedback from libc++ maintainers on this.
>> >> >>> _______________________________________________
>> >> >>> cfe-dev mailing list
>> >> >>> [hidden email]
>> >> >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>> >> >>
>> >> >>
>> >> >>
>> >> >> --
>> >> >> Regards,
>> >> >> Ilya Biryukov
>> >> >
>> >> >
>> >> >
>> >> > --
>> >> > Regards,
>> >> > Ilya Biryukov
>> >
>> >
>> >
>> > --
>> > Regards,
>> > Ilya Biryukov

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
12