Add support for Windows host and targets, ARM64 and x86_64 #642
Conversation
ArchangelX360
requested review from
dzbarsky,
fmeum,
helly25,
jsharpe and
rrbutani
as code owners
|
It's going to take me a bit of time to digest this. In the meantime, you might be curious to take a look at https://github.com/cerisier/toolchains_llvm_bootstrapped which also provides windows support out of the box with no config :) (At the very least, I'd appreciate you giving it a spin and reporting any bugs you find) |
ArchangelX360
force-pushed
the
archangelx360/llvm_windows_host_target
branch
from
a1d8eab to
b2f7dd3
Compare
|
Thank you for continuing the work I started! |
|
@dzbarsky Seems like the README must be updated, Windows support is still written under roadmap. Of course, despite that, I believe that repository should still have a decent Windows support for our beloved Windows developers wanting to build something 😄. |
|
Yes we will need to update README! We should support windows here as well, I will find some time to review this one soon |
|
@dzbarsky FYI: I commented on cerisier/toolchains_llvm_bootstrapped#24 and opened cerisier/toolchains_llvm_bootstrapped#141 which blocked me almost immediately. |
Deleting redundant keep only to be discussed.
dzbarsky
left a comment
dzbarsky
left a comment
There was a problem hiding this comment.
It seems like the changes are all behind windows conditionals so existing functionality is hopefully unaffected. From that POV I'm ok landing this with some disclaimers that it's experimental/incomplete/possibly-broken/etc.
It does seem like there are still some issues to work through and errors that aren't understood yet - do we want to get a better understanding of those before landing?
|
|
||
| # This filegroup should only have source directories, not individual files. | ||
| # We rely on this assumption in system_module_map.bzl. | ||
| filegroup( |
There was a problem hiding this comment.
@fmeum any FUD around source directories on windows?
| # not be available at runtime to allow sanitizers to work locally. | ||
| # Any library linked from the toolchain to be released should be linked statically. | ||
| "lib/clang/{LLVM_VERSION}/lib/**", | ||
| "lib/**/libc++*.a", |
There was a problem hiding this comment.
hmm are you sure about these? First of all I thought clang-cl would use .lib instead of .a, also I didn't think it uses llvm runtime libs?
There was a problem hiding this comment.
No indeed, libc++*.a have nothing to do here, I removed it.
For lib/clang/{LLVM_VERSION}/lib/**, we do have:
./lib/clang/20/lib/windows/clang_rt.builtins-aarch64.lib
./lib/clang/20/lib/windows/clang_rt.profile-aarch64.lib
but where they should be in lib and lib_legacy I'm not sure.
What is the exact use of it in the UNIX side? It seems to be something only for darwin, but I don't get exactly the use-case to arbiter whether it makes sense on Windows or not.
There was a problem hiding this comment.
I don't think it's darwin-only; this is the compiler-rt lib that provides the builtins (as well as as sanitizer libs, which are less relevant on Windows, although from a bit of research it does seem like ubsan at least might be workable on windows?)
As for lib/lib_legacy, those are mutually exclusive and which one is used depends on Bazel version, newer versions use non-legacy one which is more efficient. Since you're the only user right now if your Bazel is relatively recent you can probably just skip the legacy bits?
| # $ bazelisk.exe build //fleet/native/launcher:fleet # WORKS | ||
| # $ dir C:/temp/k7ozco62/external/toolchains_llvm++llvm+llvm_toolchain/bin # shows `-a----` file modes on binaries | ||
| # | ||
| use_absolute_paths_llvm = True |
There was a problem hiding this comment.
doesn't this break caching?
There was a problem hiding this comment.
It most probably does, I would need help investigated the original issue for which that mitigation is in place.
Bazel (or our repo rules, I don't know) does something quite odd.
Depending on the command that is run, (and in which order!) the symlinked files are changing.
I don't understand yet what is exactly happening, so I had to force use_absolute_paths_llvm to have something reliably working, for now.
It's most probably related to symlinks, as symlinks on Windows is particularly finicky, but I have no idea what is happening. Any lead or idea is welcomed, see the comment for the behaviour observed.
3 participants
The following PR adds Windows support for both usage on Windows host (ARM64 and x86_64) and to compile Windows targets (ARM64 and x86_64).
This work is based on the awesome work of both:
It has been tested internally at JetBrains (not in production yet) for our
rules_rust, we tested so far:aarch64-pc-windows-msvcfrom a Windows ARM64 host (Parallels VM) machine: ✅aarch64-pc-windows-msvcfrom a macOS aarch64 host machine (using a wired MSVC+SDK sysroot): ✅x86_64-pc-windows-msvcfrom a macOS aarch64 machine: 🔴 did not work, due to what appears to be Rust related and not toolchain relatedKnown issues
use_absolute_paths_llvmis forced toTruewhen using a Windows host as a mitigation to symlink issues (see comment in the PR's code)selectit does not really do what we need, that's why it's written as a separate file