Conversation
|
Does LLVM_TARGETS_TO_BUILD not support the target triple? |
|
No. This is about CPU architectures not operating systems. |
dtzWill
left a comment
There was a problem hiding this comment.
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.
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.
Shoudn't LLVM_TARGETS_TO_BUILD default to all? Not sure what the rust error is though:
There was a problem hiding this comment.
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.
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.
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.
@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
sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)nix path-info -Sbefore and after)