llvmPackages_*.clang: bind linker on compiling clang #220595
Conversation
|
seems like a loop occurs, let me find out how to fix |
SharzyL
force-pushed
the
clang_bind_ld
branch
2 times, most recently
from
18dd5e1
to
442b21a
Compare
|
The infinite recursion is caused by wasi libc introduced by firefox. But I find it hard to figure out how the recursion occurs. Is there someone familier with bootstrapping to offer a help? |
It's a recursion in |
|
It has been a while. Now I finally figured out how the circular dependency occurs. Recall that in our original implementation, we imitate gcc and add the following flags to clang diff --git a/pkgs/development/compilers/llvm/12/clang/default.nix b/pkgs/development/compilers/llvm/12/clang/default.nix
index 7ecd4efc0837..d8f6e50b348c 100644
--- a/pkgs/development/compilers/llvm/12/clang/default.nix
+++ b/pkgs/development/compilers/llvm/12/clang/default.nix
@@ -2,6 +2,7 @@
, buildLlvmTools
, fixDarwinDylibNames
, enableManpages ? false
+, targetPackages
}:
let
@@ -40,6 +41,8 @@ let
] ++ lib.optionals (stdenv.hostPlatform != stdenv.buildPlatform) [
"-DLLVM_TABLEGEN_EXE=${buildLlvmTools.llvm}/bin/llvm-tblgen"
"-DCLANG_TABLEGEN=${buildLlvmTools.libclang.dev}/bin/clang-tblgen"
+ ] ++ lib.optionals (stdenv.hostPlatform != stdenv.targetPlatform) [
+ "-DCLANG_DEFAULT_LINKER=${targetPackages.stdenv.cc.bintools}/bin/${stdenv.targetPlatform.config}-ld"
];
patches = [This change introduced two circles: Circle 1
How does gcc avoid this circle? The magic occurs in the last dependency step. For gcc-based pkgsCross, Now we do the same thing to diff --git a/pkgs/development/compilers/llvm/12/default.nix b/pkgs/development/compilers/llvm/12/default.nix
index b976dd2ee67a..81bed7d9db96 100644
--- a/pkgs/development/compilers/llvm/12/default.nix
+++ b/pkgs/development/compilers/llvm/12/default.nix
@@ -210,7 +210,9 @@ let
};
clangNoLibc = wrapCCWith rec {
- cc = tools.clang-unwrapped;
+ cc = tools.clang-unwrapped.override {
+ targetPackages.stdenv.cc.bintools = bintoolsNoLibc';
+ };
libcxx = null;
bintools = bintoolsNoLibc';
extraPackages = [
@@ -223,7 +225,9 @@ let
};Circle 2But the story is not coming to the end. There is another circle:
Luckily similar solution works: diff --git a/pkgs/development/compilers/llvm/12/default.nix b/pkgs/development/compilers/llvm/12/default.nix
index b976dd2ee67a..81bed7d9db96 100644
--- a/pkgs/development/compilers/llvm/12/default.nix
+++ b/pkgs/development/compilers/llvm/12/default.nix
@@ -223,7 +225,9 @@ let
};
clangNoCompilerRt = wrapCCWith rec {
- cc = tools.clang-unwrapped;
+ cc = tools.clang-unwrapped.override {
+ targetPackages.stdenv.cc.bintools = bintoolsNoLibc';
+ };
libcxx = null;
bintools = bintoolsNoLibc';
extraPackages = [ ]; |
SharzyL
force-pushed
the
clang_bind_ld
branch
from
442b21a
to
261b67c
Compare
SharzyL
force-pushed
the
clang_bind_ld
branch
from
261b67c
to
d847d68
Compare
|
Now the circle occurs in darwin. Yet another odyssey to figure out why. |
|
Thank you for this amazing PR, I was going to embark on the journey too. |
RaitoBezarius
requested review from
Ericson2314 and
alyssais
and removed request for
trofi
|
To reproduce the error locally: |
You may want to look at #256590. Darwin cross is messy because the source-based SDK is prone infinite recursions if you’re not careful. The stdenv bootstrap is able to do that, but cross-compilation doesn’t allow that level of control. #260543 is another PR that fixes cross-compilation of LLVM itself, which I found was necessary for building some things after I got x86_64-darwin cross-compilation working. The merge conflict is probably due to #260963, which made curl build like a normal package on Darwin, but it required stdenv bootstrap changes to break more infinite recursions. I was planning to fix #256590 after this staging-next (where my focus is on getting the default LLVM update through), but I can fix the merge conflict and re-push the PR if that would be helpful or there’s interest in getting it into staging for the next staging-next cycle. |
| ] ++ lib.optionals (stdenv.hostPlatform != stdenv.targetPlatform) [ | ||
| "-DCLANG_DEFAULT_LINKER=${targetPackages.stdenv.cc.bintools}/bin/${stdenv.targetPlatform.config}-ld" |
There was a problem hiding this comment.
Are you sure targetPackages is the right thing to use here?
targetPackages means pkgsTargetTarget, which is the packages which run on the targetPlatform and emit binaries for the targetPlatform.
When you're building a (build==host)!=target compiler, its linker will run on the hostPlatform
But I don't know clang very well so maybe I am misreading the meaning of -DCLANG_DEFAULT_LINKER. @Ericson2314 would know for sure.
There was a problem hiding this comment.
Yep, I agree with @amjoseph-nixpkgs here.
I would expect rather pkgsBuildTarget to be used here, but maybe there's some splicing surprises… ?
There was a problem hiding this comment.
This might look suprising. Yes targetPackages is the packages that run on targetPlatform and emit binaries for the targetPlatform, but targetPackages.stdenv is something else. It is the environment that produces targetPackages. Now that targetPackages is the cross packages, targetPackages.stdenv.cc.bintools is the bintools that runs on host platform and emit target platform binaries.
|
Holy smokes these treewide operations on |
@rrbutani tried… It was hard because we need to unify all changes first :( |
When using clang for cross compilation, clang cannot find the linker. Because, for consistency with gcc, given a linker name, clang finds the linker in the following order: 1. in prefix dirs, which contains toolchain specific paths and paths specified by `-B` 2. in ProgramPaths 3. in PATH environment (refer to `Driver::GetProgramPath` in `clang/lib/Driver/Driver.cpp`) (also refer to https://gcc.gnu.org/onlinedocs/gccint/Collect2.html) The step 2 and 3 considers all possible target triple prefixes, but the step 1 does not. In nixpkgs, all binaries in cross toolchains are prefixed by the target triple, thus clang cannot find them. For gcc in nixpkgs, the linker path is hardcoded in configuration phase. This commit copies this behavior to clang. But the added cmakeFlags also introduce circular dependency. The circle comes from the fact that clangNoLibc and clangNoCompilerRT introduce dependency of libc unexpectedly via cmakeFlags of their underlying clang. We need to override bintools for them. Refer to NixOS#220595 (comment) for details.
SharzyL
force-pushed
the
clang_bind_ld
branch
from
d847d68
to
b89ce8b
Compare
|
Added fixes for darwin packages. Let's see if ofborg likes it. |
ofborg
bot
added
10.rebuild-darwin: 1-10
10.rebuild-darwin: 1
10.rebuild-linux: 11-100
labels
wegank
added
the
2.status: merge conflict
rrbutani
added
the
6.topic: llvm/clang
wegank
added
the
2.status: stale
|
Sorry I'm late for the party, but I want to advocate for rejecting this PR. Rationale:
Therefore, it's so much better to handle this in a wrapper, we should do so even if it means a little bit of pain for some use cases. Note that I'm not saying other use cases should be harder: it's just that they probably need to be enabled in a different way. One observation is that developing the alternative way should be much easier, since it will only involve a rebuild of the compiler wrapper, and not the compiler. Another observation is that if you do this right, you never have to rebuild clang, because you can simply use one which is already a cache hit. The effect is that you can get to cross compiling your desired workload much faster. See for example a case where I eliminated a rebuild of clang, enabling reuse of the one in cache.nixos.org: #351685 There has also been some discussion about tweaking |
|
I agree that the method in this PR maybe came to a deadend. I found it even more difficult to eliminate the cyclic dependency in the darwin toolchain. That is also why this PR has been stagnating for so long. The initial idea is to make clang work the same way as gcc, which is exactly how this PR (not) works. Modifying cc-wrapper might be more feasible. But the idea of modifying cc-wrapper also urges us to rethink whether gcc is doing in the right way. Should we also change the way gcc works? Anyway I do not think that rebuilding clang is a major problem because both modifying clang and modifying cc-wrapper will trigger a mass rebuild in nixpkgs. |
|
gcc, unlike clang, is not a cross compiler by default. So it's already rebuilding if you need a different target. I don't see the sense of trying to make these have an equivalent implementation. By "Avoiding clang rebuilds", I don't mean over time, I mean, between targets. So, if I want to cross compile to an exotic target in an exotic configuration, if you have things like this in the description of the clang build, it is necessary to rebuild clang before you even start. If you just have to rebuild a clang wrapper, it is instant. So the user experience for building for their exotic target is substantially better, and it means they can use an existing compiler build from cache.nixos.org. And it's not just the rebuild cost, but it's also the storage cost. If I'm building for multiple targets, I'd much rather have one clang build than many lying around. |
stale
bot
removed
the
2.status: stale
|
Darwin should be a lot easier to handle since #346043 was merged, but I would also advocate for rejecting this PR. As part of that work, I made Darwin mostly build LLVM using the LLVM bootstrap. Once @emilazy’s and @paparodeo’s work to increase the minimum SDK and update Darwin to clang 19 lands, I plan to finish that work on the LLVM bootstrap. Darwin will build LLVM using LLVM bintools including the linker (LLD). Having to build clang again to use ld64 for the rest of the Darwin bootstrap doesn’t make sense. One change I would support (because it affects Darwin cross-compilation) is making clang try |
When using clang for cross compilation, clang cannot find the linker.
Example (works for clang 12 and later):
Let me explain why this happens. For consistency with gcc, given a linker name (
ld), clang finds the linker in the following order:-Bcommand line args.The step 2 and 3 considers all possible target triple prefixes, but the step 1 does not. In nixpkgs, all binaries in cross toolchains are prefixed by the target triple, thus clang cannot find them.
Related LLVM source: https://github.com/llvm/llvm-project/blob/8dfdcc7b7bf66834a761bd8de445840ef68e4d1a/clang/lib/Driver/Driver.cpp#L5857-L5897
Related LLVM commit: llvm/llvm-project@3452a0d
Related gcc doc: https://gcc.gnu.org/onlinedocs/gccint/Collect2.html
For gcc in nixpkgs, the linker path is hardcoded in configuration phase (as is shown below). This commit copies this behavior to clang.
nixpkgs/pkgs/development/compilers/gcc/common/configure-flags.nix
Line 38 in 5c5ca01
Description of changes
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)