Skip to content

atomic intrinsics: clarify which types are supported and (if applicable) what happens with provenance#138398

Merged
bors merged 1 commit intorust-lang:masterfrom
RalfJung:atomic-intrinsics-provenance
Mar 14, 2025
Merged

atomic intrinsics: clarify which types are supported and (if applicable) what happens with provenance#138398
bors merged 1 commit intorust-lang:masterfrom
RalfJung:atomic-intrinsics-provenance

Conversation

@RalfJung
Copy link
Member

@RalfJung RalfJung commented Mar 12, 2025

The provenance semantics match what Miri implements and what the AtomicPtr API expects.

@rustbot
Copy link
Collaborator

rustbot commented Mar 12, 2025

r? @thomcc

rustbot has assigned @thomcc.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler 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. labels Mar 12, 2025
@rustbot
Copy link
Collaborator

rustbot commented Mar 12, 2025

Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter
gets adapted for the changes, if necessary.

cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

RalfJung
RalfJung commented Mar 12, 2025
View reviewed changes
library/core/src/intrinsics/mod.rs
#[rustc_nounwind]
pub unsafe fn atomic_max_acqrel<T: Copy>(_dst: *mut T, _src: T) -> T;
/// Maximum with the current value.
/// Maximum with the current value using a signed comparison.
Copy link
Member Author

@RalfJung RalfJung Mar 12, 2025

Choose a reason for hiding this comment

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

It seems when "using a signed comparison" was added everywhere, atomic_max_relaxed was forgotten.

@bors
Copy link
Collaborator

bors commented Mar 12, 2025

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

@thomcc
Copy link
Member

thomcc commented Mar 13, 2025

Doesn't seem like a libs review really. r? compiler

@rustbot rustbot assigned nnethercote and unassigned thomcc Mar 13, 2025
nnethercote
nnethercote reviewed Mar 13, 2025
View reviewed changes
library/core/src/intrinsics/mod.rs
// memory, which is not valid for either `&` or `&mut`.

/// Stores a value if the current value is the same as the `old` value.
/// `T` must be an integer or pointer type.
Copy link
Contributor

@nnethercote nnethercote Mar 13, 2025

Choose a reason for hiding this comment

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

Not relevant for this PR, but I can't help but wonder if macros could be used to reduce the massive amounts of repetition in this file.

Copy link
Member Author

@RalfJung RalfJung Mar 13, 2025

Choose a reason for hiding this comment

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

I think that would make the file harder to read. I'd rather see the atomic intrinsics refactored so that the "ordering" becomes a const generic argument, so that we don't need 5 intrinsics for each individual operation any more.

@nnethercote
Copy link
Contributor

nnethercote commented Mar 13, 2025

I don't know much about this, but seems reasonable. r=me once the conflicts are resolved.

@RalfJung RalfJung force-pushed the atomic-intrinsics-provenance branch from 348d6a8 to 88b206d Compare March 13, 2025 07:14
@RalfJung
Copy link
Member Author

RalfJung commented Mar 13, 2025

@bors r=nnethercote rollup

@bors
Copy link
Collaborator

bors commented Mar 13, 2025

📌 Commit 88b206d has been approved by nnethercote

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 Mar 13, 2025
@jhpratt jhpratt mentioned this pull request Mar 14, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 14, 2025
@bors
Auto merge of rust-lang#138480 - jhpratt:rollup-y3b8wu5, r=jhpratt
f7b4354 
Rollup of 16 pull requests

Successful merges:

 - rust-lang#136001 (Overhaul examples for PermissionsExt)
 - rust-lang#136230 (Reword incorrect documentation about SocketAddr having varying layout)
 - rust-lang#136892 (Sync Fuchsia target spec with clang Fuchsia driver)
 - rust-lang#136911 (Add documentation URL to selected jobs)
 - rust-lang#137870 ( Improve HashMap docs for const and static initializers)
 - rust-lang#138179 (Add `src/tools/x` to the main workspace)
 - rust-lang#138389 (use `expect` instead of `allow`)
 - rust-lang#138396 (Enable metrics and verbose tests in PR CI)
 - rust-lang#138398 (atomic intrinsics: clarify which types are supported and (if applicable) what happens with provenance)
 - rust-lang#138432 (fix: remove the check of lld not supporting `@response-file)`
 - rust-lang#138434 (Visit `PatField` when collecting lint levels)
 - rust-lang#138441 (update error message)
 - rust-lang#138442 (EUV: fix place of deref pattern's interior's scrutinee)
 - rust-lang#138457 (Remove usage of legacy scheme paths on RedoxOS)
 - rust-lang#138461 (Remove an outdated line from a test comment)
 - rust-lang#138466 (Remove myself from libs review)

Failed merges:

 - rust-lang#138452 (Remove `RUN_CHECK_WITH_PARALLEL_QUERIES`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 91e4bab into rust-lang:master Mar 14, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 14, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 14, 2025
@rust-timer
Unrolled build for rust-lang#138398
e7ce833 
Rollup merge of rust-lang#138398 - RalfJung:atomic-intrinsics-provenance, r=nnethercote

atomic intrinsics: clarify which types are supported and (if applicable) what happens with provenance

The provenance semantics match what Miri implements and what the `AtomicPtr` API expects.
@RalfJung RalfJung deleted the atomic-intrinsics-provenance branch March 17, 2025 07:32
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Mar 19, 2025
@jhpratt
Rollup merge of rust-lang#138398 - RalfJung:atomic-intrinsics-provena…
8c12431 
…nce, r=nnethercote

atomic intrinsics: clarify which types are supported and (if applicable) what happens with provenance

The provenance semantics match what Miri implements and what the `AtomicPtr` API expects.
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Mar 19, 2025
@bors
Auto merge of rust-lang#138480 - jhpratt:rollup-y3b8wu5, r=jhpratt
0b766fc 
Rollup of 16 pull requests

Successful merges:

 - rust-lang#136001 (Overhaul examples for PermissionsExt)
 - rust-lang#136230 (Reword incorrect documentation about SocketAddr having varying layout)
 - rust-lang#136892 (Sync Fuchsia target spec with clang Fuchsia driver)
 - rust-lang#136911 (Add documentation URL to selected jobs)
 - rust-lang#137870 ( Improve HashMap docs for const and static initializers)
 - rust-lang#138179 (Add `src/tools/x` to the main workspace)
 - rust-lang#138389 (use `expect` instead of `allow`)
 - rust-lang#138396 (Enable metrics and verbose tests in PR CI)
 - rust-lang#138398 (atomic intrinsics: clarify which types are supported and (if applicable) what happens with provenance)
 - rust-lang#138432 (fix: remove the check of lld not supporting `@response-file)`
 - rust-lang#138434 (Visit `PatField` when collecting lint levels)
 - rust-lang#138441 (update error message)
 - rust-lang#138442 (EUV: fix place of deref pattern's interior's scrutinee)
 - rust-lang#138457 (Remove usage of legacy scheme paths on RedoxOS)
 - rust-lang#138461 (Remove an outdated line from a test comment)
 - rust-lang#138466 (Remove myself from libs review)

Failed merges:

 - rust-lang#138452 (Remove `RUN_CHECK_WITH_PARALLEL_QUERIES`)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nnethercote nnethercote nnethercote left review comments

Assignees

@nnethercote nnethercote

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler 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

Milestone

1.87.0

Development

Successfully merging this pull request may close these issues.

5 participants

@RalfJung @rustbot @bors @thomcc @nnethercote