Skip to content

simd_fmin/fmax: make semantics and name consistent with scalar intrinsics#154043

Open
RalfJung wants to merge 1 commit intorust-lang:mainfrom
RalfJung:simd-min-max
Open

simd_fmin/fmax: make semantics and name consistent with scalar intrinsics#154043
RalfJung wants to merge 1 commit intorust-lang:mainfrom
RalfJung:simd-min-max

Conversation

@RalfJung
Copy link
Member

This is the SIMD version of #153343: change the documented semantics of the SIMD float min/max intrinsics to that of the scalar intrinsics, and also make the name consistent.

In terms of providers of this API:

  • Miri, GCC, and cranelift already implement the new semantics, so no changes are needed.
  • LLVM is adjusted to use minimumnum instead of minnum, thus giving us the new semantics.

In terms of consumers of this API:

Q: Should there be an f in the intrinsic name to indicate that it is for floats? E.g., simd_fminimum_number_nsz?

Also see #153395.

@rustbot
Copy link
Collaborator

rustbot commented Mar 18, 2026

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

The Miri subtree was changed

cc @rust-lang/miri

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

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

Some changes occurred to the CTFE machinery

cc @oli-obk, @lcnr

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred to the platform-builtins intrinsics. Make sure the
LLVM backend as well as portable-simd gets adapted for the changes.

cc @antoyo, @GuillaumeGomez, @bjorn3, @calebzulawski, @programmerjake

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

cc @Amanieu, @folkertdev, @sayantn

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

cc @calebzulawski, @programmerjake

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

@rustbot rustbot added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. 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 18, 2026
@rustbot
Copy link
Collaborator

rustbot commented Mar 18, 2026

r? @chenyukang

rustbot has assigned @chenyukang.
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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 69 candidates
  • Random selection from 15 candidates

}
sym::simd_maximum_number_nsz => {
crate::num::codegen_float_max(fx, x_lane, y_lane)
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like the previous PR changed the behavior of both the scalar and SIMD intrinsics in cranelift oops. Well, with this PR that actually matches what we document then. ;)

Copy link
Member

@programmerjake programmerjake left a comment

Choose a reason for hiding this comment

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

the llvm and portable-simd parts lgtm.

View changes since this review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants