-
-
Notifications
You must be signed in to change notification settings - Fork 14.9k
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 compiling for v4, v5, v6. #40604
Conversation
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.
Quick thoughts, but maybe we should go in a different direction....
doCheck = stdenv.isLinux && (!stdenv.isi686); | ||
doCheck = stdenv.isLinux | ||
&& (!stdenv.isi686) | ||
&& (stdenv.buildPlatform == stdenv.hostPlatform); |
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 think this is handled by default/already? Please LMK if that's not what you're seeing!
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.
Huh, you're right, thanks!
] | ||
++ stdenv.lib.optionals (stdenv.buildPlatform != stdenv.hostPlatform) [ | ||
"-DCMAKE_CROSSCOMPILING=True" | ||
"-DLLVM_TABLEGEN=${hostLLVM}/bin/llvm-tblgen" |
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.
Hmm this is the way forward given how we use cmake for cross--which unfortunately is "wrong" and means we aren't taken advantage of any of LLVM / cmake's infrastructure for these things.
More outside review ....
The "right" cmake way to go is to use a toolchain file and configure multiple times, or something. Anyway I'm not sure if that's "worth" the effort to rework-- or perhaps not something I can "own" through to completion so I don't want to propose it. Here's a messy branch that tries to do things "properly", and successfully configures LLVM so that tablegen is built "native" automagically, with deps and such. Be warned this is a POC but hopefully demonstrates the idea enough for us to investigate going this way--as it's what cmake and LLVM both encourage folks to do. And maybe when we're done we can upstream some documentation about what we learned :P. Here's a tagged version: This generates a basic toolchain file for the cross-compilation, although I believe that might not be needed... I just started with that since it's what cmake pushes for this :D. Alright I'm a bit burned on this, sorry, I realize this isn't the best explanation. But hopefully will be wonderful once cleaned up and such. FWIW clang has some support for the same, although not 100% how well-supported it'll be combinng building clang separately AND cross-compiling. As an aside, in my cmake readings it appears it has some automagical goodness for taking advantage of clang's multi-target abilities -- which might be rather useful for us as part of #36867 and related. ... although what you have here I think works for LLVM at least. Apologies for dumping my POC on your PR O:). I mean well, we've long been talking about trying to do this more "properly". |
Bah, sorry. Nevermind me, I'll submit a new PR if I have anything. Sorry about the mess! :) |
5f94e7f
to
34941e9
Compare
@Ericson2314 started doing this in acc9843 it seems. |
Oh I hadn't seen this since before it was a PR I think. Turns out there's no duplicated work, however, as I've just been reformatting things while not breaking hashes for the most part. Also I've been thinking about (though it should help with both) build == host != target, as opposed to build != hsot == target. |
@@ -1,6 +1,6 @@ | |||
{ lowPrio, newScope, stdenv, targetPlatform, cmake, libstdcxxHook | |||
, libxml2, python2, isl, fetchurl, overrideCC, wrapCC, ccWrapperFun | |||
, darwin | |||
, darwin, hostLLVM |
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.
This should definitely be called buildLLVM.
Heya ! Is this PR still relevant ? If so - is there something we need to do to get this one merged (in addition to resolving conflicts) ? Thanks ! |
This has been fixed elsewhere. |
Motivation for this change
Allows LLVM versions 4, 5, and 6 to be cross-compiled by relying on
${buildPackages.llvmPackages.llvm}/bin/llvm-tblgen
when cross-compiling.Things done
build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)