Skip to content

Make min/max and copysign intrinsics generic - #162414

Open
N1ark wants to merge 5 commits into
rust-lang:mainfrom
N1ark:const-unstable-float-generics
Open

Make min/max and copysign intrinsics generic#162414
N1ark wants to merge 5 commits into
rust-lang:mainfrom
N1ark:const-unstable-float-generics

Conversation

@N1ark

@N1ark N1ark commented Sep 7, 2026

Copy link
Copy Markdown
Member

Split off from #162395

Make the following intrinsics generic over the float type:

  • copysign, with a shared fallback (bit manipulations). Note that this required adding #[rustc_const_unstable] to the whole intrinsic, which copysignf16 and copysignf128 didn't have. This was already FCP'd, see rust-lang/rust#153834 (comment).
  • minimum_number_nsz, maximum_number_nsz, minimum, maximum, with shared fallbacks.

Because these are generic and require const trait impls to have a fallback, I had to add #[rustc_allow_const_fn_unstable] to them:

  • copysign needs const_ops, const_trait_impl
  • minimum_number_nsz and maximum_number_nsz need const_cmp, const_trait_impl
  • minimum and maximum need const_cmp, const_ops, const_trait_impl

I have also bundled in the addition of the attribute to fabs, which used these features without having a formal approval (see #162409 -- this was introduced in #153834)

See #162395 (comment)

r? RalfJung

@rustbot

rustbot commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

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

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

cc @bjorn3

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

cc @rust-lang/clippy

⚠️ #[miri::intrinsic_fallback_is_spec] must only be used if the function actively checks for all UB cases,
and explores the possible non-determinism of the intrinsic.

cc @rust-lang/miri

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

cc @antoyo, @GuillaumeGomez

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Any special-casing of Miri in the standard library requires review.

cc @rust-lang/miri

⚠️ #[rustc_intrinsic_const_stable_indirect] controls whether intrinsics can be exposed to stable const
code; adding it needs t-lang approval.

cc @rust-lang/wg-const-eval

⚠️ #[rustc_allow_const_fn_unstable] needs careful audit to avoid accidentally exposing unstable
implementation details on stable.

cc @rust-lang/wg-const-eval

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

@rustbot rustbot added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label Sep 7, 2026
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-clippy Relevant to the Clippy team. 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 Sep 7, 2026
@rustbot

rustbot commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

RalfJung is not on the review rotation at the moment.
They may take a while to respond.

@RalfJung

RalfJung commented Sep 7, 2026

Copy link
Copy Markdown
Member

Because these are generic and require const trait impls to have a fallback, I had to add #[rustc_allow_const_fn_unstable] to them:

I just realized that we can maybe just use rustc_do_not_const_check for all of them? They all have built-in implementations right?

@N1ark

N1ark commented Sep 7, 2026

Copy link
Copy Markdown
Member Author

doesn't that feel a bit hackier / risk prone that just doing it like this? though i don't mind, we are calling externs from const code in other fallbacks after all ^^'

Comment thread library/core/src/intrinsics/bounds.rs Outdated
Comment on lines 62 to 164
@@ -66,6 +71,18 @@ const unsafe impl FloatPrimitive for f16 {
fn from_bits(bits: Self::UInt) -> Self {
f16::from_bits(bits)
}
#[inline]
fn is_nan(self) -> bool {
f16::is_nan(self)
}
#[inline]
fn is_sign_positive(self) -> bool {
f16::is_sign_positive(self)
}
#[inline]
fn is_sign_negative(self) -> bool {
f16::is_sign_negative(self)
}
}

#[rustc_const_unstable(feature = "core_intrinsics", issue = "none")]
@@ -80,6 +97,18 @@ const unsafe impl FloatPrimitive for f32 {
fn from_bits(bits: Self::UInt) -> Self {
f32::from_bits(bits)
}
#[inline]
fn is_nan(self) -> bool {
f32::is_nan(self)
}
#[inline]
fn is_sign_positive(self) -> bool {
f32::is_sign_positive(self)
}
#[inline]
fn is_sign_negative(self) -> bool {
f32::is_sign_negative(self)
}
}

#[rustc_const_unstable(feature = "core_intrinsics", issue = "none")]
@@ -94,6 +123,18 @@ const unsafe impl FloatPrimitive for f64 {
fn from_bits(bits: Self::UInt) -> Self {
f64::from_bits(bits)
}
#[inline]
fn is_nan(self) -> bool {
f64::is_nan(self)
}
#[inline]
fn is_sign_positive(self) -> bool {
f64::is_sign_positive(self)
}
#[inline]
fn is_sign_negative(self) -> bool {
f64::is_sign_negative(self)
}
}

#[rustc_const_unstable(feature = "core_intrinsics", issue = "none")]
@@ -108,6 +149,18 @@ const unsafe impl FloatPrimitive for f128 {
fn from_bits(bits: Self::UInt) -> Self {
f128::from_bits(bits)
}
#[inline]
fn is_nan(self) -> bool {
f128::is_nan(self)
}
#[inline]
fn is_sign_positive(self) -> bool {
f128::is_sign_positive(self)
}
#[inline]
fn is_sign_negative(self) -> bool {
f128::is_sign_negative(self)
}
}

@beetrees beetrees Sep 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since this is repeating the same thing 4 times, I think it would be tidier as a macro. Something like:

Suggested change
macro_rules! impl_float_primitive {
(unsafe $($float:ident => $bits:ident),+) => {
#[rustc_const_unstable(feature = "core_intrinsics", issue = "none")]
const unsafe impl FloatPrimitive for $float {
type UInt = $bits;
const SIGN_MASK: Self::UInt = Self::SIGN_MASK;
#[inline]
fn to_bits(self) -> Self::UInt {
Self::to_bits(self)
}
#[inline]
fn from_bits(bits: Self::UInt) -> Self {
Self::from_bits(bits)
}
#[inline]
fn is_nan(self) -> bool {
Self::is_nan(self)
}
#[inline]
fn is_sign_positive(self) -> bool {
Self::is_sign_positive(self)
}
#[inline]
fn is_sign_negative(self) -> bool {
Self::is_sign_negative(self)
}
}
};
}
impl_float_primitive!(unsafe f16 => u16, f32 => u32, f64 => u64, f128 => u128);

View changes since the review

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

true! originally we had agreed to not use a macro but given it's growing a bit and there's a macro for ints right below this makes sense; done in 1514b9c

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-clippy Relevant to the Clippy team. 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.

4 participants