Skip to content

constify float::{div,rem}_euclid #141755

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

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