New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
llvm: fix cross-compilation #51226
llvm: fix cross-compilation #51226
Conversation
Does LLVM_TARGETS_TO_BUILD not support the target triple? |
No. This is about CPU architectures not operating systems. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll try to get to the other bits soon, but for now a stylistic suggestion :).
else if platform.parsed.cpu.family == "mips" then | ||
"Mips" | ||
else | ||
throw "Unsupported system"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps a "switch"-style instead here?
{
mips = "Mips";
x86 = "X86";
...
}."${platform.parsed.cpu.family}" or throw "Unsupported system"
(or thereabouts)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shoudn't LLVM_TARGETS_TO_BUILD default to all? Not sure what the rust error is though:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it does then yes we can remove any mention of target and share llvms. Just compiler-rt would be rebuilt.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we be concerned about output size/build times with this? LLVM_TARGETS_TO_BUILD is probably being overriden from the default right now when TARGET_TRIPLE is set. I kind of like that GCC's target-specific stuff doesn't give you anything for free (you have to set depsBuildBuild, nativeBuildInputs where appropriate). We might also still need DEFAULT_TARGET_TRIPLE.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not do that because libllvm is also used for jit compiling and therefore a dependency of some applications. Do you think building all backends would have many users? Most of the time I think one picks one host platform to compile for and this is also how our cross-buildsystem is designed. So it won't be useful for many people to have all backends.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dtzWill the switch case style does not work here because of I have to use platform.parsed.cpu.name
instead of family for aarch64.
Actually, you probably won't need this is you use |
@Ericson2314 should we change "family" to use the capitalized version? Since we use LLVM as a basis for |
@matthewbauer if you look at http://llvm.org/doxygen/classllvm_1_1Triple.html it does use the lower case names for those. I'm fine with changing it, but just want to be clear that LLVM may not consistently be upper-case about this either. |
There is one test case failing on older llvm versions: https://gist.github.com/Mic92/df4ae94fdb5071cfbe660b1c753e9b20 If someone is interested in fixing this, have a look at my old branch: https://github.com/Mic92/nixpkgs/tree/llvm-cross-targets-all |
0dfd525
to
2ca39fe
Compare
This broke mesa_noglu, see #52772 |
Motivation for this change
We build for both the host and target platform because we need this for rust.
I have not yet checked the build for all llvm versions.
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)