Promote linux aarch64 to tier 1 support #13737
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
With native linux aarch64 runners available from github now available [1] we can run tests on every PR on the platform and promote it to tier 1 support [2]. This commit adds a github action test job that runs on PRs to test that the platform works on every commit. Then the wheel job is promoted to the tier 1 job which includes PGO which will improve the performance on the wheels we publish for the platform. While there is a larger effort to migrate our entire CI to github actions in Qiskit#13474 expanding our test matrix prior to that won't cause a conflict as we can easily expand the test matrix in that PR if this merges first. [1]: https://github.blog/changelog/2025-01-16-linux-arm64-hosted-runners-now-available-for-free-in-public-repositories-public-preview/ [2]: https://docs.quantum.ibm.com/guides/install-qiskit#operating-system-support
mtreinish
added
type: qa
|
One or more of the following people are relevant to this code:
|
Pull Request Test Coverage Report for Build 13474441452Details
💛 - Coveralls |
This reverts commit d39b6ee.
The aarch64 wheel builds are failing with PGO on latest stable (and stable - 1). This is likely either an issue in rustc or the llvm tools component from rustup on the later releases. But it seems to build fine with our MSRV. We build with latest stable for release jobs typically to enable the improvements from the latest compiler for what we publish. But PGO has enough benefit that using our MSRV is fine if that is what's necessary.
jakelishman
reviewed
Feb 22, 2025
There was a problem hiding this comment.
Thanks for the giant slog getting this to a point where the PGO finally passes. I'm excited to get AArch64 up to Tier 1.
edit: I posted this review pretty immediately in excitement when I saw the PGO pass - sorry if I jumped the gun and you hadn't had time to do any tidy up you were planning.
|
This should be ready to go now. For those following along, the issue seems to be a build issue in the llvm-tools libraries bundled with rustup on aarch64 linux in both 1.84.x and 1.85. It is trying to find non-portable symbols when linking the libraries used for PGO. The MSRV version works fine so for the time being this PR opts to use that. I'm working on bisecting the failure a bit more locally on an rpi 5 (which is quite slow for repeated compilations) but plan to report it upstream once I get a few more details. |
jakelishman
approved these changes
Feb 24, 2025
There was a problem hiding this comment.
Thanks for all the work - this looks good to me to merge now. I'll approve, but we need to take care with the branch protection rules. We either need to:
- temporarily disable the
macOS-arm64-tests-Python-3.*requirements for branch protection until this has merged, then enable protection on the four new test names - immediately switch the protection rules over, hold the queue so this merges clean first, then update pending PRs so they get the new test runs
I need to leave now, so I'll leave it up to you to do whichever you want.
Comment on lines
+103
to
116
| if [[ `uname -m` == "aarch64" ]] ; then | ||
| INSTALL_RUST_PATH=tools/install_rust_msrv.sh | ||
| RUST_TOOLCHAIN=1.79 | ||
| else | ||
| INSTALL_RUST_PATH=tools/install_rust.sh | ||
| RUST_TOOLCHAIN=stable | ||
| fi | ||
| cat >>"$GITHUB_ENV" <<EOF | ||
| CIBW_BEFORE_ALL_LINUX=yum install -y wget && {package}/$INSTALL_RUST_PATH | ||
| CIBW_BEFORE_BUILD=bash ./tools/build_pgo.sh $PGO_WORK_DIR $PGO_OUT_PATH | ||
| CIBW_ENVIRONMENT=RUSTUP_TOOLCHAIN=stable RUSTFLAGS='-Cprofile-use=$PGO_OUT_PATH -Cllvm-args=-pgo-warn-missing-function' | ||
| CIBW_ENVIRONMENT_MACOS=MACOSX_DEPLOYMENT_TARGET='10.12' RUSTUP_TOOLCHAIN=stable RUSTFLAGS='-Cprofile-use=$PGO_OUT_PATH -Cllvm-args=-pgo-warn-missing-function' | ||
| CIBW_ENVIRONMENT_LINUX=RUSTUP_TOOLCHAIN=stable RUSTFLAGS='-Cprofile-use=$PGO_OUT_PATH -Cllvm-args=-pgo-warn-missing-function' PATH="\$PATH:\$HOME/.cargo/bin" CARGO_NET_GIT_FETCH_WITH_CLI="true" | ||
| CIBW_ENVIRONMENT_LINUX=RUSTUP_TOOLCHAIN=$RUST_TOOLCHAIN RUSTFLAGS='-Cprofile-use=$PGO_OUT_PATH -Cllvm-args=-pgo-warn-missing-function' PATH="\$PATH:\$HOME/.cargo/bin" CARGO_NET_GIT_FETCH_WITH_CLI="true" | ||
| EOF |
There was a problem hiding this comment.
This is really nice - the logic's super clear to me now.
Comment on lines
+16
to
+17
| name: ${{ matrix.os }}-arm64-tests-Python-${{ matrix.python-version }} | ||
| runs-on: ${{ matrix.os }} |
There was a problem hiding this comment.
We branch-protect on these names - we'll need to update them in the protection settings before this can merge.
mtreinish
added a commit
to mtreinish/qiskit-core
that referenced
this pull request
Mar 13, 2025
This commit attempts to build the aarch64 linux wheels with rust 1.83. When we made aarch64 linux a tier 1 supported platform Qiskit#13737 there were packaging issues with the official distrubtions for Rust 1.85 and 1.84 on the platform which were preventing PGO builds from succeeding. At the time we we determined that our MSRV of 1.79 was able to build successfully. However, during the 2.0.0rc1 pre-release the wheel build job failed because of reported LLVM bug when combining LTO and PGO which we do for the release jobs. Local debugging determined these errors should be been fixed if built with Rust 1.85. Since we know we're not able to build with Rust 1.85 or 1.84 this commit tries building with 1.83 to see if this can work. If this does not work, follow up commits will walk backwards to see if 1.82, 1.81, or 1.80 can build successfully with LTO+PGO. Related to: Qiskit#13983
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
With native linux aarch64 runners available from github now available 1 we can run tests on every PR on the platform and promote it to tier 1 support 2. This commit adds a github action test job that runs on PRs to test that the platform works on every commit. Then the wheel job is promoted to the tier 1 job which includes PGO which will improve the performance on the wheels we publish for the platform.
While there is a larger effort to migrate our entire CI to github actions in #13474 expanding our test matrix prior to that won't cause a conflict as we can easily expand the test matrix in that PR if this merges first.
Details and comments