-
Notifications
You must be signed in to change notification settings - Fork 723
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
Windows arm64 support (aarch64-pc-windows-msvc) #1167
Comments
For Aarch64 Windows targets, we need to review the Windows AArch64 ABI to see if it is the same as Linux. If so then we need to see if PerlAsm works for Aarch64 Windows assembly. If so then we should be golden. If so then presumably we need to use clang and not MSVC unless the MS toolchain includes a GCC-/clang- compatible assembly for AAarch64. Unless/until we can figure out all of that, we'd need to skip the assembly code completely. This means we'd need to finish merging some of the pending PRs for ring to have non-assembly fallbacks for all assembly code. I'll work on that. |
I wish I could help here but I'm afraid I cannot as I'm not in any way proficient in assembly code 😬 But maybe this can help: https://docs.microsoft.com/en-us/cpp/build/arm64-windows-abi-conventions As for the assembler, MSVC seems to include armasm64.exe which is somewhat described here: https://docs.microsoft.com/en-us/cpp/assembler/arm/arm-assembler-reference The documentation talks about armasm.exe but as far as I understand it, armasm64.exe is very similar. Yet again, I cannot say anything about what dialect of assembly language does it accept (if there is any difference for ARM64). I'm ready to run any tests on my Surface Pro X though so please let me know if there's anything I could help with 😃 (e.g. if you'd want to compile and run some assembly code with MSVC tools). |
Good news: BoringSSL recently merged the Windows ARM support, so this might now be much easier: https://boringssl.googlesource.com/boringssl/+/afd5dba756b6266fa99c11af6496b39d826769cd |
Hi @briansmith Would you please find some time to look into this? Thanks a lot! 🙂 |
@briansmith @Alovchin91 any update on this? I just tried building some of my Rust code for Windows ARM64, and ring v0.16.20 fails to build for me with the same error as this ticket. |
@briansmith I'm also blocked on this with Rustup unfortunately 😞 Do you have any idea of what should be done to enable Windows ARM64 compilation? Maybe me and @awakecoding could help you somehow? |
@Alovchin91 I've been working on porting 20+ native dependencies to Windows ARM64 for Devolutions, I can definitely help, but I know absolutely nothing about the ring internals and how it builds the part of code that fails. I can take a closer look later this week. |
I inspected the upstream boringssl changes, and some of them appear to have already been ported to the current sources. I decided to take a closer look at the build failure which is caused by the BN_ULLONG type not being defined while being used from mongomery.c: Most of the time build failures result from assumptions that there are just two platforms: 32-bit intel, and 64-bit intel. ARM64 is 64-bit so it goes in the "OPENSSL_64_BIT" ifdef, but it doesn't have the "BORINGSSL_HAS_UINT128" definition. I tried manually forcing it on, but that doesn't look like those types do not exist in MSVC: Is there a 128-bit integer type in MSVC? If not, how exactly do we disable the usage of BN_ULLONG, or it is mandatory? I tried the dumb thing of defining it to uint64_t but it results in a warning treated as a error due to excessive bit shifting leading to undefined behavior. Maybe @briansmith can provide some more clarity on this part of the code |
As far as I know, the answer is no they don't. But it can be implemented by using 2 64-bit long int like this: |
@awakecoding It should be rather simple to patch
But now there's Lines 37 to 53 in fe67830
|
I have a WIP branch here: https://github.com/briansmith/ring/compare/main...Alovchin91:alovchin91/win-arm64?expand=1 Please note that I have no idea what am I doing so it might be completely wrong 😆 Also I don't seem to be able to build the tests yet:
etc. so I don't think you can actually use it in any binary yet. @briansmith Would you kindly have a look at it and maybe point me in the direction where I could find a solution? 🙂 |
@Alovchin91 I made a branch in which I imported your patches, it does build, but the test executable won't link with the same unresolved externals. I have a feeling that maybe the assembly code is not getting correctly built - are the unresolved externals all assembly functions? One of the difficult things with Windows ARM64 porting is its weird assembler program (armasm64.exe). The Windows assembler program for Intel appears to be nasm, and the current configuration you've set for Windows ARM64 appears to imply the sources should contain precompiled object files. Is there a way to confirm if we're getting the object files at all for the missing symbols? |
Ok, so I digged a little deeper into the build system to figure out what it does - on Windows / intel it uses nasm (for some odd reason it doesn't seem to pick up nasm.exe from my path, but expects it to be inside the target/tools/windows/nasm directory). I don't know much about perlasm but my guess is it's one of those assembler format conversion tools to help adapt the same assembly code to different assemblers. I have no idea which assembler it calls, but it does produce something with the added Windows aarch64 configuration. The interesting part is the ring_core_generated directory that contains both a C header and an asm include file with all the exported symbols - those same missing symbols we don't have. I suspect maybe perlasm doesn't get the right combination of parameters, or it simply doesn't do the right thing, and we either don't end up with the right declarations, or we're not getting the symbols for the right architecture here and they get ignored. |
I think the current build system simply has no support for the Windows ARM64 assembler. If you set the RING_PREGENERATE_ASM environment variable to 1, it'll try pre-generating everything for the assembly files. I modified ASM_TARGETS to contain only Windows ARM64 so I could focus only on that one. The pre-generation fails because it tries calling nasm on the assembly files, which is obviously unsupported. At least now we have something to work with - try and modify the build system to make sure it does call a working assembler program for Windows ARM64 (armasm64.exe or clang.exe). |
Ok, armasm64.exe is basically a very obscure tool with a syntax I'm not sure a lot of people know about, so my attempts at calling it failed. However, I did manage to get ring built for Windows ARM64 using clang.exe as the assembler program! I have a bunch of unclean local changes, but this seems to be the way to go: |
@Alovchin91 ok if you want to give it a try, here's here to do it using my branch (https://github.com/awakecoding/ring/tree/windows-arm64): first, copy nasm.exe to target/tools/windows/nasm/nasm.exe in the ring source tree, that's for intel assembly. You will no longer get the unresolved symbols, but the test code will fail to build properly. If you have issues with pregeneration, just delete the "pregenerated" directory and it should re-generate correctly. Let me know if you get any further! |
@awakecoding Looks promising! I'll probably have time to look into it tomorrow 🙂 FYI I've managed to build my branch having Perl in the PATH (I've used Git for Windows bash under x64 emulation for this). |
Okay, I didn't look into your code yet @awakecoding but here's what I've found regarding armasm64.exe. First of all, it appears that armasm.exe and armasm64.exe are basically the same tool when speaking of MSVC. It has some scarce documentation but it appears to be enough because:
https://docs.microsoft.com/en-us/cpp/assembler/arm/arm-assembler-directives?view=msvc-160 Second, it looks like some manual setup is required to use armasm64.exe with tools like Cmake, but it shouldn't be an issue since we're using manually written build.rs anyway. So it looks like the first step would be to invoke armasm64.exe from build.rs instead of nasm.exe for Windows ARM. The latest version of cc (1.0.69 and up) should make finding it as easy as calling Here are examples of how .NET team does that (using Cmake; found here): Regarding the perlasm_format being "win64", I just copied it from the commit mentioned by @briansmith in January 🙂 |
@Alovchin91 my original attempt was to call armasm64.exe instead of clang, but even once I got the right command-line parameters, it just wouldn't accept pretty much the entire file with syntax errors and unrecognized stuff, while clang.exe took it instantly like a big boy. You can give it a shot, just do a search in the visual studio directories to find the executable and then manually add it to your path, and edit the code on my branch to call it instead. Just invoke armasm64.exe -h to see what parameters it takes. I recently updated my libvpx builds with ARM64 support and it does seem to be calling armasm64.exe - it has a rather simple perl script for adapting the syntax, so I guess it just have been reusing something else I haven't found. I would suggest going the clang.exe route at least for now to get everything built correctly. Once you go past the missing symbols, it fails to compile the actual test code, it may not require much but it still need to be done. There is also the possibility of improving support for the Windows assemblers - nasm is a bit tricky because it has to be copied over, but it would be trivial to add a detection function. I'm thinking we could support both clang.exe and armasm64.exe and select the one we want with an environment variable + a default. |
In fact, I noticed just now that the message of the commit that introduces ASM support for Windows on ARM in BoringSSL actually explicitly states that it expects Clang syntax:
How didn't I notice that before 🤦♂️ It's probably not worth the effort to support both, especially since it's mostly on BoringSSL side, but I guess we should leave this decision to @briansmith 🙂 I'll have a look at the test build failures when I get home. BTW my math in the previous commits is likely terrible and might not work 🙈 Also I'm not so sure about ignoring the 4133 warning. |
I've used your code @awakecoding
Okay, so the issue was with the .asm extension as opposed to .S. Wow. So the final two commits are: And that was it! Now tests also do build! And they also do succeed! 😱 Test results
Now I need to implement proper feature detection apparently which shouldn't be rocket science I guess: And I still don't believe in my math guys... |
Okay I'm so happy that it builds AND runs that I've opened a draft pull request: #1339 I've also invited you @awakecoding as a collaborator on that fork so if you feel like doing some fixes/additional magic, please feel free 🙂 Branch is Time to sleep now, it's 3am here... |
@Alovchin91 good job!!! I will try it out tomorrow! |
Okay, CPU feature detection is done 👍 @briansmith I've left a few comments in the PR #1339 for you to review, but other than that it looks to be... ready? 🤔 (I'm a bit afraid to say so tbh) |
@Alovchin91 there is still a bit of cleanup to be done, it won't build if you don't set $Env:RING_PREGENERATE_ASM='1' once before, and it's a bit annoying because I have to copy nasm.exe inside target/tools/windows/nasm/ every time. I haven't fully figured out how this works, but I believe maybe the assembly files are precompiled by default? The pregeneration does it for all targets, which is why it calls nasm even if I want to target Windows ARM64. |
@awakecoding Hmm, I don't have that issue but I'm building directly on Surface Pro X, maybe that's why. Did you run |
Closing this as completed with the ring 0.17.0 release. I was able to build and test both natively on ARM64 Windows and cross-compile from x86_64 Windows. PR #1691 clarifies the documentation. I also filed issue #882 for cc-rs to help make this easier so that modifying Thanks for all the help here! I really appreciate it. |
Thanks, @briansmith. I'm struggling to get this to work though. I added this to my Cargo.toml file: [patch.crates-io]
ring = "0.17.0" And I get this error:
Any idea why? |
Why not just put it in |
It isn't a direct dependency. Only an indirect one. Adding it to This works: [patch.crates-io]
ring = { git = "https://github.com/awakecoding/ring", branch = "0.16.20_alpha" } But as it breaks win-x64, I'm eager to adopt your 0.17.0 version that presumably is one version that works for all targets at once. I also tried a variant of this that consumes your repo. But since you have no 0.17.0 tag, or v0.17 branch, I just pointed it at main: [patch.crates-io]
ring = { git = "https://github.com/briansmith/ring", branch = "main" } But that oddly produces this error (where
|
The patch mechanism doesn't let you patch something using 0.16 with ring 0.17. Check the documentation for the patch mechanism to see its version matching requirements. |
Swapping 0.16 with 0.17 won't work even if you fake the version number and cross fingers - all individual dependencies need to be updated to use 0.17, which is why my patch branch just swaps v0.16.20 specifically, which most dependencies use today. To identify what needs updating, use cargo tree like this:
In some other projects, ring gets referenced in a lot more dependencies:
Every single one of those package references needs to be updated to 0.17, otherwise you'll end up with a ring 0.16.20 build that needs patching to build for Windows on ARM. Maybe use my patch branch with a condition in your CI environment if it doesn't build properly for win-x64? It works for both win-x64 and win-arm64 for me in sspi-rs. |
I've had to use terrible hacks like maintain two cargo.toml files and swap then in based on whether I'm building win-arm64 or any other target. |
indeed, but if you inject the patch line in the CI environment, you can add a condition to only modify the Cargo.toml file for win-arm64, which should at least unblock the issue for now until all dependencies are updated to use 0.17 |
I need to be able to do this conveniently from my local machine, which depending on the machine is x64 or arm64. So the trick the CI plays has to be easily reproducible and maintainable locally. |
Upgrades `aws-config` because `aws-config@0.56` has a dependency on `ring@0.16` [^1] which does not support `aarch64-pc-windows` [^2]. > error: failed to run custom build command for `ring v0.16.20`. Support for `aarch64-pc-windows` was added in `ring@0.17` [^3] which `aws-config@0.57` uses [^4]. Unfortunately, there's a hideous amount of changes between these two version, although no breaking changes are mentioned in the changelog [^5]. Does a maintainer here have a suggestion of what kind of testing we might need to do? [^1]: https://github.com/awslabs/aws-sdk-rust/blob/d212f8dd428e140b8ff414261f6921b2901a7dbc/sdk/aws-config/Cargo.toml#L98-L100 [^2]: briansmith/ring#1514 [^3]: briansmith/ring#1167 (comment) [^4]: https://github.com/awslabs/aws-sdk-rust/blob/511abe92a476c27d1e49758fe54eac0886ecf731/sdk/aws-config/Cargo.toml#L98-L100 [^5]: https://github.com/awslabs/aws-sdk-rust/compare/d212f8d..511abe9
Hi,
I'm trying to compile reqwest with rustls support on Windows arm64 and I'm getting the following error:
I think it's somewhat related to #960 though in this case I'm working on aarch64-pc-windows-msvc platform which is not UWP.
This is currently blocking rust-lang/rustup#2612 as rustup is using reqwest's rustls feature.
Please note that unfortunately there is no hosted CI/CD to run tests on Windows arm64 platform yet, so this would be cross-compile only for now.
The text was updated successfully, but these errors were encountered: