Atomic intrinsics : use const generic ordering, part 2 #141700
Merged
Conversation
This file contains hidden or 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
rustbot
added
A-LLVM
|
Some changes occurred in compiler/rustc_codegen_gcc Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr Some changes occurred in compiler/rustc_codegen_ssa The Miri subtree was changed cc @rust-lang/miri Some changes occurred in compiler/rustc_codegen_cranelift cc @bjorn3 |
RalfJung
changed the title
This comment has been minimized.
This comment has been minimized.
RalfJung
commented
May 28, 2025
Comment on lines
-225
to
-231
| } else if intrinsic_name == sym::contract_check_ensures { | ||
| // contract_check_ensures::<Ret, C>(Ret, C) -> Ret | ||
| // where C: for<'a> Fn(&'a Ret) -> bool, | ||
| // | ||
| // so: two type params, 0 lifetime param, 0 const params, two inputs, no return | ||
| (2, 0, 0, vec![param(0), param(1)], param(1), hir::Safety::Safe) |
There was a problem hiding this comment.
@celinval do you know why this intrinsic is handled separately rather than in the big match with all the others?
RalfJung
force-pushed
the
atomic-intrinsics-part2
branch
from
28cacd2 to
b35d186
Compare
This comment has been minimized.
This comment has been minimized.
RalfJung
added
the
S-blocked
|
☔ The latest upstream changes (presumably #141716) made this pull request unmergeable. Please resolve the merge conflicts. |
RalfJung
force-pushed
the
atomic-intrinsics-part2
branch
from
b35d186 to
1b4ba1f
Compare
bjorn3
reviewed
May 30, 2025
| #[rustc_intrinsic] | ||
| pub unsafe fn atomic_xchg_seqcst<T>(dst: *mut T, src: T) -> T; | ||
| pub unsafe fn atomic_xchg<T, const ORD: AtomicOrdering>(dst: *mut T, src: T) -> T; |
There was a problem hiding this comment.
Can this test use core::intrinsics instead?
Edit: Nope, that would invalidate the test. It seems like it could use a different intrinsic though.
Edit2: This test was added in 22e3a85
There was a problem hiding this comment.
Yeah I don't know what this is testing so I changed it as little as possible...
This comment has been minimized.
This comment has been minimized.
RalfJung
force-pushed
the
atomic-intrinsics-part2
branch
from
1b4ba1f to
bfcece2
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
RalfJung
force-pushed
the
atomic-intrinsics-part2
branch
from
bfcece2 to
581de04
Compare
This comment has been minimized.
This comment has been minimized.
|
Uh, how could this PR possibly break that incremental test... ( |
|
I don't really know much about incremental 🙏 Hope you manage to figure it out |
|
I can look later if you are willing to wait a day or so 🤔 |
|
The PR is blocked on some other stuff anyway. |
|
This isn't an incremental bug per se, but instead a bug that has to do with debug printing query keys when debug assertions and I've pushed a commit which should fix this. |
rustbot
added
the
A-query-system
This comment has been minimized.
This comment has been minimized.
|
☔ The latest upstream changes (presumably #139118) made this pull request unmergeable. Please resolve the merge conflicts. |
RalfJung
force-pushed
the
atomic-intrinsics-part2
branch
from
0c133b6 to
5282405
Compare
|
This PR modifies cc @jieyouxu |
|
Finally. @bors r=bjorn3 |
bors
added
S-waiting-on-bors
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Jun 8, 2025
…r=bjorn3 Atomic intrinsics : use const generic ordering, part 2 This completes what got started in rust-lang#141507 by using a const generic for the ordering for all intrinsics. It is based on that PR; only the last commit is new. Blocked on: - rust-lang#141507 - rust-lang#141687 - rust-lang/stdarch#1811 - rust-lang#141964 r? `@bjorn3`
bors
added a commit
that referenced
this pull request
Jun 8, 2025
Rollup of 7 pull requests Successful merges: - #141001 (Make NonZero<char> possible) - #141700 (Atomic intrinsics : use const generic ordering, part 2) - #141993 (Use the in-tree `compiler-builtins` for the sysroot) - #142008 (const-eval error: always say in which item the error occurred) - #142053 (Add new Tier-3 targets: `loongarch32-unknown-none*`) - #142132 (`tests/ui`: A New Order [6/N]) - #142179 (store `target.min_global_align` as an `Align`) r? `@ghost` `@rustbot` modify labels: rollup
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Jun 8, 2025
…r=bjorn3 Atomic intrinsics : use const generic ordering, part 2 This completes what got started in rust-lang#141507 by using a const generic for the ordering for all intrinsics. It is based on that PR; only the last commit is new. Blocked on: - rust-lang#141507 - rust-lang#141687 - rust-lang/stdarch#1811 - rust-lang#141964 r? ``@bjorn3``
bors
added a commit
that referenced
this pull request
Jun 8, 2025
Rollup of 6 pull requests Successful merges: - #141001 (Make NonZero<char> possible) - #141700 (Atomic intrinsics : use const generic ordering, part 2) - #142008 (const-eval error: always say in which item the error occurred) - #142053 (Add new Tier-3 targets: `loongarch32-unknown-none*`) - #142132 (`tests/ui`: A New Order [6/N]) - #142179 (store `target.min_global_align` as an `Align`) r? `@ghost` `@rustbot` modify labels: rollup
workingjubilee
added a commit
to workingjubilee/rustc
that referenced
this pull request
Jun 8, 2025
…r=bjorn3 Atomic intrinsics : use const generic ordering, part 2 This completes what got started in rust-lang#141507 by using a const generic for the ordering for all intrinsics. It is based on that PR; only the last commit is new. Blocked on: - rust-lang#141507 - rust-lang#141687 - rust-lang/stdarch#1811 - rust-lang#141964 r? ```@bjorn3```
bors
added a commit
that referenced
this pull request
Jun 8, 2025
Rollup of 11 pull requests Successful merges: - #140774 (Affirm `-Cforce-frame-pointers=off` does not override) - #141001 (Make NonZero<char> possible) - #141700 (Atomic intrinsics : use const generic ordering, part 2) - #142008 (const-eval error: always say in which item the error occurred) - #142053 (Add new Tier-3 targets: `loongarch32-unknown-none*`) - #142089 (Replace all uses of sysroot_candidates with get_or_default_sysroot) - #142108 (compiler: Add track_caller to AbiMapping::unwrap) - #142132 (`tests/ui`: A New Order [6/N]) - #142162 (UnsafePinned: update get() docs and signature to allow shared mutation) - #142171 (`tests/ui`: A New Order [7/N]) - #142179 (store `target.min_global_align` as an `Align`) r? `@ghost` `@rustbot` modify labels: rollup
|
☀️ Test successful - checks-actions |
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 fb644e6 (parent) -> 6ccd447 (this PR) Test differencesShow 904 test diffsStage 1
Stage 2
(and 18 additional test diffs) Additionally, 786 doctest diffs were found. These are ignored, as they are noisy. Job group index
Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 6ccd4476036edfce364e6271f9e190ec7a2a1ff5 --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
This was referenced Jun 8, 2025
|
Finished benchmarking commit (6ccd447): comparison URL. Overall result: ❌✅ regressions and improvements - no action needed@rustbot label: -perf-regression Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (primary 0.8%, secondary 2.1%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (primary -1.4%, secondary -2.4%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeResults (primary 0.0%, secondary 0.0%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Bootstrap: 750.755s -> 751.825s (0.14%) |
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.
This completes what got started in #141507 by using a const generic for the ordering for all intrinsics. It is based on that PR; only the last commit is new.
Blocked on:
r? @bjorn3