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

src/tools/miri/src/lib.rs

compiler/rustc_codegen_ssa/src/mir/intrinsic.rs

src/tools/miri/src/lib.rs

rustbot · 2025-05-28T10:15:12Z

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

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

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 force-pushed the atomic-intrinsics branch 2 times, most recently from 45b1a24 to ee0da5a Compare May 28, 2025 09:45

bjorn3 reviewed May 28, 2025

View reviewed changes

src/tools/miri/src/lib.rs Outdated Show resolved Hide resolved

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

RalfJung force-pushed the atomic-intrinsics branch from ee0da5a to f6766cf Compare May 28, 2025 10:09

bjorn3 reviewed May 28, 2025

View reviewed changes

src/tools/miri/src/lib.rs Outdated Show resolved Hide resolved

RalfJung force-pushed the atomic-intrinsics branch from f6766cf to b1b4974 Compare May 28, 2025 10:15

rustbot added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label May 28, 2025

RalfJung force-pushed the atomic-intrinsics branch 2 times, most recently from 9fb2266 to d2a9cb0 Compare May 28, 2025 10:16

RalfJung mentioned this pull request May 28, 2025

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

Open

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

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!

Uh oh!

Uh oh!

Uh oh!

rustbot commented May 28, 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!