Make min/max and copysign intrinsics generic by N1ark · Pull Request #162414 · rust-lang/rust

N1ark · 2026-09-07T10:24:24Z

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 · 2026-09-07T10:24:28Z

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 · 2026-09-07T10:24:31Z

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

RalfJung · 2026-09-07T10:27:10Z

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 · 2026-09-07T10:33:32Z

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 ^^'

RalfJung · 2026-09-07T11:52:02Z

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 ^^'

IMO it makes sense -- in both cases it's a fallback body that we don't want to use for const-eval, just the reason for not using it is different.

RalfJung · 2026-09-07T11:53:06Z

CI is red. Also please add the same kind of test as in your other PR. Yeah that means they will conflict -- sorry for that. Shouldn't be too hard of a conflict to deal with I hope.

@rustbot author

N1ark · 2026-09-07T12:13:07Z

de-constified the trait, as done in #162409, and switched to #[rustc_do_not_const_check]

RalfJung

LGTM on the Rust + LLVM side, but I can't review the other codegen backends. Looks like the GCC changes already got approved -- @bjorn3 could you look at the cranelift changes?

View changes since this review

RalfJung · 2026-09-07T14:12:37Z

-    } else if y < x {
+#[rustc_do_not_const_check]
+pub const fn maximum_number_nsz<T: bounds::FloatPrimitive>(x: T, y: T) -> T {
+    if x.is_nan() || y >= x {


FWIW this would have been easier to review if you had kept the original order of the intrinsics. Please keep that in mind when writing a PR that someone else is expected to review -- never reorder and change code at the same time, git can't really deal with that.

RalfJung · 2026-09-07T14:14:41Z

 /// Therefore, implementations must not require the user to uphold
 /// any safety invariants.
 #[rustc_nounwind]
+#[rustc_intrinsic_const_stable_indirect]


Is this attribute needed? It was not needed before...

Well it will be needed with my PR, so let's keep it. :) Just curious why you added it here as I didn't think anything would force you to add it before my PR.

Process-wise, adding the attribute is covered by the FCP in #130843 (comment).

No wait, that was for min/maximum_number_nsz. This here is min/maximum which is not yet stable. It really shouldn't get the attribute then.

Add #[rustc_const_unstable] instead.

RalfJung · 2026-09-07T15:52:34Z

+/// Used to distinguish fallbacks of float intrinsics. For some we have a codegen fallback,
+/// while for others we fallback to an external libcall.
+enum IntrinsicFallback {
+    Fallback(&'static str),


So this does not refer to a Rust fallback body? Please pick a different name then, this is confusing. Maybe Libcall or so?

Or, given that this is pre-existing, at least please add a comment.

I think all cases which use IntrinsicFallback::Fallback should be able to be removed and just use the Rust fallback body? In which case adding a FIXME here to remove it in a future PR seems like a good idea.

rust-bors · 2026-09-08T00:46:32Z

☔ The latest upstream changes (presumably #162445) made this pull request unmergeable. Please resolve the merge conflicts by rebasing.

RalfJung · 2026-09-08T07:48:59Z

+// Check that the float intrinsics carrying `#[rustc_do_not_const_check]` can actually be called in
+// a const context, for every float width. Their bodies are never const-checked and never run by
+// const-eval, which has to implement each of these intrinsics itself.


FWIW there's no good reason to only cover rustc_do_not_const_check intrinsics here. All const intrinsics without const-capable fallback body should ideally be covered somewhere. Maybe we already test some of them somewhere? I don't know.

View changes since the review

bjorn3 · 2026-09-09T10:37:12Z

+                    let arg = AbiParam::new(ty);
+                    fx.lib_call(name, vec![arg, arg], vec![arg], &[x, y])[0]
+                }
+            };


The fallback variant is never used here.

View changes since the review

N1ark added 4 commits September 7, 2026 10:46

cranelift: use fallbacks for abs f16, f128

0dcb734

Make copysign generic

7b86261

Make minimum/maximum generic

78c2b96

Add rustc_allow_const_fn_unstable to fabs

a707a46

rustbot added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label Sep 7, 2026

rustbot assigned RalfJung Sep 7, 2026

N1ark mentioned this pull request Sep 7, 2026

Make more float intrinsics generic #162395

Open

beetrees requested changes Sep 7, 2026

View reviewed changes

Comment thread library/core/src/intrinsics/bounds.rs Outdated

This comment has been minimized.

Sign in to view

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 7, 2026

N1ark added 3 commits September 7, 2026 13:12

Replace float bound impls with a macro

c63c387

De-constify the FloatPrimitive sealed trait

a4edfff

Add test

101eb51

N1ark force-pushed the const-unstable-float-generics branch from 1514b9c to 101eb51 Compare September 7, 2026 12:12

antoyo reviewed Sep 7, 2026

View reviewed changes

Comment thread compiler/rustc_codegen_gcc/src/intrinsic/mod.rs

RalfJung approved these changes Sep 7, 2026

View reviewed changes

RalfJung reviewed Sep 8, 2026

View reviewed changes

bjorn3 reviewed Sep 9, 2026

View reviewed changes

Uh oh!

Uh oh!

Conversation

N1ark commented Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Sep 7, 2026

Uh oh!

rustbot commented Sep 7, 2026

Uh oh!

RalfJung commented Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

N1ark commented Sep 7, 2026

Uh oh!

Uh oh!

This comment has been minimized.

RalfJung commented Sep 7, 2026

Uh oh!

RalfJung commented Sep 7, 2026

Uh oh!

N1ark commented Sep 7, 2026

Uh oh!

Uh oh!

RalfJung left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RalfJung Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

RalfJung Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

RalfJung Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

RalfJung Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

RalfJung Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RalfJung Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

beetrees Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

rust-bors Bot commented Sep 8, 2026

Uh oh!

RalfJung Sep 8, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bjorn3 Sep 9, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

N1ark commented Sep 7, 2026 •

edited

Loading

RalfJung commented Sep 7, 2026 •

edited

Loading

RalfJung left a comment •

edited by rustbot

Loading

RalfJung Sep 8, 2026 •

edited

Loading

RalfJung Sep 8, 2026 •

edited by rustbot

Loading

bjorn3 Sep 9, 2026 •

edited by rustbot

Loading