Skip to content

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented May 30, 2025

Tracking issue: #141572

Based on top of #141521; only the last two commits are new.

r? @tgross35

ruancomelli and others added 2 commits May 29, 2025 13:50
Add const support for the float rounding methods floor, ceil, trunc,
fract, round and round_ties_even.
This works by moving the calculation logic from

     src/tools/miri/src/intrinsics/mod.rs

into

     compiler/rustc_const_eval/src/interpret/intrinsics.rs.

All relevant method definitions were adjusted to include the `const`
keyword for all supported float types: f16, f32, f64 and f128.

The constness is hidden behind the feature gate

     feature(const_float_round_methods)

which is tracked in

     rust-lang#141555

This commit is a squash of the following commits:
- test: add tests that we expect to pass when float rounding becomes const
- feat: make float rounding methods `const`
- fix: replace `rustc_allow_const_fn_unstable(core_intrinsics)` attribute with `#[rustc_const_unstable(feature = "f128", issue = "116909")]` in `library/core/src/num/f128.rs`
- revert: undo update to `library/stdarch`
- refactor: replace multiple `float_<mode>_intrinsic` rounding methods with a single, parametrized one
- fix: add `#[cfg(not(bootstrap))]` to new const method tests
- test: add extra sign tests to check `+0.0` and `-0.0`
- revert: undo accidental changes to `round` docs
- fix: gate `const` float round method behind `const_float_round_methods`
- fix: remove unnecessary `#![feature(const_float_methods)]`
- fix: remove unnecessary `#![feature(const_float_methods)]` [2]
- revert: undo changes to `tests/ui/consts/const-eval/float_methods.rs`
- fix: adjust after rebase
- test: fix float tests
- test: add tests for `fract`
- chore: add commented-out `const_float_round_methods` feature gates to `f16` and `f128`
- fix: adjust NaN when rounding floats
- chore: add FIXME comment for de-duplicating float tests
- test: remove unnecessary test file `tests/ui/consts/const-eval/float_methods.rs`
- test: fix tests after upstream simplification of how float tests are run
@rustbot 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 30, 2025
@rustbot
Copy link
Collaborator

rustbot commented May 30, 2025

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

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

@RalfJung RalfJung changed the title Const float euclidean division constify float::{div,rem}_euclid May 30, 2025
@RalfJung RalfJung force-pushed the const_float_euclidean_division branch from 404cbac to 1df3a8c Compare May 30, 2025 07:18
@RalfJung RalfJung added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label May 30, 2025
@tgross35
Copy link
Contributor

Unfortunately I'm not sure if we should do this yet; these methods don't do correct rounding and I think we want to fix that first, which #134145 proposes to do. It would be difficult to keep constness through that change, and IMO they should be in libm for testing (rather than in-crate as that PR proposes).

I take it you have a use for these? :)

@RalfJung
Copy link
Member Author

I take it you have a use for these? :)

No, I was just bothered by the fact that one of the tests in this big macro in library/coretests/tests/num/mod.rs doesn't use $fassert! and hence runs the same tests twice.^^

@RalfJung
Copy link
Member Author

It would be difficult to keep constness through that change, and IMO they should be in libm for testing (rather than in-crate as that PR proposes).

That sounds like they would become intrinsics and we could have them in const -- but that would require a softfloat implementation in const-eval.

@tgross35
Copy link
Contributor

If it's eventually in libm I think we would likely be able to use the generic implementations to do that. I just did a bit of a scratch test using apfloat in our generic sqrt which seems to work, modulo some probably-fixable rounding bugs in f64 and f128 rust-lang/compiler-builtins@54ef3d9. (Not that sqrt itself is all that useful given Miri has an implementation).

@RalfJung
Copy link
Member Author

Well I guess we shouldn't constify it yet then, and the issue with the tests is already tracked in #141726.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-blocked Status: Blocked on something else such as an RFC or other implementation work. 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants