Use intrinsics for `{f16,f32,f64,f128}::{minimum,maximum}` operations #140792

Urgau · 2025-05-08T10:56:07Z

This PR creates intrinsics for {f16,f32,f64,f64}::{minimum,maximum} operations.

This wasn't done when those operations were added as the LLVM support was too weak but now that LLVM has libcalls for unsupported platforms we can finally use them.

Cranelift and GCC¹ support are partial, Cranelift doesn't support f16 and f128, while GCC doesn't support f16.

r? @tgross35

try-job: aarch64-gnu
try-job: dist-various-1
try-job: dist-various-2

https://www.gnu.org/software///gnulib/manual/html_node/Functions-in-_003cmath_002eh_003e.html ↩

rustbot · 2025-05-08T10:56:13Z

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

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

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

compiler/rustc_const_eval/src/interpret/intrinsics.rs

library/core/src/intrinsics/mod.rs

RalfJung · 2025-05-08T12:11:48Z

library/core/src/num/f16.rs

+            if self > other {
+                self
+            } else if other > self {
+                other
+            } else if self == other {
+                if self.is_sign_positive() && other.is_sign_negative() { self } else { other }
+            } else {
+                self + other
+            }


Would it be worth preserving this as "fallback" implementation (by giving the intrinsic a body)? Then const-eval wouldn't need an implementation at all, and also cranelift and GCC would not need any extra work.

We could, but it's not currently done min and max, @tgross35 was that intentional?

min and max are way older than our ability to have fallback bodies.

Yes, but as part of adding f16 and f128 we could have added a fallback, but it wasn't done. Hence my question if it was intentional that fallback bodies haven't been added.

Also the minnum16 and minnum128 intrinsics are quite recent, idk if we had already had fallback body.

Even the f16/f128 ones might be older than fallback bodies. Also they were obviously just made to be consistent with the f32/f64 ones (hence the terrible name...).

I have added a fallback for {minimum,maximum}{16,128}, GCC and Cranelift should have full support.

I didn't touched const-eval as we have the necessary support in rustc_apfloat.

We could, but it's not currently done min and max, @tgross35 was that intentional?

To follow up, when I did the original implementation I tried to keep things as similar as possible to the existing float types. Maybe it makes sense to update those later?

Yeah, the more things that can have fallback MIR the better -- it means they work for out-of-tree backends too, for example. (And might help tools like kani too, to not have to manually handle them in verifiers.)

As was mentioned, it's just that for a long time it wasn't possible to give things fallback MIR, so there's a big backlog.

compiler/rustc_codegen_gcc/src/intrinsic/mod.rs

tgross35 · 2025-05-09T12:12:10Z

library/core/src/num/f16.rs

+            if self > other {
+                self
+            } else if other > self {
+                other
+            } else if self == other {
+                if self.is_sign_positive() && other.is_sign_negative() { self } else { other }
+            } else {
+                self + other
+            }


We could, but it's not currently done min and max, @tgross35 was that intentional?

To follow up, when I did the original implementation I tried to keep things as similar as possible to the existing float types. Maybe it makes sense to update those later?

library/core/src/num/f128.rs

compiler/rustc_codegen_gcc/src/intrinsic/mod.rs

library/core/src/intrinsics/mod.rs

RalfJung · 2025-05-09T13:34:48Z

@rust-lang/lang Never mind, turns out the functions are not actually stable, so we definitely don't need an FCP then. :)

compiler/rustc_codegen_gcc/src/intrinsic/mod.rs

library/core/src/num/f32.rs

library/core/src/intrinsics/mod.rs

scottmcm

This looks good to me.

traviscross

LGTM.

library/core/src/intrinsics/mod.rs

tgross35

r=me as well :)

Urgau · 2025-05-09T19:59:54Z

@bors r=scottmcm,traviscross,tgross35

bors · 2025-05-09T19:59:57Z

📌 Commit dc69020 has been approved by scottmcm,traviscross,tgross35

It is now in the queue for this repository.

…r=scottmcm,traviscross,tgross35 Use intrinsics for `{f16,f32,f64,f128}::{minimum,maximum}` operations This PR creates intrinsics for `{f16,f32,f64,f64}::{minimum,maximum}` operations. This wasn't done when those operations were added as the LLVM support was too weak but now that LLVM has libcalls for unsupported platforms we can finally use them. Cranelift and GCC[^1] support are partial, Cranelift doesn't support `f16` and `f128`, while GCC doesn't support `f16`. r? `@tgross35` [^1]: https://www.gnu.org/software///gnulib/manual/html_node/Functions-in-_003cmath_002eh_003e.html

Rollup of 10 pull requests Successful merges: - rust-lang#129334 (Implement (part of) ACP 429: add `DerefMut` to `Lazy[Cell/Lock]`) - rust-lang#135015 (Partially stabilize LoongArch target features) - rust-lang#138736 (Sanitizers target modificators) - rust-lang#139562 (rustdoc: add a handle that makes sidebar resizing more obvious) - rust-lang#140151 (remove intrinsics::drop_in_place) - rust-lang#140660 (remove 'unordered' atomic intrinsics) - rust-lang#140783 (Update documentation of OnceLock::get_or_init.) - rust-lang#140789 (Update hermit-abi to 0.5.1) - rust-lang#140792 (Use intrinsics for `{f16,f32,f64,f128}::{minimum,maximum}` operations) - rust-lang#140862 (Enable non-leaf Frame Pointers for Arm64EC Windows) r? `@ghost` `@rustbot` modify labels: rollup

…r=scottmcm,traviscross,tgross35 Use intrinsics for `{f16,f32,f64,f128}::{minimum,maximum}` operations This PR creates intrinsics for `{f16,f32,f64,f64}::{minimum,maximum}` operations. This wasn't done when those operations were added as the LLVM support was too weak but now that LLVM has libcalls for unsupported platforms we can finally use them. Cranelift and GCC[^1] support are partial, Cranelift doesn't support `f16` and `f128`, while GCC doesn't support `f16`. r? `@tgross35` [^1]: https://www.gnu.org/software///gnulib/manual/html_node/Functions-in-_003cmath_002eh_003e.html

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#129334 (Implement (part of) ACP 429: add `DerefMut` to `Lazy[Cell/Lock]`) - rust-lang#139562 (rustdoc: add a handle that makes sidebar resizing more obvious) - rust-lang#140151 (remove intrinsics::drop_in_place) - rust-lang#140660 (remove 'unordered' atomic intrinsics) - rust-lang#140783 (Update documentation of OnceLock::get_or_init.) - rust-lang#140789 (Update hermit-abi to 0.5.1) - rust-lang#140792 (Use intrinsics for `{f16,f32,f64,f128}::{minimum,maximum}` operations) - rust-lang#140879 (1.87.0 release notes: remove nonsensical `~` operator) r? `@ghost` `@rustbot` modify labels: rollup

matthiaskrgr · 2025-05-10T14:25:31Z

@bors r-
#140890 (comment)

Urgau · 2025-05-10T15:37:20Z

@bors try

bors · 2025-05-10T15:38:31Z

⌛ Trying commit 7f0ae5e with merge b8a72228198170e8c3dc3df531635c7bf11c92ae...

…<try> Use intrinsics for `{f16,f32,f64,f128}::{minimum,maximum}` operations This PR creates intrinsics for `{f16,f32,f64,f64}::{minimum,maximum}` operations. This wasn't done when those operations were added as the LLVM support was too weak but now that LLVM has libcalls for unsupported platforms we can finally use them. Cranelift and GCC[^1] support are partial, Cranelift doesn't support `f16` and `f128`, while GCC doesn't support `f16`. r? `@tgross35` try-job: aarch64-gnu try-job: dist-various-1 try-job: dist-various-2 [^1]: https://www.gnu.org/software///gnulib/manual/html_node/Functions-in-_003cmath_002eh_003e.html

rustbot assigned tgross35 May 8, 2025

rustbot added 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 May 8, 2025

This comment has been minimized.

Sign in to view

Urgau force-pushed the minimum-maximum-intrinsics branch from 360cdaf to f308845 Compare May 8, 2025 11:09

RalfJung reviewed May 8, 2025

View reviewed changes

compiler/rustc_const_eval/src/interpret/intrinsics.rs Outdated Show resolved Hide resolved

RalfJung reviewed May 8, 2025

View reviewed changes

library/core/src/intrinsics/mod.rs Show resolved Hide resolved

RalfJung reviewed May 8, 2025

View reviewed changes

antoyo reviewed May 8, 2025

View reviewed changes

compiler/rustc_codegen_gcc/src/intrinsic/mod.rs Outdated Show resolved Hide resolved

Urgau force-pushed the minimum-maximum-intrinsics branch from f308845 to 8b600e5 Compare May 8, 2025 13:39

Urgau changed the title ~~Use intrinsics for {f16,f32,f64,f64}::{minimum,maximum} operations~~ Use intrinsics for {f16,f32,f64,f128}::{minimum,maximum} operations May 8, 2025

tgross35 reviewed May 9, 2025

View reviewed changes

RalfJung reviewed May 9, 2025

View reviewed changes

library/core/src/intrinsics/mod.rs Outdated Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

Urgau force-pushed the minimum-maximum-intrinsics branch from 96b7a3d to f63c9d7 Compare May 9, 2025 13:44

antoyo reviewed May 9, 2025

View reviewed changes

compiler/rustc_codegen_gcc/src/intrinsic/mod.rs Outdated Show resolved Hide resolved

RalfJung reviewed May 9, 2025

View reviewed changes

library/core/src/num/f32.rs Outdated Show resolved Hide resolved

Urgau force-pushed the minimum-maximum-intrinsics branch from f63c9d7 to 7d53e31 Compare May 9, 2025 14:25

This comment has been minimized.

Sign in to view

Urgau force-pushed the minimum-maximum-intrinsics branch from 7d53e31 to ca9dfb2 Compare May 9, 2025 14:48

This comment has been minimized.

Sign in to view

Urgau force-pushed the minimum-maximum-intrinsics branch from ca9dfb2 to 5482ba9 Compare May 9, 2025 15:00

This comment has been minimized.

Sign in to view

Use intrinsics for {f16,f32,f64,f128}::{minimum,maximum} operations

e7247df

Urgau force-pushed the minimum-maximum-intrinsics branch from 5482ba9 to 4a9f2b4 Compare May 9, 2025 15:17

scottmcm reviewed May 9, 2025

View reviewed changes

library/core/src/intrinsics/mod.rs Outdated Show resolved Hide resolved

Urgau force-pushed the minimum-maximum-intrinsics branch from 4a9f2b4 to a6b6316 Compare May 9, 2025 17:42

scottmcm approved these changes May 9, 2025

View reviewed changes

traviscross approved these changes May 9, 2025

View reviewed changes

bjorn3 reviewed May 9, 2025

View reviewed changes

library/core/src/intrinsics/mod.rs Outdated Show resolved Hide resolved

Add intrinsic fallback for {minimum,maximum}{16,32,64,128}

dc69020

Urgau force-pushed the minimum-maximum-intrinsics branch from a6b6316 to dc69020 Compare May 9, 2025 19:00

tgross35 approved these changes May 9, 2025

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 9, 2025

Zalathar mentioned this pull request May 10, 2025

Rollup of 10 pull requests #140885

Closed

matthiaskrgr mentioned this pull request May 10, 2025

Rollup of 8 pull requests #140890

Closed

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 10, 2025

Use the fallback body for {minimum,maximum}f128 on LLVM as well.

7f0ae5e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use intrinsics for `{f16,f32,f64,f128}::{minimum,maximum}` operations #140792

Use intrinsics for `{f16,f32,f64,f128}::{minimum,maximum}` operations #140792

Urgau commented May 8, 2025 •

edited

Loading

rustbot commented May 8, 2025

This comment has been minimized.

RalfJung May 8, 2025 •

edited

Loading

Urgau May 8, 2025

RalfJung May 8, 2025

Urgau May 8, 2025 •

edited

Loading

RalfJung May 8, 2025 •

edited

Loading

Urgau May 8, 2025

tgross35 May 9, 2025

scottmcm May 9, 2025 •

edited

Loading

tgross35 May 9, 2025

This comment was marked as resolved.

RalfJung commented May 9, 2025 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

scottmcm left a comment

traviscross left a comment

tgross35 left a comment

Urgau commented May 9, 2025

bors commented May 9, 2025

matthiaskrgr commented May 10, 2025

Urgau commented May 10, 2025

bors commented May 10, 2025

Use intrinsics for {f16,f32,f64,f128}::{minimum,maximum} operations #140792

Are you sure you want to change the base?

Use intrinsics for {f16,f32,f64,f128}::{minimum,maximum} operations #140792

Conversation

Urgau commented May 8, 2025 • edited Loading

Footnotes

rustbot commented May 8, 2025

This comment has been minimized.

RalfJung May 8, 2025 • edited Loading

Choose a reason for hiding this comment

Urgau May 8, 2025

Choose a reason for hiding this comment

RalfJung May 8, 2025

Choose a reason for hiding this comment

Urgau May 8, 2025 • edited Loading

Choose a reason for hiding this comment

RalfJung May 8, 2025 • edited Loading

Choose a reason for hiding this comment

Urgau May 8, 2025

Choose a reason for hiding this comment

tgross35 May 9, 2025

Choose a reason for hiding this comment

scottmcm May 9, 2025 • edited Loading

Choose a reason for hiding this comment

tgross35 May 9, 2025

Choose a reason for hiding this comment

This comment was marked as resolved.

RalfJung commented May 9, 2025 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

scottmcm left a comment

Choose a reason for hiding this comment

traviscross left a comment

Choose a reason for hiding this comment

tgross35 left a comment

Choose a reason for hiding this comment

Urgau commented May 9, 2025

bors commented May 9, 2025

matthiaskrgr commented May 10, 2025

Urgau commented May 10, 2025

bors commented May 10, 2025

Use intrinsics for `{f16,f32,f64,f128}::{minimum,maximum}` operations #140792

Use intrinsics for `{f16,f32,f64,f128}::{minimum,maximum}` operations #140792

Urgau commented May 8, 2025 •

edited

Loading

RalfJung May 8, 2025 •

edited

Loading

Urgau May 8, 2025 •

edited

Loading

RalfJung May 8, 2025 •

edited

Loading

scottmcm May 9, 2025 •

edited

Loading

RalfJung commented May 9, 2025 •

edited

Loading