intrinsics: unify rint, roundeven, nearbyint in a single round_ties_even intrinsic - #136543

RalfJung · 2025-02-04T14:23:52Z

LLVM has three intrinsics here that all do the same thing (when used in the default FP environment). There's no reason Rust needs to copy that historically-grown mess -- let's just have one intrinsic and leave it up to the LLVM backend to decide how to lower that.

Suggested by @hanna-kruppe in #136459; Cc @tgross35

try-job: test-various

rustbot · 2025-02-04T14:24:00Z

r? @ibraheemdev

rustbot has assigned @ibraheemdev.
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

rustbot · 2025-02-04T14:24:03Z

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

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, @rust-lang/wg-const-eval

The Miri subtree was changed

cc @rust-lang/miri

RalfJung · 2025-02-04T14:25:37Z

-                sym::nearbyintf32 | sym::nearbyintf64 => fx.bcx.ins().nearest(args[0]),
+                sym::round_ties_even_f32 | sym::round_ties_even_f64 => {
+                    fx.bcx.ins().nearest(args[0])
+                }


This means the cranelift backend will actually change behavior with this PR -- the round_ties_even functions will lower to the "nearest" instruction rather than to rint libcalls. @bjorn3 is that okay?

I am also surprised to not see a case for round here (rounding half-way cases away from zero); is that the one rounding mode cranelit does not support natively?

If this ever lowers to nearbyint then we do not currently provide that in libm so there may be linking problems on targets without a system libm. I don't expect this to be a problem for cranelift though.

…ven intrinsic

ibraheemdev · 2025-02-11T02:21:13Z

r? compiler

RalfJung · 2025-02-20T12:58:56Z

r? @tgross35 could you review this one?

tgross35

r=me with an answer to https://github.com/rust-lang/rust/pull/136543/files#r1941275491 (@bjorn3).

I do still think it would be nicer to emit roundeven rather than rint for GCC (possibly also cranelift if applicable) since, given the option, a libcall that doesn't touch fpenv seems cleaner. That's blocked on a few things though, so the current version lgtm.

tgross35 · 2025-02-21T20:00:57Z

 #[rustc_nounwind]
-pub unsafe fn rintf16(_x: f16) -> f16 {
+#[cfg(not(bootstrap))]
+pub unsafe fn round_ties_even_f16(_x: f16) -> f16 {


What's the latest on safe vs. unsafe intrinsics? There shouldn't be any preconditions here so it could probably be updated while you're at it (not necessary ofc).

Sure I guess I can mark this safe while I am at it, though it is strangely inconsistent with the other rounding intrinsics.

RalfJung · 2025-02-22T13:14:12Z

I do still think it would be nicer to emit roundeven rather than rint for GCC (possibly also cranelift if applicable) since, given the option, a libcall that doesn't touch fpenv seems cleaner. That's blocked on a few things though, so the current version lgtm.

This PR intentionally does not change the code we emit for f*::round_ties_even, but leaves that as a future option. :)

RalfJung · 2025-02-22T13:30:09Z

@bors r=tgross35

bors · 2025-02-22T13:30:11Z

📌 Commit d1b34ac has been approved by tgross35

It is now in the queue for this repository.

intrinsics: unify rint, roundeven, nearbyint in a single round_ties_even intrinsic LLVM has three intrinsics here that all do the same thing (when used in the default FP environment). There's no reason Rust needs to copy that historically-grown mess -- let's just have one intrinsic and leave it up to the LLVM backend to decide how to lower that. Suggested by `@hanna-kruppe` in rust-lang#136459; Cc `@tgross35`

intrinsics: unify rint, roundeven, nearbyint in a single round_ties_even intrinsic LLVM has three intrinsics here that all do the same thing (when used in the default FP environment). There's no reason Rust needs to copy that historically-grown mess -- let's just have one intrinsic and leave it up to the LLVM backend to decide how to lower that. Suggested by `@hanna-kruppe` in rust-lang#136459; Cc `@tgross35` try-job: test-various

bors · 2025-02-23T12:17:43Z

☀️ Try build successful - checks-actions
Build commit: 08959bf (08959bfe7a4e50cc5d38c0549fd2e835b2c1831a)

RalfJung · 2025-02-23T13:00:15Z

@bors r=tgross35

bors · 2025-02-23T13:00:17Z

📌 Commit b9d0555 has been approved by tgross35

It is now in the queue for this repository.

Rollup of 8 pull requests Successful merges: - rust-lang#136439 (Misc. `rustc_codegen_ssa` cleanups 🧹) - rust-lang#136543 (intrinsics: unify rint, roundeven, nearbyint in a single round_ties_even intrinsic) - rust-lang#136637 (Add binary_format to rustc target specs) - rust-lang#137099 (Fix rustdoc test directives that were accidentally ignored 🧐) - rust-lang#137297 (Update `compiler-builtins` to 0.1.147) - rust-lang#137451 (FIx `sym` -> `syn` typo in tail-expr-drop-order type opt-out) - rust-lang#137452 (bootstrap: add module docs for core:metadata) - rust-lang#137483 (rename sub_ptr to offset_from_unsigned) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#136543 - RalfJung:round-ties-even, r=tgross35 intrinsics: unify rint, roundeven, nearbyint in a single round_ties_even intrinsic LLVM has three intrinsics here that all do the same thing (when used in the default FP environment). There's no reason Rust needs to copy that historically-grown mess -- let's just have one intrinsic and leave it up to the LLVM backend to decide how to lower that. Suggested by `@hanna-kruppe` in rust-lang#136459; Cc `@tgross35` try-job: test-various

…ven`. Propagate changes from rust-lang/rust#136543 Use `__sort_of_CPROVER_round_to_integral` to model `round_ties_even`. Update tests.

Changes to attribute parsing and representation rust-lang/rust#135726 Map methods moved to `TyCtx` rust-lang/rust#137162 rust-lang/rust#137397 Remove `BackendRepr::Unihabited` rust-lang/rust#136985 Intrinsics rint, roundeven, nearbyint replaced by `round_ties_even`. rust-lang/rust#136543. Use `__sort_of_CPROVER_round_to_integral` to model `round_ties_even`. Rename `sub_ptr` to `offset_from_unsigned`. The feature gate is still `ptr_sub_ptr`. rust-lang/rust#137483. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses. --------- Co-authored-by: Remi Delmas <delmasrd@amazon.com>

rustbot assigned ibraheemdev Feb 4, 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 Feb 4, 2025

RalfJung force-pushed the round-ties-even branch from 7d67210 to 6015136 Compare February 4, 2025 14:24

RalfJung commented Feb 4, 2025

View reviewed changes

RalfJung force-pushed the round-ties-even branch from 6015136 to 6f39172 Compare February 4, 2025 14:28

RalfJung mentioned this pull request Feb 4, 2025

Change rint to roundeven in round_ties_even implementation #136459

Open

This comment has been minimized.

Sign in to view

RalfJung force-pushed the round-ties-even branch from 6f39172 to 7d04265 Compare February 4, 2025 14:59

This comment has been minimized.

Sign in to view

intrinsics: unify rint, roundeven, nearbyint in a single round_ties_e…

04e7a10

…ven intrinsic

RalfJung force-pushed the round-ties-even branch from 7d04265 to 04e7a10 Compare February 4, 2025 15:27

rustbot assigned wesleywiser and unassigned ibraheemdev Feb 11, 2025

rustbot assigned tgross35 and unassigned wesleywiser Feb 20, 2025

tgross35 reviewed Feb 21, 2025

View reviewed changes

make the new intrinsics safe

d1b34ac

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 Feb 22, 2025

matthiaskrgr mentioned this pull request Feb 22, 2025

Rollup of 8 pull requests #137431

Closed

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 23, 2025

tgross35 mentioned this pull request Feb 23, 2025

Rollup of 8 pull requests #137497

Merged

bors merged commit a2bb4d7 into rust-lang:master Feb 24, 2025

rustbot added this to the 1.87.0 milestone Feb 24, 2025

RalfJung deleted the round-ties-even branch February 24, 2025 07:51

carolynzech mentioned this pull request Feb 27, 2025

Update toolchain to 2025-03-02 model-checking/kani#3911

Merged

RalfJung mentioned this pull request Mar 7, 2025

Miri build preventing landing changes which require both lib and compiler work #138065

Closed

Uh oh!

Uh oh!

Conversation

RalfJung commented Feb 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Feb 4, 2025

Uh oh!

rustbot commented Feb 4, 2025

Uh oh!

RalfJung Feb 4, 2025 • 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!

tgross35 Feb 5, 2025 • 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!

This comment has been minimized.

This comment has been minimized.

ibraheemdev commented Feb 11, 2025

Uh oh!

RalfJung commented Feb 20, 2025

Uh oh!

tgross35 left a comment

Choose a reason for hiding this comment

Uh oh!

tgross35 Feb 21, 2025

Choose a reason for hiding this comment

Uh oh!

RalfJung Feb 22, 2025

Choose a reason for hiding this comment

Uh oh!

RalfJung commented Feb 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RalfJung commented Feb 22, 2025

Uh oh!

bors commented Feb 22, 2025

Uh oh!

bors commented Feb 23, 2025

Uh oh!

RalfJung commented Feb 23, 2025

Uh oh!

bors commented Feb 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

RalfJung commented Feb 4, 2025 •

edited

Loading

RalfJung Feb 4, 2025 •

edited

Loading

tgross35 Feb 5, 2025 •

edited

Loading

RalfJung commented Feb 22, 2025 •

edited

Loading