Skip to content

bootstrap: add bootstrap step to run intrinsic-test in CI#156356

Open
xonx4l wants to merge 17 commits into
rust-lang:mainfrom
xonx4l:port-intrinsic-test
Open

bootstrap: add bootstrap step to run intrinsic-test in CI#156356
xonx4l wants to merge 17 commits into
rust-lang:mainfrom
xonx4l:port-intrinsic-test

Conversation

@xonx4l
Copy link
Copy Markdown
Contributor

@xonx4l xonx4l commented May 9, 2026

View all comments

This PR ports library/stdarch/crates/intrinsic-test crate into the bootstrap test runner as a step, so that we can run the intrinsic-test suite via x.py test.

Changes -:
Ports intrinsic-test from the stdarch crate into the bootstrap system as a new test step so it runs inside the existing
x86_64-gnu and aarch64-gnu CI jobs .
Added intrinsic-test in src/bootstrap/src/core/build_steps/test.rs .
Registers intrinsic-test as a bootstrap tool in tool.rs .
Installs clang, lld, and Intel SDE in the respective Dockerfiles.

r? @Kobzol

@rustbot rustbot added A-CI Area: Our Github Actions CI 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-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels May 9, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 9, 2026

Kobzol is not on the review rotation at the moment.
They may take a while to respond.

@rustbot rustbot added the T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. label May 9, 2026
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

Comment on lines +916 to +918
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
run.path("library/stdarch/crates/intrinsic-test")
}
Copy link
Copy Markdown
Contributor

@folkertdev folkertdev May 10, 2026

Choose a reason for hiding this comment

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

the intrinsic tests should also run when any of the source in core_arch changes (unless I'm misinterpreting what this function does).

View changes since the review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No you are not misinterpreting :) . You're so right I have added the core_arch path so it triggers . Thanks for the review.

Comment on lines +27 to +30
RUN curl -L http://ci-mirrors.rust-lang.org/sde-external-10.8.0-2026-03-15-lin.tar.xz -o /tmp/sde.tar.xz \
&& mkdir -p /intel-sde \
&& tar -xJf /tmp/sde.tar.xz --strip-components=1 -C /intel-sde \
&& rm /tmp/sde.tar.xz
Copy link
Copy Markdown
Contributor

@folkertdev folkertdev May 10, 2026

Choose a reason for hiding this comment

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

this doesn't need an instant fix, but we bump the version of this tool occasionally, so ideally we'd sync that version between the repositories.

View changes since the review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok, noted.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

Copy link
Copy Markdown
Member

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

Thank you!

I wonder what to do about the required dependencies. GCC/Clang is fine, but the SDE emulator seems problematic. We have a lot of CI jobs that will run the intrinsics test and I don't think it is great if we install it on all of them. Of course it is also not great if it is required when running tests locally.

I think that we should make this test be non-default for now, and run it explicitly on selected CI jobs.

View changes since this review

Comment thread src/bootstrap/src/core/build_steps/test.rs
Comment thread src/bootstrap/src/core/build_steps/test.rs Outdated
Comment thread src/bootstrap/src/core/build_steps/test.rs Outdated

let crates_link = out_dir.join("crates");
if !crates_link.exists() {
std::os::unix::fs::symlink(builder.src.join("library/stdarch/crates"), &crates_link)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This won't work on non-Linux OSes. Why do we need this symlink?

There are existing functions in bootstrap to create symlinks, see symlink_dir.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we need symlink because the generated Cargo.toml files depend on core_arch via a relative path. We generate into the build directory to avoid writing into the submodule so that path doesn't exist there. The symlink points back at the real stdarch crates so it resolves.

Comment thread src/bootstrap/src/core/build_steps/test.rs Outdated
Comment thread src/bootstrap/src/core/build_steps/test.rs Outdated
Comment thread src/bootstrap/src/core/build_steps/tool.rs Outdated
@xonx4l
Copy link
Copy Markdown
Contributor Author

xonx4l commented May 27, 2026

Thank you!

I wonder what to do about the required dependencies. GCC/Clang is fine, but the SDE emulator seems problematic. We have a lot of CI jobs that will run the intrinsics test and I don't think it is great if we install it on all of them. Of course it is also not great if it is required when running tests locally.

I think that we should make this test be non-default for now, and run it explicitly on selected CI jobs.

View changes since this review

Yes , non-default and run it explicitly on selected CI jobs is better way to handle instead on every CI jobs .

@rust-log-analyzer

This comment has been minimized.

@xonx4l xonx4l force-pushed the port-intrinsic-test branch from 9383a8f to f10bf02 Compare May 29, 2026 13:22
@Kobzol
Copy link
Copy Markdown
Member

Kobzol commented May 30, 2026

Let's run this test explicitly in the ‎x86_64-gnu CI job, as a start. The Dockerfile will need to run something like x test intrinsic-test.

@xonx4l
Copy link
Copy Markdown
Contributor Author

xonx4l commented Jun 1, 2026

Let's run this test explicitly in the ‎x86_64-gnu CI job, as a start. The Dockerfile will need to run something like x test intrinsic-test.

@Kobzol As I added the x86_64-gnu Dockerfile invocation you suggested and ran ./x test library/stdarch/crates/intrinsic-test locally . It failed giving this error.

warning: hidden lifetime parameters in types are deprecated
--> library/stdarch/crates/intrinsic-test/src/common/indentation.rs:20:37
|
20 | fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
| ----------^^^^^^^^^
| |
| expected lifetime parameter
|
= note: -W elided-lifetimes-in-paths implied by -W rust-2018-idioms
= help: to override -W rust-2018-idioms add #[allow(elided_lifetimes_in_paths)]
help: indicate the anonymous lifetime
|
20 | fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
| ++++

error: intrinsic-test (bin "intrinsic-test") generated 1 warning
error: warnings are denied by build.warnings configuration
Build completed unsuccessfully in 0:00:07

@Kobzol
Copy link
Copy Markdown
Member

Kobzol commented Jun 1, 2026

You can just fix the warning, the change will be later synced back to stdarch.

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Jun 1, 2026

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-CI Area: Our Github Actions CI 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-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) 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