Skip to content
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

Enable native support for Windows on arm64 #14340

Closed
wants to merge 8 commits into from

Conversation

niyas-sait
Copy link
Contributor

@niyas-sait niyas-sait commented Nov 27, 2021

This PR will enable cross-compilation of Bazel binaries for win/arm64 from win/x64

bazel build -c opt --cpu=x64_arm64_windows //src:bazel.exe

Generated bazel executable can be used for native compilation in win/arm64

bazel.exe build //main:hello-world

Following changes are included

  • Add win/arm64 JDK 17
  • Fix AutoCpuConverter.java to identify win/arm64 CPU
  • Extend build_bazel_binaries.yml to cross-compile for win/arm64
  • Fix msvc toolchain to look for tools in HostX86 directory as well
  • add clang-cl support for windows/arm64 host
  • Extend host_windows config to handle windows x64 and arm64 hosts.

@google-cla google-cla bot added the cla: no label Nov 27, 2021
@niyas-sait
Copy link
Contributor Author

I am not sure how the CI failures are related to this patch. I've tried to run the test locally on top of master and the tests are failing there too.

@philwo
Copy link
Member

philwo commented Dec 13, 2021

Sorry for the delay @nsait-linaro. I just saw your e-mail on bazel-dev@!

@meteorcloudy @mai93 Could you have a look at this PR when you have time? It will be very hard for us to test as we don't have any Windows arm64 machines either personally nor on CI, but it looks pretty exciting and not too complex!

@nsait-linaro Could you try to upstream your change to gRPC and link the PR here? We can apply a patch in our local copy, but we generally like to avoid keeping these around for too long to reduce complexity on our side.

@philwo philwo added cla: yes and removed cla: no labels Dec 13, 2021
@philwo
Copy link
Member

philwo commented Dec 13, 2021

I manually flipped the CLA label to green, because signcla says we have a valid CLA.

@meteorcloudy
Copy link
Member

Auto-Configuration Error: 'PROCESSOR_ARCHITECTURE' environment variable is not set
Can you please fix this error in the tests?

@niyas-sait
Copy link
Contributor Author

Auto-Configuration Error: 'PROCESSOR_ARCHITECTURE' environment variable is not set Can you please fix this error in the tests?

Sure. I am on holiday now. I will have a look when I am back.

@niyas-sait
Copy link
Contributor Author

Thanks, @philwo, and @meteorcloudy for reviewing the patch.

I've identified a few more gaps while trying to build TensorFlow with Bazel for Windows on Arm.

  • def_parser binaries cannot handle arm64 objects yet
  • clang-cl toolchain support needs to be added (TensorFlow cannot be compiled with MSVC due to XNNPACK dependency having a lot of assembly code that cannot be compiled with MSVC)
  • host_windows bazel condition is hardcoded to x64 architecture and requires fixing to identify arm64 hosts.
  • is_msvc function always returns FALSE for win/arm64

I will push a few more commits to fix the above issues.

I've upstreamed one of the fixes for gRPC ( grpc/grpc#28164 ). I will need to upstream one more patch to enable gRPC Bazel to build configuration to add windows arm64 platform.

I would also like to ask your opinion on renaming the CPU name from x64_arm64_windows to arm64_windows. I think with native compilation x64 prefix doesn't seem relevant. Thoughts?

@meteorcloudy
Copy link
Member

@nsait-linaro Thanks for working on this, this is awesome!

def_parser binaries cannot handle arm64 objects yet

We probably need to sync with the upstream code in CMake:
https://github.com/Kitware/CMake/commits/master/Source/bindexplib.cxx
ARM64 support: Kitware/CMake@8950183

clang-cl toolchain support needs to be added (TensorFlow cannot be compiled with MSVC due to XNNPACK dependency having a lot of assembly code that cannot be compiled with MSVC)

Do you mean XNNPACK cannot be compiled with MSVC on ARM64, because at least the TF x64 build is working with MSVC, right? Also, we already support clang-cl in Bazel, see https://docs.bazel.build/versions/main/windows.html#build-c-with-clang. But I think last time I tried to build TF with clang-cl, I got some compilation errors.

I would also like to ask your opinion on renaming the CPU name from x64_arm64_windows to arm64_windows

Agree, we should rename it. FYI @mai93

@niyas-sait
Copy link
Contributor Author

@meteorcloudy and @philwo - Apologies for the delay. Finally, I managed to put together a more complete patch to enable windows on arm. Waiting for review!

@aiuto
Copy link
Contributor

aiuto commented Jan 31, 2022

This is not really my wheelhouse but I would feel more comfortable if this were two distinct PRs.

The first would add enough toolchain support so that bazel for x86_64 can cross compile for arm64.
If we have our core features right, that should require almost no Bazel changes. We might even be able to build for windows arm64 by cross compiling from linux.

The followup PR would add support to build bazel itself for windows arm.

@philwo
Copy link
Member

philwo commented Jan 31, 2022

Seems like a lot of tests are failing with this error now:

ERROR: /var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/ec321eb2cc2d0f8f91b676b6d4c66c29/sandbox/linux-sandbox/5933/execroot/io_bazel/_tmp/0f0d5afbc0f3af3b7e882cc5dbea196c/root/dbac4e5567f590e08c93c2d7d21a8944/external/bazel_tools/src/conditions/BUILD:145:27: configurable attribute "actual" in @bazel_tools//src/conditions:host_windows doesn't match this configuration. Would a default condition help?

Conditions checked:
@bazel_tools//src/conditions:host_windows_x64_constraint
@bazel_tools//src/conditions:host_windows_arm64_constraint

To see a condition's definition, run: bazel query --output=build .

This instance of @bazel_tools//src/conditions:host_windows has configuration identifier dafa3bb. To inspect its configuration, run: bazel config dafa3bb.

For more help, see https://docs.bazel.build/configurable-attributes.html#why-doesnt-my-select-choose-what-i-expect.

Could you please have a look? I think this is fine to merge once tests pass. It's basically just adding one more case to our existing multi-arch/platform support. FYI @meteorcloudy is out of office for two days, I think he'll review after he is back.

@niyas-sait
Copy link
Contributor Author

Seems like a lot of tests are failing with this error now:

ERROR: /var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/ec321eb2cc2d0f8f91b676b6d4c66c29/sandbox/linux-sandbox/5933/execroot/io_bazel/_tmp/0f0d5afbc0f3af3b7e882cc5dbea196c/root/dbac4e5567f590e08c93c2d7d21a8944/external/bazel_tools/src/conditions/BUILD:145:27: configurable attribute "actual" in @bazel_tools//src/conditions:host_windows doesn't match this configuration. Would a default condition help?
Conditions checked:
@bazel_tools//src/conditions:host_windows_x64_constraint
@bazel_tools//src/conditions:host_windows_arm64_constraint
To see a condition's definition, run: bazel query --output=build .
This instance of @bazel_tools//src/conditions:host_windows has configuration identifier dafa3bb. To inspect its configuration, run: bazel config dafa3bb.
For more help, see https://docs.bazel.build/configurable-attributes.html#why-doesnt-my-select-choose-what-i-expect.

Could you please have a look? I think this is fine to merge once tests pass. It's basically just adding one more case to our existing multi-arch/platform support. FYI @meteorcloudy is out of office for two days, I think he'll review after he is back.

Thanks @philwo. I've pushed a new commit to fix the above issue. How do I see the error log for the remaining failures?

@philwo
Copy link
Member

philwo commented Feb 1, 2022

Hi @nsait-linaro,

Thanks! To see the error logs, first click the Details link next to one of the failing Buildkite checks in GitHub:
Screenshot 2022-02-01 at 14 11 14

Then you can see the individual CI jobs and whether they passed or failed. If a test failed, there will be a test log in the Artifacts tab of the job:
Screenshot 2022-02-01 at 14 12 33

If it's a build failure, the error message will instead be in the main Bazel log shown for the job.

In this case, it seems like there's only a small tweak left to do in order to make everything pass:

//:srcs filegroup do not contains all the sources, missing:
tools/windows/BUILD
tools/windows/windows_config.bzl

You'll have to add your //tools/windows:srcs filegroup that you already created in your new BUILD file to this list: https://github.com/bazelbuild/bazel/blob/master/tools/BUILD#L4 :)

third_party/grpc/BUILD Outdated Show resolved Hide resolved
@@ -323,6 +323,9 @@ def _get_vc_env_vars(repository_ctx, vc_path, msvc_vars_x64, target_arch):
lib = msvc_vars_x64["%{msvc_env_lib_x64}"]
full_version = _get_vc_full_version(repository_ctx, vc_path)
tools_path = "%s\\Tools\\MSVC\\%s\\bin\\HostX64\\%s" % (vc_path, full_version, target_arch)
# For native windows on arm64 builds host toolchain runs in an emulated x86 environment
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, do you mean there is no C++ build tools that runs natively on Windows arm64 platform?

Also, can you make it a parameter instead of using "replace", I think that's safer.

tools_path = "%s\\Tools\\MSVC\\%s\\bin\\%s\\%s" % (vc_path, full_version, host_arch, target_arch)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, they all run in emulated mode now.

For Windows 11, x64 toolchain is available so I want to keep the logic of checking for x64 toolchain first and then fallback to x86 if x64 toolchain is not available.

tools/cpp/windows_cc_configure.bzl Show resolved Hide resolved
src/conditions/BUILD Show resolved Hide resolved
src/test/py/bazel/bazel_windows_cpp_test.py Outdated Show resolved Hide resolved
tools/cpp/BUILD.windows.tpl Outdated Show resolved Hide resolved
@niyas-sait
Copy link
Contributor Author

I think that's OK, but you'll have to rebase after we merge the first one.

Ok, First PR with third_party contents: #14689

I will rebase this patch once that is merged

@meteorcloudy
Copy link
Member

OK, #14689 is merged.

Following changes are included

- Fix AutoCpuConverter.java to identify win/arm64 CPU
- Add win/arm64 JDK 17
- Add grpc patch to workaround build issues
- Extend build_bazel_binaries.yml to cross-compile for win/arm64
- Fix msvc toolchain to look for tools in HostX86 directory as well

Fixes: bazelbuild#14339
WORKSPACE Outdated Show resolved Hide resolved
meteorcloudy added a commit to bazelbuild/continuous-integration that referenced this pull request Feb 2, 2022
Currently we are only using x64 machines to do cross-compile.
This is to support bazelbuild/bazel#14340
@meteorcloudy
Copy link
Member

BTW, can normal x64 binaries run on Windows arm64? Bazel pulls java_tools as an external dependency, which contains some x64 binaries. Can those binaries run seamlessly just like on Apple silicon? Eventually, we should publish java_tools for arm64 platforms.

@niyas-sait
Copy link
Contributor Author

BTW, can normal x64 binaries run on Windows arm64? Bazel pulls java_tools as an external dependency, which contains some x64 binaries. Can those binaries run seamlessly just like on Apple silicon? Eventually, we should publish java_tools for arm64 platforms.

Yes, Windows 11 supports x64 emulation so that should work.

meteorcloudy added a commit to bazelbuild/continuous-integration that referenced this pull request Feb 2, 2022
Currently we are only using x64 machines to do cross-compile.
This is to support bazelbuild/bazel#14340
@bazel-io bazel-io closed this in 0ba4caa Feb 2, 2022
@niyas-sait
Copy link
Contributor Author

Thanks, @meteorcloudy and @philwo for your help! It is merged now 🥳

I would like to check one of the bazel(arm64) builds generated by CI system. Does it produce any nightly builds I could try ? Or should I wait for the next release.

@meteorcloudy
Copy link
Member

meteorcloudy commented Feb 3, 2022

Thanks! It'll be great if you could help check if everything is working correctly on Windows ARM64 before we adding it to the official release.

We actually build Bazel at every commit, the binary should be available at https://storage.googleapis.com/bazel-builds/artifacts/windows_arm64/c2ddbd1954af5baab63b93f2b055a410a27832c8/bazel. I just enabled publishing binary for windows_arm64, you'll have to use latest commits to fetch the binary.

If you are interested, you could also try to make Bazelisk work on Windows arm64, then it'll be even easier to fetch Bazel binary at your desired version/commit.

@niyas-sait
Copy link
Contributor Author

Bazelisk

Thanks, @meteorcloudy. I will try the binary.

Yes, Bazelisk would be nice as well. I will have a look

@niyas-sait
Copy link
Contributor Author

Thanks! It'll be great if you could help check if everything is working correctly on Windows ARM64 before we adding it to the official release.

We actually build Bazel at every commit, the binary should be available at https://storage.googleapis.com/bazel-builds/artifacts/windows_arm64/c2ddbd1954af5baab63b93f2b055a410a27832c8/bazel. I just enabled publishing binary for windows_arm64, you'll have to use latest commits to fetch the binary.

If you are interested, you could also try to make Bazelisk work on Windows arm64, then it'll be even easier to fetch

Thanks, @meteorcloudy. I can confirm that Bazel binary is working as expected. Tried native and cross-compilation builds for arm64 and x64 windows targets with MSVC and clang and seems to work. Log attached for reference.

I think we need to also add exe suffix to the published binary. Now it is published as bazel and windows doesn't run it until I add an exe extension to it.

@meteorcloudy
Copy link
Member

That's awesome! Have you also tried to build Java binary?

I think we need to also add exe suffix to the published binary. Now it is published as bazel and windows doesn't run it until I add an exe extension to it.

Yes, you'll have to rename it, but it won't be a problem with Bazelisk, it will do it for you!

@niyas-sait
Copy link
Contributor Author

That's awesome! Have you also tried to build Java binary?

I think we need to also add exe suffix to the published binary. Now it is published as bazel and windows doesn't run it until I add an exe extension to it.

Yes, you'll have to rename it, but it won't be a problem with Bazelisk, it will do it for you!

@meteorcloudy , Unfortunately, the java build didn't work due to missing JDK configurations. I've created #14700 which should fix the java builds now. Can you please review ?

niyas-sait added a commit to niyas-sait/bazel that referenced this pull request Feb 11, 2022
This PR will enable cross-compilation of Bazel binaries for win/arm64 from win/x64

`bazel build -c opt --cpu=x64_arm64_windows //src:bazel.exe`

Generated bazel executable can be used for native compilation in win/arm64

`bazel.exe build //main:hello-world`

Following changes are included

- Add win/arm64 JDK 17
- Fix AutoCpuConverter.java to identify win/arm64 CPU
- Extend build_bazel_binaries.yml to cross-compile for win/arm64
- Fix msvc toolchain to look for tools in HostX86 directory as well
- add clang-cl support for windows/arm64 host
- Extend host_windows config to handle windows x64 and arm64 hosts.

Closes bazelbuild#14340.

PiperOrigin-RevId: 425919351
meteorcloudy pushed a commit that referenced this pull request Feb 14, 2022
* Enable native support for Windows on arm64 (Part 1)

Contains following changes to third_party:

 - Extended def_parser to handle ARM64 binaries
 - Add grpc patch to workaround build issues

Closes: #14689

Partial commit for third_party/*, see #14689.

Signed-off-by: Yun Peng <pcloudy@google.com>

* Enable native support for Windows on arm64

This PR will enable cross-compilation of Bazel binaries for win/arm64 from win/x64

`bazel build -c opt --cpu=x64_arm64_windows //src:bazel.exe`

Generated bazel executable can be used for native compilation in win/arm64

`bazel.exe build //main:hello-world`

Following changes are included

- Add win/arm64 JDK 17
- Fix AutoCpuConverter.java to identify win/arm64 CPU
- Extend build_bazel_binaries.yml to cross-compile for win/arm64
- Fix msvc toolchain to look for tools in HostX86 directory as well
- add clang-cl support for windows/arm64 host
- Extend host_windows config to handle windows x64 and arm64 hosts.

Closes #14340.

PiperOrigin-RevId: 425919351

* [windows/arm64] Add missing JDK toolchain for java build

Extend configurations to add JDK 11 and 17 for windows/arm64 platforms.

This should fix the Java builds on windows/arm64

Closes #14700.

PiperOrigin-RevId: 427737536

* add missing openjdk11_windows_arm64_archive

* Deduplicate urls parsed to reduce crawl requests

#14700 added couple more URLs to fetch JDK package and seems to be causing some infrastructure as discussed in #14700.

This patch workaround the issue by removing the duplicated URLs and reduce the crawl request.

Closes #14763.

PiperOrigin-RevId: 427464876

* fix jdk_http_archives.tmpl
fmeum pushed a commit to fmeum/continuous-integration that referenced this pull request Dec 10, 2023
Currently we are only using x64 machines to do cross-compile.
This is to support bazelbuild/bazel#14340
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants