Skip to content

Revert "Merge pull request #1871 from folkertdev/aarch64-float-min-max"#2052

Merged
folkertdev merged 1 commit intorust-lang:mainfrom
RalfJung:avoid-simd-minmax
Mar 10, 2026
Merged

Revert "Merge pull request #1871 from folkertdev/aarch64-float-min-max"#2052
folkertdev merged 1 commit intorust-lang:mainfrom
RalfJung:avoid-simd-minmax

Conversation

@RalfJung
Copy link
Member

@RalfJung RalfJung commented Mar 10, 2026

This reverts #1871. For context, see the discussion in rust-lang/rust#153395.

Using simd_fmin/max for the aarch64 intrinsics is semantically incorrect:

  • If we take the way simd_fmin/max are documented as ground truth, then they behave non-deterministically for comparing signed zeros. The Arm intrinsic (if I understood the docs correctly) guarantees that -0.0 < +0.0 for the purpose of min/max.
  • If we take the documentation of the LLVM intrinsics that simd_fmin/max map to, that avoids the signed zero problem ([LangRef] Clarify specification for float min/max operations llvm/llvm-project#172012 says that fmin/max now respect signed zeros, though this is probably not yet correctly implemented everywhere), but behavior is still incorrect for NaNs: the Arm operation (if I understood the docs correctly) returns NaN when an input is SNaN (only QNaN gets "ignored"), but LLVM is allowed to fold fmin(SNaN, x) to x.

Given that float min/max behavior has an entire zoo of options and differs widely across targets, I think it just doesn't make sense to try to use target-independent intrinsics like simd_fmin/max to implement target-specific semantics for the stdarch intrinsics.

Cc @folkertdev

…at-min-max"

This reverts commit 6a8a764, reversing
changes made to a37563b.
@rustbot
Copy link
Collaborator

rustbot commented Mar 10, 2026

r? @folkertdev

rustbot has assigned @folkertdev.
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: @Amanieu, @folkertdev, @sayantn
  • @Amanieu, @folkertdev, @sayantn expanded to Amanieu, folkertdev, sayantn
  • Random selection from Amanieu, folkertdev, sayantn

@folkertdev
Copy link
Contributor

Seems fine. The PR also touches some integer min/max functions which aren't problematic but we can handle that separately. Also this means that these float intrinsics need to be handled by miri? At least based on the discussion elsewhere we won't be adding rust intrinsics that have the behavior of the aarch64 intrinsics.

@folkertdev folkertdev added this pull request to the merge queue Mar 10, 2026
Merged via the queue into rust-lang:main with commit 81ecf82 Mar 10, 2026
77 checks passed
@RalfJung
Copy link
Member Author

The PR also touches some integer min/max functions which aren't problematic but we can handle that separately.

Ah, sorry for that. Thanks for taking care of it!

@RalfJung RalfJung deleted the avoid-simd-minmax branch March 10, 2026 12:35
@RalfJung
Copy link
Member Author

Also this means that these float intrinsics need to be handled by miri?

Yeah as usual -- if there's a good motivation, we can add support for them.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants