[PATCH] D26244: [Driver] Prefer libraries installed next to Clang over those from GCC
mgorny added a comment. I think the easiest solution would be for you to use a path related to clang runtime directory, and install the runtimes there. However, this would require rethinking the structure a bit to cover multilib, different ABIs, maybe cross. We have a cheap hack for this in clang-suite (I can give you link when I get home) but this really requires some smart thinking. https://reviews.llvm.org/D26244 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26244: [Driver] Prefer libraries installed next to Clang over those from GCC
Hahnfeld added a comment.
In https://reviews.llvm.org/D26244#641099, @chandlerc wrote:
> Those are just the libriaries LLVM is installing.
>
> If it is installed into a prefix, say, /opt/mumble/{bin,lib,...} along with
> other libraries, then suddenly that directory is searched in a very different
> place.
>
> I'm not arguing one way or the other here. I'm pointing out that this is a
> *huge* change to make. It will cause a large number of users to silently have
> a different library search path than they have had before, something that has
> been fairly stable for several years. So whatever we do must be considered
> very carefully.
Haven't thought about that! If I don't install to `/usr/lib`, I usually only
install one software per prefix but there might be cases where that is not true.
What do you propose? Should I send an RFC to cfe-dev?
https://reviews.llvm.org/D26244
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26244: [Driver] Prefer libraries installed next to Clang over those from GCC
chandlerc added a comment.
In https://reviews.llvm.org/D26244#641076, @Hahnfeld wrote:
> This change doesn't break existing tests but maybe I should extend
> `linux-ld.c` and so on?
>
> About the impact: Which libraries are there next to clang that could conflict
> with the system? When I look at my current installation dir:
>
> - `libLLVM*`, `libclang*`, `liblld*` libraries which should be fine to use
> - `libc++`, `libc++experimental`, `libc++abi` and `libunwind` which we really
> want to use IMO
> - OpenMP runtime(s) which are fine to use (or even better than the system
> default)
> - I'm not sure about `libLTO` but I *think* that users do not link to this
> library directly?
Those are just the libriaries LLVM is installing.
If it is installed into a prefix, say, /opt/mumble/{bin,lib,...} along with
other libraries, then suddenly that directory is searched in a very different
place.
I'm not arguing one way or the other here. I'm pointing out that this is a
*huge* change to make. It will cause a large number of users to silently have a
different library search path than they have had before, something that has
been fairly stable for several years. So whatever we do must be considered very
carefully.
https://reviews.llvm.org/D26244
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26244: [Driver] Prefer libraries installed next to Clang over those from GCC
Hahnfeld updated this revision to Diff 83784.
Hahnfeld added a comment.
Add test case to check that
- `bin/../lib` is the first library path by default
- user-specified `-L` preceeds
https://reviews.llvm.org/D26244
Files:
lib/Driver/ToolChains.cpp
test/Driver/clang-libraries.c
Index: test/Driver/clang-libraries.c
===
--- /dev/null
+++ test/Driver/clang-libraries.c
@@ -0,0 +1,24 @@
+// Check that libraries installed next to Clang are preferred over those from
GCC.
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux
--gcc-toolchain=%S/Inputs/basic_linux_tree/usr \
+// RUN: | FileCheck --check-prefix=CHECK-DEFAULT %s
+
+// CHECK-DEFAULT: "[[CLANG_BIN:[^"]+]]/clang"
+// CHECK-DEFAULT: "{{.*}}ld{{(.exe)?}}"
+// CHECK-DEFAULT-NOT: "-L
+// CHECK-DEFAULT: "-L[[CLANG_BIN]]/../lib
+
+
+// Check that manual library paths preceed the default ones.
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux
--gcc-toolchain=%S/Inputs/basic_linux_tree/usr \
+// RUN: -L%S/Inputs/basic_linux_tree/usr/lib \
+// RUN: | FileCheck --check-prefix=CHECK-MANUAL %s
+
+// CHECK-MANUAL: "[[CLANG_BIN:[^"]+]]/clang"
+// CHECK-MANUAL: "{{.*}}ld{{(.exe)?}}"
+// CHECK-MANUAL: "-L{{[^"]*}}Inputs/basic_linux_tree/usr/lib
+// CHECK-MANUAL-NOT: "-L
+// CHECK-MANUAL: "-L[[CLANG_BIN]]/../lib
Index: lib/Driver/ToolChains.cpp
===
--- lib/Driver/ToolChains.cpp
+++ lib/Driver/ToolChains.cpp
@@ -4161,6 +4161,17 @@
const std::string OSLibDir = getOSLibDir(Triple, Args);
const std::string MultiarchTriple = getMultiarchTriple(D, Triple, SysRoot);
+ // Similar to the logic for GCC below, if we currently running Clang inside
+ // of the requested system root, add its parent library paths to those
+ // searched.
+ // FIXME: It's not clear whether we should use the driver's installed
+ // directory ('Dir' below) or the ResourceDir.
+ if (StringRef(D.Dir).startswith(SysRoot)) {
+addPathIfExists(D, D.Dir + "/../lib/" + MultiarchTriple, Paths);
+addPathIfExists(D, D.Dir + "/../" + OSLibDir, Paths);
+addPathIfExists(D, D.Dir + "/../lib", Paths);
+ }
+
// Add the multilib suffixed paths where they are available.
if (GCCInstallation.isValid()) {
const llvm::Triple = GCCInstallation.getTriple();
@@ -4214,16 +4225,6 @@
}
}
- // Similar to the logic for GCC above, if we currently running Clang inside
- // of the requested system root, add its parent library paths to
- // those searched.
- // FIXME: It's not clear whether we should use the driver's installed
- // directory ('Dir' below) or the ResourceDir.
- if (StringRef(D.Dir).startswith(SysRoot)) {
-addPathIfExists(D, D.Dir + "/../lib/" + MultiarchTriple, Paths);
-addPathIfExists(D, D.Dir + "/../" + OSLibDir, Paths);
- }
-
addPathIfExists(D, SysRoot + "/lib/" + MultiarchTriple, Paths);
addPathIfExists(D, SysRoot + "/lib/../" + OSLibDir, Paths);
addPathIfExists(D, SysRoot + "/usr/lib/" + MultiarchTriple, Paths);
@@ -4260,14 +4261,6 @@
addPathIfExists(D, LibPath, Paths);
}
- // Similar to the logic for GCC above, if we are currently running Clang
- // inside of the requested system root, add its parent library path to those
- // searched.
- // FIXME: It's not clear whether we should use the driver's installed
- // directory ('Dir' below) or the ResourceDir.
- if (StringRef(D.Dir).startswith(SysRoot))
-addPathIfExists(D, D.Dir + "/../lib", Paths);
-
addPathIfExists(D, SysRoot + "/lib", Paths);
addPathIfExists(D, SysRoot + "/usr/lib", Paths);
}
Index: test/Driver/clang-libraries.c
===
--- /dev/null
+++ test/Driver/clang-libraries.c
@@ -0,0 +1,24 @@
+// Check that libraries installed next to Clang are preferred over those from GCC.
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux --gcc-toolchain=%S/Inputs/basic_linux_tree/usr \
+// RUN: | FileCheck --check-prefix=CHECK-DEFAULT %s
+
+// CHECK-DEFAULT: "[[CLANG_BIN:[^"]+]]/clang"
+// CHECK-DEFAULT: "{{.*}}ld{{(.exe)?}}"
+// CHECK-DEFAULT-NOT: "-L
+// CHECK-DEFAULT: "-L[[CLANG_BIN]]/../lib
+
+
+// Check that manual library paths preceed the default ones.
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux --gcc-toolchain=%S/Inputs/basic_linux_tree/usr \
+// RUN: -L%S/Inputs/basic_linux_tree/usr/lib \
+// RUN: | FileCheck --check-prefix=CHECK-MANUAL %s
+
+// CHECK-MANUAL: "[[CLANG_BIN:[^"]+]]/clang"
+// CHECK-MANUAL: "{{.*}}ld{{(.exe)?}}"
+// CHECK-MANUAL: "-L{{[^"]*}}Inputs/basic_linux_tree/usr/lib
+// CHECK-MANUAL-NOT: "-L
+// CHECK-MANUAL: "-L[[CLANG_BIN]]/../lib
Index: lib/Driver/ToolChains.cpp
[PATCH] D26244: [Driver] Prefer libraries installed next to Clang over those from GCC
Hahnfeld added a comment. This change doesn't break existing tests but maybe I should extend `linux-ld.c` and so on? About the impact: Which libraries are there next to clang that could conflict with the system? When I look at my current installation dir: - `libLLVM*`, `libclang*`, `liblld*` libraries which should be fine to use - `libc++`, `libc++experimental`, `libc++abi` and `libunwind` which we really want to use IMO - OpenMP runtime(s) which are fine to use (or even better than the system default) - I'm not sure about `libLTO` but I *think* that users do not link to this library directly? https://reviews.llvm.org/D26244 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26244: [Driver] Prefer libraries installed next to Clang over those from GCC
chandlerc requested changes to this revision. chandlerc added a comment. This revision now requires changes to proceed. There are some issues with this patch currently. At a basic level, it needs well crafted test cases. You can find out how to write them by looking at other Driver tests. But at a deeper level, the change here will be an very significant and potentially very disruptive change. While libunwind is your motivation, it could well have a dramatic impact on the selected set of libraries for links. I'm very concerned to make this kind of change without a *lot* of careful testing, and a very clear rationale about why this is the right search order *for all libraries*, not just libunwind. Because this applies to all libraries. =] https://reviews.llvm.org/D26244 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26244: [Driver] Prefer libraries installed next to Clang over those from GCC
rsmith added a comment. This makes sense to me in principle, but I'd like @chandlerc to weigh in. https://reviews.llvm.org/D26244 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26244: [Driver] Prefer libraries installed next to Clang over those from GCC
Hahnfeld added a comment. Ping. Could I get comments before the branching this week? https://reviews.llvm.org/D26244 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26244: [Driver] Prefer libraries installed next to Clang over those from GCC
Hahnfeld added a reviewer: chandlerc. Hahnfeld added a comment. Ping. Looks like Chandler originally added this logic... https://reviews.llvm.org/D26244 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26244: [Driver] Prefer libraries installed next to Clang over those from GCC
EricWF resigned from this revision. EricWF removed a reviewer: EricWF. EricWF added a comment. I don't feel comfortable reviewing driver changes, since I never work on that code. https://reviews.llvm.org/D26244 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26244: [Driver] Prefer libraries installed next to Clang over those from GCC
Hahnfeld added a comment. ping https://reviews.llvm.org/D26244 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26244: [Driver] Prefer libraries installed next to Clang over those from GCC
Hahnfeld added a comment. Ping! https://reviews.llvm.org/D26244 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26244: [Driver] Prefer libraries installed next to Clang over those from GCC
Hahnfeld added a comment. ping https://reviews.llvm.org/D26244 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26244: [Driver] Prefer libraries installed next to Clang over those from GCC
Hahnfeld added a comment. ping https://reviews.llvm.org/D26244 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26244: [Driver] Prefer libraries installed next to Clang over those from GCC
Hahnfeld created this revision.
Hahnfeld added reviewers: mgorny, rsmith, EricWF.
Hahnfeld added a subscriber: cfe-commits.
This is for example needed to make sure we get LLVM's `libunwind.so.1` and
don't end up with the system default non-GNU `libunwind.so.8` as reported in
https://reviews.llvm.org/D25008.
https://reviews.llvm.org/D26244
Files:
lib/Driver/ToolChains.cpp
Index: lib/Driver/ToolChains.cpp
===
--- lib/Driver/ToolChains.cpp
+++ lib/Driver/ToolChains.cpp
@@ -4222,6 +4222,17 @@
const std::string OSLibDir = getOSLibDir(Triple, Args);
const std::string MultiarchTriple = getMultiarchTriple(D, Triple, SysRoot);
+ // Similar to the logic for GCC below, if we currently running Clang inside
+ // of the requested system root, add its parent library paths to those
+ // searched.
+ // FIXME: It's not clear whether we should use the driver's installed
+ // directory ('Dir' below) or the ResourceDir.
+ if (StringRef(D.Dir).startswith(SysRoot)) {
+addPathIfExists(D, D.Dir + "/../lib/" + MultiarchTriple, Paths);
+addPathIfExists(D, D.Dir + "/../" + OSLibDir, Paths);
+addPathIfExists(D, D.Dir + "/../lib", Paths);
+ }
+
// Add the multilib suffixed paths where they are available.
if (GCCInstallation.isValid()) {
const llvm::Triple = GCCInstallation.getTriple();
@@ -4275,16 +4286,6 @@
}
}
- // Similar to the logic for GCC above, if we currently running Clang inside
- // of the requested system root, add its parent library paths to
- // those searched.
- // FIXME: It's not clear whether we should use the driver's installed
- // directory ('Dir' below) or the ResourceDir.
- if (StringRef(D.Dir).startswith(SysRoot)) {
-addPathIfExists(D, D.Dir + "/../lib/" + MultiarchTriple, Paths);
-addPathIfExists(D, D.Dir + "/../" + OSLibDir, Paths);
- }
-
addPathIfExists(D, SysRoot + "/lib/" + MultiarchTriple, Paths);
addPathIfExists(D, SysRoot + "/lib/../" + OSLibDir, Paths);
addPathIfExists(D, SysRoot + "/usr/lib/" + MultiarchTriple, Paths);
@@ -4321,14 +4322,6 @@
addPathIfExists(D, LibPath, Paths);
}
- // Similar to the logic for GCC above, if we are currently running Clang
- // inside of the requested system root, add its parent library path to those
- // searched.
- // FIXME: It's not clear whether we should use the driver's installed
- // directory ('Dir' below) or the ResourceDir.
- if (StringRef(D.Dir).startswith(SysRoot))
-addPathIfExists(D, D.Dir + "/../lib", Paths);
-
addPathIfExists(D, SysRoot + "/lib", Paths);
addPathIfExists(D, SysRoot + "/usr/lib", Paths);
}
Index: lib/Driver/ToolChains.cpp
===
--- lib/Driver/ToolChains.cpp
+++ lib/Driver/ToolChains.cpp
@@ -4222,6 +4222,17 @@
const std::string OSLibDir = getOSLibDir(Triple, Args);
const std::string MultiarchTriple = getMultiarchTriple(D, Triple, SysRoot);
+ // Similar to the logic for GCC below, if we currently running Clang inside
+ // of the requested system root, add its parent library paths to those
+ // searched.
+ // FIXME: It's not clear whether we should use the driver's installed
+ // directory ('Dir' below) or the ResourceDir.
+ if (StringRef(D.Dir).startswith(SysRoot)) {
+addPathIfExists(D, D.Dir + "/../lib/" + MultiarchTriple, Paths);
+addPathIfExists(D, D.Dir + "/../" + OSLibDir, Paths);
+addPathIfExists(D, D.Dir + "/../lib", Paths);
+ }
+
// Add the multilib suffixed paths where they are available.
if (GCCInstallation.isValid()) {
const llvm::Triple = GCCInstallation.getTriple();
@@ -4275,16 +4286,6 @@
}
}
- // Similar to the logic for GCC above, if we currently running Clang inside
- // of the requested system root, add its parent library paths to
- // those searched.
- // FIXME: It's not clear whether we should use the driver's installed
- // directory ('Dir' below) or the ResourceDir.
- if (StringRef(D.Dir).startswith(SysRoot)) {
-addPathIfExists(D, D.Dir + "/../lib/" + MultiarchTriple, Paths);
-addPathIfExists(D, D.Dir + "/../" + OSLibDir, Paths);
- }
-
addPathIfExists(D, SysRoot + "/lib/" + MultiarchTriple, Paths);
addPathIfExists(D, SysRoot + "/lib/../" + OSLibDir, Paths);
addPathIfExists(D, SysRoot + "/usr/lib/" + MultiarchTriple, Paths);
@@ -4321,14 +4322,6 @@
addPathIfExists(D, LibPath, Paths);
}
- // Similar to the logic for GCC above, if we are currently running Clang
- // inside of the requested system root, add its parent library path to those
- // searched.
- // FIXME: It's not clear whether we should use the driver's installed
- // directory ('Dir' below) or the ResourceDir.
- if (StringRef(D.Dir).startswith(SysRoot))
-addPathIfExists(D, D.Dir + "/../lib", Paths);
-
addPathIfExists(D, SysRoot + "/lib", Paths);
addPathIfExists(D, SysRoot + "/usr/lib", Paths);
}
