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

ci: Move dist-aarch64-linux to an aarch64 runner #135262

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mrkajetanp
Copy link
Contributor

Move the dist-aarch64-linux CI job to an aarch64 runner instead of cross-compiling it from an x86 one. This will make it possible to perform optimisations such as LTO, PGO and BOLT later on.

r? @Kobzol

Reland of #133809 now that the higher page sizes are fixed.

try-job: dist-aarch64-linux
try-job: dist-x86_64-linux
try-job: dist-i686-linux

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Jan 8, 2025
@mrkajetanp mrkajetanp force-pushed the ci-aarch64-dist-reland branch from 528f263 to 7d9903c Compare January 8, 2025 17:51
@mrkajetanp
Copy link
Contributor Author

Same as before, plus a couple small changes to account for the LTO+clang workarounds that were merged in the meantime

@tgross35
Copy link
Contributor

tgross35 commented Jan 9, 2025

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 9, 2025
…=<try>

ci: Move dist-aarch64-linux to an aarch64 runner

Move the dist-aarch64-linux CI job to an aarch64 runner instead of cross-compiling it from an x86 one. This will make it possible to perform optimisations such as LTO, PGO and BOLT later on.

r? `@Kobzol`

Reland of rust-lang#133809 now that the higher page sizes are fixed.

try-job: dist-aarch64-linux
try-job: dist-x86_64-linux
try-job: dist-i686-linux
@bors
Copy link
Contributor

bors commented Jan 9, 2025

⌛ Trying commit 7d9903c with merge a8e7b21...

@bors
Copy link
Contributor

bors commented Jan 9, 2025

💥 Test timed out

@mrkajetanp
Copy link
Contributor Author

Do we need a longer timeout here again?

@Kobzol
Copy link
Contributor

Kobzol commented Jan 9, 2025

We probably turned that one to a free runner recently-ish, so it times out now. With the cache primed, it should be fine.

@bors try

@bors
Copy link
Contributor

bors commented Jan 9, 2025

⌛ Trying commit 7d9903c with merge 4c7352d...

bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 9, 2025
…=<try>

ci: Move dist-aarch64-linux to an aarch64 runner

Move the dist-aarch64-linux CI job to an aarch64 runner instead of cross-compiling it from an x86 one. This will make it possible to perform optimisations such as LTO, PGO and BOLT later on.

r? `@Kobzol`

Reland of rust-lang#133809 now that the higher page sizes are fixed.

try-job: dist-aarch64-linux
try-job: dist-x86_64-linux
try-job: dist-i686-linux
@bors
Copy link
Contributor

bors commented Jan 9, 2025

☔ The latest upstream changes (presumably #135286) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Jan 9, 2025

💥 Test timed out

@mrkajetanp
Copy link
Contributor Author

mrkajetanp commented Jan 9, 2025

Ahh I see, I think we need to apply the same fix as in #134690 to i686 as well.

But then it already seems like it was tested there when that PR was merged? build-clang.sh is failing with some errors mentioning Gold and that's causing the timeout in any case.

Copy link
Contributor

Choose a reason for hiding this comment

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

There is the path src/ci/docker/host-*/disabled for jobs that are still useful but don't run in CI. I'm not sure what the policy is, but maybe it would be good to move this there instead of removing it? I would find it occasionally useful for verifying aarch64 builds on x86 machines. (non-dist would be better yet, but leaving the dist job around still seems convenient).

Copy link
Contributor

@Kobzol Kobzol Jan 10, 2025

Choose a reason for hiding this comment

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

Jobs in the disabled directory rot fast, and they can be restored from git history, so I personally don't see a lot of value in this. You can always copy-paste the contents locally to test it.

-DLLVM_INCLUDE_BENCHMARKS=OFF \
-DLLVM_INCLUDE_TESTS=OFF \
-DLLVM_INCLUDE_EXAMPLES=OFF \
-DLLVM_ENABLE_PROJECTS="clang;lld;compiler-rt;bolt" \
-DLLVM_BINUTILS_INCDIR="/rustroot/lib/gcc/x86_64-pc-linux-gnu/$GCC_VERSION/plugin/include/" \
-DLLVM_BINUTILS_INCDIR="/rustroot/lib/gcc/$GCC_BUILD_TARGET/$GCC_VERSION/plugin/include/" \
Copy link
Contributor

@Kobzol Kobzol Jan 10, 2025

Choose a reason for hiding this comment

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

It seems to me that this is the only change that happened to the 32-bit runner. I think that the problem is that only the 64-bit version of binutils is installed in the CentOS image:

2025-01-09T14:25:36.9324939Z #13 5.216 ---> Package binutils.x86_64 0:2.27-44.base.el7 will be updated
2025-01-09T14:25:36.9325811Z #13 5.217 ---> Package binutils.x86_64 0:2.27-44.base.el7_9.1 will be an update

So either this should be reverted (keep it hardcoded for 64-bit), or the 32-bit image should install also the 32-bit version of binutils.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm fairly sure the problem is here

-DLLVM_BINUTILS_INCDIR="/rustroot/lib/gcc/$GCC_BUILD_TARGET/$GCC_VERSION/plugin/include/" \

Because GCC_BUILD_TARGET will be i686 for the i686 job but the plugin/include will end up under x86_64 because that's where the job is running. I'm running a local build to confirm atm

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that the cause is necessarily that the host is x64, rather than just the binutils package is installed only in the x64 version, not the i686 version. So downloading a binutils.i686 CentOS package should hopefully fix it. If not, I would just hardcode the previous x64 path.

@mrkajetanp mrkajetanp force-pushed the ci-aarch64-dist-reland branch from 7d9903c to 1ee0062 Compare January 10, 2025 13:59
@rustbot
Copy link
Collaborator

rustbot commented Jan 10, 2025

Some changes occurred in src/tools/cargo

cc @ehuss

@mrkajetanp
Copy link
Contributor Author

ah sorry about that

Move the dist-aarch64-linux CI job to an aarch64 runner instead of
cross-compiling it from an x86 one. This will make it possible to
perform optimisations such as LTO, PGO and BOLT later on.
Move shared helper scripts used by Docker builds under docker/scripts.
@mrkajetanp mrkajetanp force-pushed the ci-aarch64-dist-reland branch from 1ee0062 to 307480a Compare January 10, 2025 14:13
@Kobzol
Copy link
Contributor

Kobzol commented Jan 10, 2025

It seems like you did both, i.e. install 32-bit binutils, but use the x64 path? :D

Anyway, let's see if it helped.

@bors try

@bors
Copy link
Contributor

bors commented Jan 10, 2025

⌛ Trying commit 307480a with merge c2013d90324bd57e903ea300e181b37f130d4b74...

bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 10, 2025
…=<try>

ci: Move dist-aarch64-linux to an aarch64 runner

Move the dist-aarch64-linux CI job to an aarch64 runner instead of cross-compiling it from an x86 one. This will make it possible to perform optimisations such as LTO, PGO and BOLT later on.

r? `@Kobzol`

Reland of rust-lang#133809 now that the higher page sizes are fixed.

try-job: dist-aarch64-linux
try-job: dist-x86_64-linux
try-job: dist-i686-linux
@mrkajetanp
Copy link
Contributor Author

mrkajetanp commented Jan 10, 2025

Well I figured the builds take so long to run that I might as well try the more conservative approach :) I think if it needed the i686 binutils it probably would have failed sooner, but it failing on the linking stage of building clang suggests that it's the linker plugin include that was messed up

@bors
Copy link
Contributor

bors commented Jan 10, 2025

☀️ Try build successful - checks-actions
Build commit: c2013d9 (c2013d90324bd57e903ea300e181b37f130d4b74)

@Kobzol
Copy link
Contributor

Kobzol commented Jan 10, 2025

@bors try

(To see cached durations).

bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 10, 2025
…=<try>

ci: Move dist-aarch64-linux to an aarch64 runner

Move the dist-aarch64-linux CI job to an aarch64 runner instead of cross-compiling it from an x86 one. This will make it possible to perform optimisations such as LTO, PGO and BOLT later on.

r? `@Kobzol`

Reland of rust-lang#133809 now that the higher page sizes are fixed.

try-job: dist-aarch64-linux
try-job: dist-x86_64-linux
try-job: dist-i686-linux
@bors
Copy link
Contributor

bors commented Jan 10, 2025

⌛ Trying commit 307480a with merge 291f58e...

@bors
Copy link
Contributor

bors commented Jan 10, 2025

☀️ Try build successful - checks-actions
Build commit: 291f58e (291f58ebd9118530a988bc921530bd05628562ce)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants