atomic_load intrinsic: use const generic parameter for ordering #141507

RalfJung · 2025-05-24T14:41:32Z

We have a gazillion intrinsics for the atomics because we encode the ordering into the intrinsic name rather than making it a parameter. This is particularly bad for those operations that take two orderings. Let's fix that!

This PR only converts load, to see if there's any feedback that would fundamentally change the strategy we pursue for the const generic intrinsics.

The first two commits are preparation and could be a separate PR if you prefer.

@BoxyUwU -- I hope this is a use of const generics that is unlikely to explode? All we need is a const generic of enum type. We could funnel it through an integer if we had to but an enum is obviously nicer...

@bjorn3 it seems like the cranelift backend entirely ignores the ordering?

rustbot · 2025-05-24T14:41:36Z

r? @fee1-dead

rustbot has assigned @fee1-dead.
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 · 2025-05-24T14:41:38Z

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

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

Some changes occurred to the CTFE / Miri interpreter

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

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

bjorn3 · 2025-05-24T15:02:32Z

@bjorn3 it seems like the cranelift backend entirely ignores the ordering?

Correct. Wasm and by extension Cranelift only have SeqCst atomics.

compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs

compiler/rustc_codegen_ssa/src/mir/intrinsic.rs

BoxyUwU · 2025-05-24T20:34:54Z

I hope this is a use of const generics that is unlikely to explode

I think that anything the compiler lets you do with solely adt_const_params (i.e. not unsized_const_params) should work perfectly fine. AFAIK the only outstanding issue with the feature is the problems around CTFE being invoked on illformed bodies but that should only matter for code that doesn't compile. tl;dr I don't see any problems with using an enum in const generics in std.

fee1-dead · 2025-05-26T09:55:14Z

Even though this on the whole LGTM, I'm not really familiar with the backend, so..

r? bjorn3

compiler/rustc_codegen_ssa/src/mir/intrinsic.rs

bjorn3 · 2025-05-26T11:33:32Z

The first two commits are preparation and could be a separate PR if you prefer.

The first two commits LGTM, so if you want to open a separate PR, they can be r+'ed immediately.

intrinsics, ScalarInt: minor cleanup Taken out of rust-lang#141507 while we resolve technical disagreements in that PR. r? `@bjorn3`

intrinsics, ScalarInt: minor cleanup Taken out of rust-lang#141507 while we resolve technical disagreements in that PR. r? ``@bjorn3``

Rollup merge of #141582 - RalfJung:cleanup, r=bjorn3 intrinsics, ScalarInt: minor cleanup Taken out of #141507 while we resolve technical disagreements in that PR. r? ``@bjorn3``

bjorn3 · 2025-05-28T10:22:08Z

LGTM, but there will be a minor conflict with #141404 which is currently in the queue. r=me once it merges and the conflict is fixed.

RalfJung · 2025-05-29T08:08:38Z

@bors r=bjorn3

bors · 2025-05-29T08:08:41Z

📌 Commit a387c86 has been approved by bjorn3

It is now in the queue for this repository.

Rollup of 8 pull requests Successful merges: - #133823 (Use `cfg_attr_trace` in AST with a placeholder attribute for accurate suggestion) - #141004 (Report text_direction_codepoint_in_literal when parsing) - #141407 (Refactor the two-phase check for impls and impl items) - #141430 (remove `visit_clobber` and move `DummyAstNode` to `rustc_expand`) - #141507 (atomic_load intrinsic: use const generic parameter for ordering) - #141538 (implement `va_arg` for x86_64 systemv) - #141669 (float: Replace some approximate assertions with exact) - #141747 (rustdoc: display doc(cfg(false)) properly) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #141507 - RalfJung:atomic-intrinsics, r=bjorn3 atomic_load intrinsic: use const generic parameter for ordering We have a gazillion intrinsics for the atomics because we encode the ordering into the intrinsic name rather than making it a parameter. This is particularly bad for those operations that take two orderings. Let's fix that! This PR only converts `load`, to see if there's any feedback that would fundamentally change the strategy we pursue for the const generic intrinsics. The first two commits are preparation and could be a separate PR if you prefer. `@BoxyUwU` -- I hope this is a use of const generics that is unlikely to explode? All we need is a const generic of enum type. We could funnel it through an integer if we had to but an enum is obviously nicer... `@bjorn3` it seems like the cranelift backend entirely ignores the ordering?

intrinsics, ScalarInt: minor cleanup Taken out of rust-lang#141507 while we resolve technical disagreements in that PR. r? ``@bjorn3``

Rollup of 8 pull requests Successful merges: - rust-lang/rust#133823 (Use `cfg_attr_trace` in AST with a placeholder attribute for accurate suggestion) - rust-lang/rust#141004 (Report text_direction_codepoint_in_literal when parsing) - rust-lang/rust#141407 (Refactor the two-phase check for impls and impl items) - rust-lang/rust#141430 (remove `visit_clobber` and move `DummyAstNode` to `rustc_expand`) - rust-lang/rust#141507 (atomic_load intrinsic: use const generic parameter for ordering) - rust-lang/rust#141538 (implement `va_arg` for x86_64 systemv) - rust-lang/rust#141669 (float: Replace some approximate assertions with exact) - rust-lang/rust#141747 (rustdoc: display doc(cfg(false)) properly) r? `@ghost` `@rustbot` modify labels: rollup

@tautschnig

Relevant upstream PRs: - rust-lang/rust#141507 (identified by @tautschnig): replaces the atomic_load intrinstics (`atomic_load_seqcst`, `atomic_load_acquire`, and `atomic_load_relaxed`) by a new `atomic_load` intrinsic that is generic in the atomic ordering. Update the Kani code and tests accordingly. - rust-lang/rust#137574: Moved the `num` module from `std/src/num.rs` to `std/src/num/mod.rs` which required updating the corresponding path in the `script-based-pre/verify_std_cmd` test. Resolves #4120 ### Call-outs: - Instead of replacing the `String` parameter of Kani's `AtomicLoad` enum variant by `AtomicOrdering`, I opted to remove the parameter altogether since Kani doesn't do anything with it at the moment. We can easily add a parameter to that enum variant in the future if Kani needs it. - The upstream change to `atomic_load` is expected to be followed by similar changes to all atomic intrinsics. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.

atomic_load intrinsic: use const generic parameter for ordering We have a gazillion intrinsics for the atomics because we encode the ordering into the intrinsic name rather than making it a parameter. This is particularly bad for those operations that take two orderings. Let's fix that! This PR only converts `load`, to see if there's any feedback that would fundamentally change the strategy we pursue for the const generic intrinsics. The first two commits are preparation and could be a separate PR if you prefer. `@BoxyUwU` -- I hope this is a use of const generics that is unlikely to explode? All we need is a const generic of enum type. We could funnel it through an integer if we had to but an enum is obviously nicer... `@bjorn3` it seems like the cranelift backend entirely ignores the ordering?

In rust-lang/rust#141507, the `atomic_load_{ordering}` intrinsics were removed and replaced with a single `atomic_load` intrinsic. This commit moves the portable-atomic crate to that new intrinsic.

In rust-lang/rust#141507, the `atomic_load_{ordering}` intrinsics were gated behind the `bootstrap` config and replaced with a single `atomic_load` intrinsic. This commit moves the portable-atomic crate to that new intrinsic.

…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`

…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``

…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```

Atomic intrinsics : use const generic ordering, part 2 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: - #141507 - #141687 - rust-lang/stdarch#1811 - #141964 r? `@bjorn3`

Atomic intrinsics : use const generic ordering, part 2 This completes what got started in rust-lang/rust#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/rust#141507 - rust-lang/rust#141687 - rust-lang/stdarch#1811 - rust-lang/rust#141964 r? `@bjorn3`

rustbot assigned fee1-dead May 24, 2025

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 May 24, 2025

RalfJung force-pushed the atomic-intrinsics branch from 23b2539 to 4c7129b Compare May 24, 2025 14:46

This comment has been minimized.

Sign in to view

bjorn3 reviewed May 24, 2025

View reviewed changes

compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs Outdated Show resolved Hide resolved

bjorn3 reviewed May 24, 2025

View reviewed changes

compiler/rustc_codegen_ssa/src/mir/intrinsic.rs Outdated Show resolved Hide resolved

RalfJung force-pushed the atomic-intrinsics branch 2 times, most recently from 8dbc7a6 to 0cd5bdb Compare May 24, 2025 15:46

This comment has been minimized.

Sign in to view

RalfJung force-pushed the atomic-intrinsics branch from 0cd5bdb to dc1bd79 Compare May 24, 2025 16:43

This comment has been minimized.

Sign in to view

RalfJung force-pushed the atomic-intrinsics branch from dc1bd79 to a9e3996 Compare May 24, 2025 17:11

rustbot assigned bjorn3 and unassigned fee1-dead May 26, 2025

bjorn3 reviewed May 26, 2025

View reviewed changes

compiler/rustc_codegen_ssa/src/mir/intrinsic.rs Outdated Show resolved Hide resolved

RalfJung force-pushed the atomic-intrinsics branch from a9e3996 to cf77933 Compare May 26, 2025 10:16

This comment has been minimized.

Sign in to view

RalfJung force-pushed the atomic-intrinsics branch from cf77933 to 27cd3eb Compare May 26, 2025 11:08

RalfJung mentioned this pull request May 26, 2025

intrinsics, ScalarInt: minor cleanup #141582

Merged

jieyouxu added a commit to jieyouxu/rust that referenced this pull request May 26, 2025

Rollup merge of rust-lang#141582 - RalfJung:cleanup, r=bjorn3

ea4e427

intrinsics, ScalarInt: minor cleanup Taken out of rust-lang#141507 while we resolve technical disagreements in that PR. r? `@bjorn3`

rust-timer added a commit that referenced this pull request May 27, 2025

Unrolled build for #141582

6d5f9b9

Rollup merge of #141582 - RalfJung:cleanup, r=bjorn3 intrinsics, ScalarInt: minor cleanup Taken out of #141507 while we resolve technical disagreements in that PR. r? ``@bjorn3``

RalfJung mentioned this pull request May 28, 2025

Atomic intrinsics : use const generic ordering, part 2 #141700

Merged

RalfJung force-pushed the atomic-intrinsics branch from 28cacd2 to d2a9cb0 Compare May 28, 2025 16:51

RalfJung added 2 commits May 28, 2025 22:57

atomic_load intrinsic: use const generic parameter for ordering

4794ea1

get rid of rustc_codegen_ssa::common::AtomicOrdering

a387c86

RalfJung force-pushed the atomic-intrinsics branch from d2a9cb0 to a387c86 Compare May 28, 2025 20:58

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 May 29, 2025

matthiaskrgr mentioned this pull request May 30, 2025

Rollup of 8 pull requests #141753

Merged

bors merged commit ad2d91c into rust-lang:master May 30, 2025
9 checks passed

rustbot added this to the 1.89.0 milestone May 30, 2025

RalfJung deleted the atomic-intrinsics branch June 1, 2025 08:01

tautschnig mentioned this pull request Jun 2, 2025

Toolchain upgrade to nightly-2025-05-31 failed model-checking/kani#4120

Closed

zhassan-aws mentioned this pull request Jun 2, 2025

Upgrade Rust toolchain to 2025-06-02 model-checking/kani#4123

Merged

teskje mentioned this pull request Jun 5, 2025

atomic128: use the new intrinsics::atomic_load taiki-e/portable-atomic#210

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

atomic_load intrinsic: use const generic parameter for ordering #141507

atomic_load intrinsic: use const generic parameter for ordering #141507

Uh oh!

RalfJung commented May 24, 2025

Uh oh!

rustbot commented May 24, 2025

Uh oh!

rustbot commented May 24, 2025

Uh oh!

This comment has been minimized.

bjorn3 commented May 24, 2025

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

BoxyUwU commented May 24, 2025

Uh oh!

fee1-dead commented May 26, 2025

Uh oh!

Uh oh!

This comment has been minimized.

bjorn3 commented May 26, 2025

Uh oh!

bjorn3 commented May 28, 2025

Uh oh!

RalfJung commented May 29, 2025

Uh oh!

bors commented May 29, 2025

Uh oh!

Uh oh!

Uh oh!

atomic_load intrinsic: use const generic parameter for ordering #141507

atomic_load intrinsic: use const generic parameter for ordering #141507

Uh oh!

Conversation

RalfJung commented May 24, 2025

Uh oh!

rustbot commented May 24, 2025

Uh oh!

rustbot commented May 24, 2025

Uh oh!

This comment has been minimized.

bjorn3 commented May 24, 2025

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

BoxyUwU commented May 24, 2025

Uh oh!

fee1-dead commented May 26, 2025

Uh oh!

Uh oh!

This comment has been minimized.

bjorn3 commented May 26, 2025

Uh oh!

bjorn3 commented May 28, 2025

Uh oh!

RalfJung commented May 29, 2025

Uh oh!

bors commented May 29, 2025

Uh oh!

Uh oh!

Uh oh!