Skip to content

Move std_detect into stdlib #143412

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

Merged
merged 13 commits into from
Jul 23, 2025
Merged

Move std_detect into stdlib #143412

merged 13 commits into from
Jul 23, 2025

Conversation

Kobzol
Copy link
Member

@Kobzol Kobzol commented Jul 4, 2025

This PR moves the std_detect crate from stdarch to be a part of rust-lang/rust instead.

The first commit actually moves the whole directory from the stdarch Josh subtree, so that git blame history is kept intact. Then I had to make a few changes to appease tidy.

The most complex thing here is porting the tests. We can't have std_detect both in r-l/r and stdarch, because they could get desynchronized, so we have to perform the move more or less "atomically", which means that we also have to port all the existing std_detect tests from the stdarch repository.

The stdarch repo runs the following std_detect tests:

Build

The build-std-detect.sh script (https://github.com/rust-lang/stdarch/blob/e2b6512aed87df45294ae680181eeef7a802cd95/ci/build-std-detect.sh) builds std_detect using the nightly compiler for several targets. This will be subsumed by normal x build library on our Tier 1/2 targets. However, the stdarch repository also tests the following targets:

  • aarch64-unknown-freebsd
  • armv6-unknown-freebsd
  • powerpc-unknown-freebsd
  • powerpc64-unknown-freebsd
  • aarch64-unknown-openbsd

Which we don't build/test on our CI currently. I think we have mostly two options here:

  1. Ignore these targets
  2. Create a special CI job that will build stage 1 rustc and then cross-compile std (or just the std_detect crate?) for these targets.

Documentation

The dox.sh script (https://github.com/rust-lang/stdarch/blob/3fec5adcd52a815f227805d4959a25b6402c7fd5/ci/dox.sh) builds and documents std_detect for several targets. All of them are Tier 2/we have dist- jobs for them, so I think that we can just skip this and let our normal CI subsume it?

Tests

The run.sh script (https://github.com/rust-lang/stdarch/blob/1b201cec2cca7465602a65ed6ae60517224b15f3/ci/run.sh) runs cargo test on std_detect with a bunch of variations of feature flags. This will be subsumed by x test library in our CI. The only problem is that stdarch runs these tests for a ludicrous number of targets:

        - tuple: i686-unknown-linux-gnu
        - tuple: x86_64-unknown-linux-gnu
        - tuple: arm-unknown-linux-gnueabihf
        - tuple: armv7-unknown-linux-gnueabihf
        - tuple: aarch64-unknown-linux-gnu
        - tuple: aarch64_be-unknown-linux-gnu
        - tuple: riscv32gc-unknown-linux-gnu
        - tuple: riscv64gc-unknown-linux-gnu
        - tuple: powerpc-unknown-linux-gnu
        - tuple: powerpc64-unknown-linux-gnu
        - tuple: powerpc64le-unknown-linux-gnu
        - tuple: s390x-unknown-linux-gnu
        - tuple: i586-unknown-linux-gnu
        - tuple: nvptx64-nvidia-cuda
        - tuple: thumbv6m-none-eabi
        - tuple: thumbv7m-none-eabi
        - tuple: thumbv7em-none-eabi
        - tuple: thumbv7em-none-eabihf
        - tuple: loongarch64-unknown-linux-gnu
        - tuple: wasm32-wasip1
        - tuple: x86_64-apple-darwin
        - tuple: x86_64-apple-ios-macabi
        - tuple: aarch64-apple-darwin
        - tuple: aarch64-apple-ios-macabi
        - tuple: x86_64-pc-windows-msvc
        - tuple: i686-pc-windows-msvc
        - tuple: aarch64-pc-windows-msvc
        - tuple: x86_64-pc-windows-gnu
        - tuple: aarch64-unknown-linux-gnu
        - tuple: aarch64_be-unknown-linux-gnu
        - tuple: armv7-unknown-linux-gnueabihf
        - tuple: loongarch64-unknown-linux-gnu
        - tuple: powerpc-unknown-linux-gnu
        - tuple: powerpc64-unknown-linux-gnu
        - tuple: powerpc64le-unknown-linux-gnu
        - tuple: riscv32gc-unknown-linux-gnu
        - tuple: riscv64gc-unknown-linux-gnu
        - tuple: s390x-unknown-linux-gnu
        - tuple: x86_64-unknown-linux-gnu
        - tuple: aarch64-apple-darwin
        - tuple: aarch64-apple-ios-macabi

We definitely do not run tests for all of these targets on our CI.

Outcome

We have decided to just subsume std_detect tests by our normal test suite for now, and not create a separate CI job. Therefore, this PR performs the following changes in target testing for std_detect:

The following T3 targets would go from "build" to "nothing":

aarch64-unknown-freebsd (T3)
armv6-unknown-freebsd (T3)
powerpc-unknown-freebsd (T3)
powerpc64-unknown-freebsd (T3)
aarch64-unknown-openbsd (T3)

The following T3 targets would go from "test" to "nothing":

aarch64_be-unknown-linux-gnu (T3)
riscv32gc-unknown-liux-gnu (T3)

The following T2 targets would go from "test" to "build":

arm-unknown-linux-gnueabihf (T2)
armv7-unknown-linux-gnueabihf (T2)
riscv64gc-unknown-linux-gnu (T2)
powerpc-unknown-linux-gnu (T2)
powerpc64-unknown-linux-gnu (T2)
powerpc64le-unknown-linux-gnu (T2)
s390x-unknown-linux-gnu (T2)
i586-unknown-linux-gnu (T2)
loongarch64-unknown-linux-gnu (T2)
wasm32-wasip1 (T2)
x86_64-apple-ios-macabi (T2)
aarch64-apple-ios-macabi (T2)
aarch64-pc-windows-msvc (T2)
armv7-unknown-linux-gnueabihf (T2)
loongarch64-unknown-linux-gnu (T2)
powerpc-unknown-linux-gnu (T2)

I have confirmed in rust-lang/stdarch#1873 that the current version of this PR would pass stdarch's CI testsuite.

r? @ghost

try-job: armhf-gnu
try-job: arm-android

@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Jul 4, 2025
@rust-log-analyzer

This comment has been minimized.

@rustbot rustbot added A-tidy Area: The tidy tool T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jul 4, 2025
@rust-log-analyzer

This comment has been minimized.

@Kobzol Kobzol force-pushed the std-detect-in-stdlib branch from 9086668 to 277837d Compare July 4, 2025 13:25
@Kobzol Kobzol marked this pull request as ready for review July 8, 2025 08:45
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 8, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 8, 2025

These commits modify the library/Cargo.lock file. Unintentional changes to library/Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

There are changes to the tidy tool.

cc @jieyouxu

@Kobzol Kobzol changed the title Move std_detect into stdlib [do not merge] Move std_detect into stdlib Jul 8, 2025
@Kobzol
Copy link
Member Author

Kobzol commented Jul 8, 2025

This is not yet ready for review, but it will need some input from stdarch maintainers regarding the test changes, as they have to happen in this PR, otherwise we either lose testing for std_detect for some time, or we will have two duplicated sources for std_detect, neither is great. Not sure who to ping exactly about the CI changes though.

CC @rust-lang/libs

@cuviper
Copy link
Member

cuviper commented Jul 8, 2025

One of my peers is looking at implementing aarch64 outlined atomics in compiler-builtins, which needs to check for the LSE feature. Will it still be possible for it to use is_aarch64_feature_detected! if this is moved in-tree?

(That said, I'm not sure if that can work either way since the feature caching also uses atomics, so that might create a chicken-and-egg situation...)

@Kobzol
Copy link
Member Author

Kobzol commented Jul 8, 2025

How would it work before? std_detect wasn't published to crates.io in ~6 years. So I assume that the code just accessed std_detect through std? That should still continue working.

@tgross35
Copy link
Contributor

tgross35 commented Jul 8, 2025

One of my peers is looking at implementing aarch64 outlined atomics in compiler-builtins, which needs to check for the LSE feature. Will it still be possible for it to use is_aarch64_feature_detected! if this is moved in-tree?

Do you mean LSE would need to be checked within compiler-builtins? I don't think that is easily possible since AIUI std_detect for aarch64 requires filesystem access and builtins has to be dependency-free. It might be possible to do some things with weak symbols or a separate .o that gets linked only if std is available, but that gets tricky. We do some detection on x86, which is a clone of core_detect, but that doesn't require access to privileged registers https://github.com/rust-lang/compiler-builtins/blob/8aba4c899ee89eef7fe688cdfa6629ddd56908f9/libm/src/math/arch/x86/detect.rs.

(Amanieu would know more if you want to zulip this)

@cuviper
Copy link
Member

cuviper commented Jul 8, 2025

How would it work before? std_detect wasn't published to crates.io in ~6 years. So I assume that the code just accessed std_detect through std?

The code is currently not doing this, only implementing the more basic LL/LC mode that doesn't need feature checks. That kind of defeats the point of "outlined atomics", but it's functional.

https://github.com/rust-lang/compiler-builtins/blob/8aba4c899ee89eef7fe688cdfa6629ddd56908f9/compiler-builtins/src/aarch64_linux.rs

Do you mean LSE would need to be checked within compiler-builtins?

Yes, I think so, but I'm open to suggestions.

I don't think that is easily possible since AIUI std_detect for aarch64 requires filesystem access and builtins has to be dependency-free.

Note that compiler-builtins recommends to just use compiler-rt if you care, i.e. setting bootstrap build.optimized-compiler-builtins, but that implementation is also not dependency-free. On Linux, it uses getauxval in a constructor here:

https://github.com/llvm/llvm-project/blob/6a993264ee0105da32a6a57fb077796076cf6bf4/compiler-rt/lib/builtins/cpu_model/aarch64.c#L49-L51
https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/builtins/cpu_model/aarch64/lse_atomics/getauxval.inc

@cuviper
Copy link
Member

cuviper commented Jul 8, 2025

Anyway, as I said, I'm not even sure it's possible for that to work, given that std_detect caching also uses atomics. I just wanted to raise this possibility in case it affects our choices here, to move or not.

@tgross35
Copy link
Contributor

tgross35 commented Jul 8, 2025

Rather than checking auxvec in compiler-builtins itself, could we check a weakly defined symbol? Then if std is available it provides this symbol, and if it's not available then compiler-builtins will fall back to the default implementation. That keeps the dependency tree clean and all of std (mostly fs and alloc) can be used to implement the feature detection. I think the proposed way of doing feature detection in core would work along those lines rust-lang/rfcs#3469 (comment).

LLVM indeed puts everything into compiler-rt, but I don't believe we need to follow exact suit here (#138944 is similar).

@cuviper
Copy link
Member

cuviper commented Jul 9, 2025

If we allow a constructor like LLVM is doing, then we could have a static symbol like __aarch64_have_lse_atomics (not necessarily that same name) in compiler-builtins that's initialized false, and a constructor in std that checks for LSE to set it true.

@bors
Copy link
Collaborator

bors commented Jul 11, 2025

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

@Kobzol
Copy link
Member Author

Kobzol commented Jul 11, 2025

r? @Amanieu

Not sure who is responsible for maintaining the stdarch CI test suite :)

Copy link
Member

@Amanieu Amanieu left a comment

Choose a reason for hiding this comment

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

I'm happy with the test changes. r=me

@Kobzol
Copy link
Member Author

Kobzol commented Jul 11, 2025

Just to clarify, I haven't implemented any of these test changes yet 😅 Just wanted to get a vibe check on the general direction. I'll work on it then.

@Kobzol Kobzol force-pushed the std-detect-in-stdlib branch from 277837d to 92d9d7f Compare July 13, 2025 11:14
@rustbot
Copy link
Collaborator

rustbot commented Jul 13, 2025

stdarch is developed in its own repository. If possible, consider making this change to rust-lang/stdarch instead.

cc @Amanieu, @folkertdev, @sayantn

@Kobzol
Copy link
Member Author

Kobzol commented Jul 22, 2025

Rebased on top of #144222.

@Kobzol
Copy link
Member Author

Kobzol commented Jul 22, 2025

@bors try

rust-bors bot added a commit that referenced this pull request Jul 22, 2025
Move `std_detect` into stdlib

try-job: armhf-gnu
try-job: arm-android
@rust-bors
Copy link

rust-bors bot commented Jul 22, 2025

⌛ Trying commit 9a1179c with merge f1a4070

To cancel the try build, run the command @bors try cancel.

@rust-bors
Copy link

rust-bors bot commented Jul 22, 2025

☀️ Try build successful (CI)
Build commit: f1a4070 (f1a4070cf722f76eb91baa2a07c58f6e235c7642, parent: 2e5367566819ca7878baa9600ae7a93eb0e37bbf)

@Kobzol
Copy link
Member Author

Kobzol commented Jul 23, 2025

This was just a rebase and git history cleanup, no functional changes, so reapproving.

@bors r=Amanieu

@bors
Copy link
Collaborator

bors commented Jul 23, 2025

📌 Commit 9a1179c has been approved by Amanieu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 23, 2025
@bors
Copy link
Collaborator

bors commented Jul 23, 2025

⌛ Testing commit 9a1179c with merge 5a30e43...

@bors
Copy link
Collaborator

bors commented Jul 23, 2025

☀️ Test successful - checks-actions
Approved by: Amanieu
Pushing 5a30e43 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 23, 2025
@bors bors merged commit 5a30e43 into rust-lang:master Jul 23, 2025
13 checks passed
@rustbot rustbot added this to the 1.90.0 milestone Jul 23, 2025
Copy link
Contributor

What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 20aa182 (parent) -> 5a30e43 (this PR)

Test differences

Show 31 test diffs

Stage 1

  • dump: [missing] -> pass (J0)
  • x86: [missing] -> pass (J0)
  • x86_deprecated: [missing] -> pass (J0)
  • all: [missing] -> pass (J1)
  • detect::arch::aarch64::unexpected_cfgs: [missing] -> pass (J1)
  • detect::arch::arm::unexpected_cfgs: [missing] -> pass (J1)
  • detect::arch::loongarch::unexpected_cfgs: [missing] -> pass (J1)
  • detect::arch::mips64::unexpected_cfgs: [missing] -> pass (J1)
  • detect::arch::mips::unexpected_cfgs: [missing] -> pass (J1)
  • detect::arch::powerpc64::unexpected_cfgs: [missing] -> pass (J1)
  • detect::arch::powerpc::unexpected_cfgs: [missing] -> pass (J1)
  • detect::arch::riscv::unexpected_cfgs: [missing] -> pass (J1)
  • detect::arch::s390x::unexpected_cfgs: [missing] -> pass (J1)
  • detect::arch::x86::unexpected_cfgs: [missing] -> pass (J1)
  • arm: [missing] -> pass (J2)
  • detect::os::auxvec::tests::auxv_crate: [missing] -> pass (J2)
  • detect::os::auxvec::tests::linux_macos_vb: [missing] -> pass (J2)
  • detect::os::auxvec::tests::linux_rpi3: [missing] -> pass (J2)
  • detect::os::aarch64::tests::auxv_from_file::linux_empty_hwcap2_aarch64: [missing] -> pass (J3)
  • detect::os::aarch64::tests::auxv_from_file::linux_hwcap2_aarch64: [missing] -> pass (J3)
  • detect::os::aarch64::tests::auxv_from_file::linux_no_hwcap2_aarch64: [missing] -> pass (J3)
  • detect::os::auxvec::tests::linux_artificial_aarch64: [missing] -> pass (J3)
  • detect::os::auxvec::tests::linux_no_hwcap2_aarch64: [missing] -> pass (J3)
  • detect::os::auxvec::tests::auxv_crate_procfs: [missing] -> pass (J4)
  • detect::os::auxvec::tests::auxv_dump: [missing] -> pass (J4)
  • detect::os::auxvec::tests::auxv_dump_procfs: [missing] -> pass (J4)
  • aarch64_windows: [missing] -> pass (J5)
  • aarch64: [missing] -> pass (J6)
  • aarch64_darwin: [missing] -> pass (J7)

Additionally, 2 doctest diffs were found. These are ignored, as they are noisy.

Job group index

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 5a30e4307f0506bed87eeecd171f8366fdbda1dc --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

  1. dist-x86_64-apple: 15319.3s -> 8998.5s (-41.3%)
  2. dist-aarch64-apple: 5128.4s -> 6557.4s (27.9%)
  3. aarch64-apple: 5254.3s -> 6035.4s (14.9%)
  4. pr-check-2: 2594.8s -> 2210.3s (-14.8%)
  5. x86_64-gnu-aux: 6785.1s -> 5905.3s (-13.0%)
  6. i686-gnu-nopt-1: 8138.7s -> 7166.0s (-12.0%)
  7. x86_64-rust-for-linux: 2908.0s -> 2562.7s (-11.9%)
  8. x86_64-gnu-llvm-20-1: 3684.6s -> 3270.5s (-11.2%)
  9. x86_64-gnu-debug: 6123.7s -> 5556.6s (-9.3%)
  10. dist-aarch64-linux: 5494.5s -> 5990.7s (9.0%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@Kobzol Kobzol deleted the std-detect-in-stdlib branch July 23, 2025 12:57
Kobzol pushed a commit to Kobzol/stdarch that referenced this pull request Jul 23, 2025
stdarch subtree update

Subtree update of `stdarch` to rust-lang@5531955.

Created using https://github.com/rust-lang/josh-sync.

I saw that there were non-trivial changes made to `std_detect` in `stdarch` recently. So I want to get them merged here before we move forward with rust-lang/rust#143412.

r? `@folkertdev`
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (5a30e43): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.1% [-0.1%, -0.1%] 1
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary -1.2%, secondary -0.9%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
2.8% [2.8%, 2.8%] 1
Regressions ❌
(secondary)
1.1% [1.1%, 1.1%] 2
Improvements ✅
(primary)
-5.3% [-5.3%, -5.3%] 1
Improvements ✅
(secondary)
-2.9% [-3.7%, -2.1%] 2
All ❌✅ (primary) -1.2% [-5.3%, 2.8%] 2

Cycles

Results (secondary 2.3%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.3% [2.0%, 3.0%] 3
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 469.114s -> 467.483s (-0.35%)
Artifact size: 374.63 MiB -> 374.67 MiB (0.01%)

github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Jul 24, 2025
stdarch subtree update

Subtree update of `stdarch` to rust-lang/stdarch@5531955.

Created using https://github.com/rust-lang/josh-sync.

I saw that there were non-trivial changes made to `std_detect` in `stdarch` recently. So I want to get them merged here before we move forward with rust-lang/rust#143412.

r? `@folkertdev`
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 A-tidy Area: The tidy tool merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants