Skip to content

simd_reduce_min/max: remove float support#155189

Open
RalfJung wants to merge 1 commit intorust-lang:mainfrom
RalfJung:reduce-minmax-float
Open

simd_reduce_min/max: remove float support#155189
RalfJung wants to merge 1 commit intorust-lang:mainfrom
RalfJung:reduce-minmax-float

Conversation

@RalfJung
Copy link
Copy Markdown
Member

@RalfJung RalfJung commented Apr 12, 2026

LLVM currently doesn't have an intrinsic with the right semantics here (see llvm/llvm-project#185827). The only remaining user of this intrinsic with float types is portable-simd and it's easier to implement a fallback there than here, so I opted for making the intrinsic int-only. I kept around the float support in cranelift and Miri because there it already has the desired semantics (matching scalar min/max).

Fixes #153395
Blocked on rust-lang/portable-simd#515

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 12, 2026

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

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

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

@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 Apr 12, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 12, 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 12 candidates

acc = self.select(cmp, acc, elem);
}
acc
}
Copy link
Copy Markdown
Member Author

@RalfJung RalfJung Apr 12, 2026

Choose a reason for hiding this comment

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

I removed the GCC code because it seems completely wrong under any conceivable semantics. For an input of [0.0, NaN, 1.0, 2.0] this would return 1.0!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you. T_T


/// Returns the maximum element of a vector.
///
/// `T` must be a vector of integers or floats.
Copy link
Copy Markdown
Member

@chenyukang chenyukang Apr 12, 2026

Choose a reason for hiding this comment

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

This comment below still have

/// For floating-point values, uses IEEE-754 `maxNum`.`

which maybe need to be removed?

there are several similar places.

@rust-log-analyzer

This comment has been minimized.

@chenyukang
Copy link
Copy Markdown
Member

@chenyukang
Copy link
Copy Markdown
Member

need more reviewers from the domain
@rustbot reroll

@rustbot rustbot assigned petrochenkov and unassigned chenyukang Apr 12, 2026
@RalfJung
Copy link
Copy Markdown
Member Author

@RalfJung RalfJung force-pushed the reduce-minmax-float branch from d7ab695 to 1df099e Compare April 12, 2026 10:19
@RalfJung RalfJung force-pushed the reduce-minmax-float branch from 1df099e to c77f465 Compare April 12, 2026 10:20
@rust-log-analyzer
Copy link
Copy Markdown
Collaborator

The job pr-check-2 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)
test library/core/src/unit.rs - unit::() (line 6) ... ok

failures:

---- library/core/src/../../portable-simd/crates/core_simd/src/simd/num/float.rs - core_simd::simd::num::float::SimdFloat::reduce_max (line 194) stdout ----
error[E0511]: invalid monomorphization of `simd_reduce_max` intrinsic: unsupported simd_reduce_max from `Simd<f32, 2>` with element `f32` to `f32`
   --> library/core/src/../../portable-simd/crates/core_simd/src/simd/num/float.rs:434:26
    |
434 |                 unsafe { core::intrinsics::simd::simd_reduce_max(self) }
    |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
447 | impl_trait! { f16 { bits: u16, mask: i16 }, f32 { bits: u32, mask: i32 }, f64 { bits: u64, mask: i64 } }
    | -------------------------------------------------------------------------------------------------------- in this macro invocation
    |
    = note: this error originates in the macro `impl_trait` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0511`.
Couldn't compile the test.
\ (no newline at end of output)
---- library/core/src/../../portable-simd/crates/core_simd/src/simd/num/float.rs - core_simd::simd::num::float::SimdFloat::reduce_max (line 194) stdout end ----
---- library/core/src/../../portable-simd/crates/core_simd/src/simd/num/float.rs - core_simd::simd::num::float::SimdFloat::reduce_min (line 221) stdout ----
error[E0511]: invalid monomorphization of `simd_reduce_min` intrinsic: unsupported simd_reduce_min from `Simd<f32, 2>` with element `f32` to `f32`
   --> library/core/src/../../portable-simd/crates/core_simd/src/simd/num/float.rs:440:26
    |
440 |                 unsafe { core::intrinsics::simd::simd_reduce_min(self) }
    |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
447 | impl_trait! { f16 { bits: u16, mask: i16 }, f32 { bits: u32, mask: i32 }, f64 { bits: u64, mask: i64 } }
    | -------------------------------------------------------------------------------------------------------- in this macro invocation
    |
    = note: this error originates in the macro `impl_trait` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 1 previous error

@petrochenkov petrochenkov added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 13, 2026
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-blocked Status: Blocked on something else such as an RFC or other implementation work. 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.

What are the intended semantics for simd_fmin/fmax and simd_reduce_min/max vs signaling NaN?

6 participants