Add `f16` and `f128` const eval for binary and unary operationations by tgross35 · Pull Request #126429 · rust-lang/rust

tgross35 · 2024-06-13T19:29:16Z

Add const evaluation and Miri support for f16 and f128, including unary and binary operations. Casts are not yet included.

Fixes #124583

r? @RalfJung

tgross35 · 2024-06-13T19:33:56Z

src/tools/miri/tests/pass/float.rs is really tedious to update and I'm not entirely clear what a lot of the magic numbers are intended to test. @RalfJung do you have any suggestions about how this could be cleaned up or made generic to some extent? Compiler-builtins uses a bit of fuzzing + property testing to compare the builtins function to the system function or apfloat https://github.com/rust-lang/compiler-builtins/blob/c04eb9e1afb72bdf943f5e5d77b3812f40526602/testcrate/src/lib.rs#L255, but we don't really have anything to compare the result to here.

RalfJung · 2024-06-13T20:08:10Z

The file is kind of grown over time, and I did at some point copy a whole bunch of corner case tests over from the rustc test suite -- most of the cast tests are from that, I think. So I don't always know the point of the magic numbers either, I just hoped whoever who wrote the rustc tests knew.^^

But I don't know a good way to make this generic, no.

tgross35 · 2024-06-13T20:35:18Z

That makes more sense if they came from the rust suite. I just removed the const eval casting until I can take a closer look at that and hopefully clean up that test a bit. Having only unary and binary operations still saves a ton of the headache that was blocking me elsewhere (ICE on negative numbers 😠)

Ready for a look when you get the chance.

rustbot · 2024-06-13T20:35:22Z

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

matthiaskrgr · 2024-06-13T20:58:44Z

tests/crashes/124583.rs

-//@ known-bug: rust-lang/rust#124583
-
-fn main() {
-    let _ = -(-0.0f16);
-}


Could you add Fixes #124583 to pr header so that this is auto closed? thx!

I had it in a commit message and thought it would snag it there, but added just in case :)

ah I didn't check the commits 🙃 should be good enough then :)

tgross35 · 2024-06-13T21:23:13Z

Spurious failure: #126430

oli-obk · 2024-06-14T15:09:01Z

library/std/src/f128/tests.rs

+
+#[test]
+fn test_neg_zero() {
+    // only verify const eval negation until we add the rest of the checks
+    let _neg_zero: f16 = -0.0;
+}


These are thoroughly covered by the miri tests, I don't think we need it here, but if you wanna keep it, this needs to be f128

Also this isn't even doing any const-eval? If anything it might be hitting some mir-opt codepaths.

yea, const prop was hitting the mir interp code

Ah thanks for the catch, I missed that it was double negation that caused the crash in #124583. Updated tests/ui/numbers-arithmetic/f16-f128-lit.rs instead

RalfJung · 2024-06-14T17:36:08Z

src/tools/miri/tests/pass/float.rs

+float_to_int_fallback!(f16 => i8, u8, i16, u16, i32, u32, i64, u64, i128, u128);
+float_to_int!(f32=> i8, u8, i16, u16, i32, u32, i64, u64, i128, u128);
+float_to_int!(f64 => i8, u8, i16, u16, i32, u32, i64, u64, i128, u128);
+float_to_int_fallback!(f128 => i8, u8, i16, u16, i32, u32, i64, u64, i128, u128);


The new impls aren't actually used, are they?

Not without the cast changes that I wound up removing, I left the switch to the macro but removed f16/f128.

RalfJung · 2024-06-14T17:37:06Z

The last commit seems to un-do a bunch of the changes from the earlier commits...?

This excludes casting, which needs more tests.

Add a spot that was missed in <rust-lang#121997>.

Const eval negation was added. This test is now covered by Miri tests, and merged into an existing UI test. Fixes <rust-lang#124583>

tgross35 · 2024-06-14T17:52:41Z

The last commit seems to un-do a bunch of the changes from the earlier commits...?

I meant to remove the changes to cast entirely since I don't have a good test (getting tedious as mentioned above), just did a bad job rebasing. History should be cleaner now.

RalfJung · 2024-06-14T18:08:21Z

src/tools/miri/src/helpers.rs

                float_to_int_inner::<Double>(this, src.to_scalar().to_f64()?, cast_to, round),
-            FloatTy::F128 => unimplemented!("f16_f128"),
+            FloatTy::F128 =>
+                float_to_int_inner::<Quad>(this, src.to_scalar().to_f128()?, cast_to, round),


This seems to be dead code now that you removed the cast tests?

OTOH, the code is rather obvious, so I am also fine with keeping it.

Yeah you are right. If you have no problem then I think I may as well just leave it, casts won't be far behind.

RalfJung · 2024-06-14T18:10:38Z

Maybe update the PR title and description to say what is covered? Mainly unops and binops, it seems.

r=me after that, with or without the helpers.rs change.

tgross35 · 2024-06-14T18:23:01Z

I updated the description, feel free to tweak it if I'm missing something. (You or someone else need to r for me).

@rustbot label +F-f16_and_f128

RalfJung · 2024-06-14T18:46:03Z

@bors r+

bors · 2024-06-14T18:46:06Z

📌 Commit e649042 has been approved by RalfJung

It is now in the queue for this repository.

…lfJung Add `f16` and `f128` const eval for binary and unary operationations Add const evaluation and Miri support for f16 and f128, including unary and binary operations. Casts are not yet included. Fixes rust-lang#124583 r? `@RalfJung`

…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#125829 (rustc_span: Add conveniences for working with span formats) - rust-lang#126279 (Migrate `inaccessible-temp-dir`, `output-with-hyphens` and `issue-10971-temps-dir` `run-make` tests to `rmake`) - rust-lang#126361 (Unify intrinsics body handling in StableMIR) - rust-lang#126417 (Add `f16` and `f128` inline ASM support for `x86` and `x86-64`) - rust-lang#126424 ( Also sort `crt-static` in `--print target-features` output) - rust-lang#126428 (Polish `std::path::absolute` documentation.) - rust-lang#126429 (Add `f16` and `f128` const eval for binary and unary operationations) - rust-lang#126448 (End support for Python 3.8 in tidy) - rust-lang#126488 (Use `std::path::absolute` in bootstrap) - rust-lang#126511 (.mailmap: Associate both my work and my private email with me) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#125829 (rustc_span: Add conveniences for working with span formats) - rust-lang#126361 (Unify intrinsics body handling in StableMIR) - rust-lang#126417 (Add `f16` and `f128` inline ASM support for `x86` and `x86-64`) - rust-lang#126424 ( Also sort `crt-static` in `--print target-features` output) - rust-lang#126428 (Polish `std::path::absolute` documentation.) - rust-lang#126429 (Add `f16` and `f128` const eval for binary and unary operationations) - rust-lang#126448 (End support for Python 3.8 in tidy) - rust-lang#126488 (Use `std::path::absolute` in bootstrap) - rust-lang#126511 (.mailmap: Associate both my work and my private email with me) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#126429 - tgross35:f16-f128-const-eval, r=RalfJung Add `f16` and `f128` const eval for binary and unary operationations Add const evaluation and Miri support for f16 and f128, including unary and binary operations. Casts are not yet included. Fixes rust-lang#124583 r? ``@RalfJung``

…Simulacrum Add more constants, functions, and tests for `f16` and `f128` This adds everything that was in some way blocked on const eval, since rust-lang#126429 landed. There is a lot of `cfg(bootstrap)` since that is a fairly recent change. `f128` tests are disabled on everything except x86_64 and Linux aarch64, which are two platforms I know have "good" support for these types - meaning basic math symbols are available and LLVM doesn't hit selection crashes. `f16` tests are enabled on almost everything except for known LLVM crashes. Doctests are only enabled on x86_64. Tracking issue: rust-lang#116909

…mulacrum Add more constants, functions, and tests for `f16` and `f128` This adds everything that was in some way blocked on const eval, since rust-lang#126429 landed. There is a lot of `cfg(bootstrap)` since that is a fairly recent change. `f128` tests are disabled on everything except x86_64 and Linux aarch64, which are two platforms I know have "good" support for these types - meaning basic math symbols are available and LLVM doesn't hit selection crashes. `f16` tests are enabled on almost everything except for known LLVM crashes. Doctests are only enabled on x86_64. Tracking issue: rust-lang#116909

Add more constants, functions, and tests for `f16` and `f128` This adds everything that was in some way blocked on const eval, since rust-lang/rust#126429 landed. There is a lot of `cfg(bootstrap)` since that is a fairly recent change. `f128` tests are disabled on everything except x86_64 and Linux aarch64, which are two platforms I know have "good" support for these types - meaning basic math symbols are available and LLVM doesn't hit selection crashes. `f16` tests are enabled on almost everything except for known LLVM crashes. Doctests are only enabled on x86_64. Tracking issue: rust-lang/rust#116909

tgross35 force-pushed the f16-f128-const-eval branch from 6b41a65 to 0cedee1 Compare June 13, 2024 19:29

rustbot assigned RalfJung Jun 13, 2024

rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 13, 2024

This comment has been minimized.

Sign in to view

tgross35 force-pushed the f16-f128-const-eval branch from 0cedee1 to bf9961c Compare June 13, 2024 20:23

tgross35 marked this pull request as ready for review June 13, 2024 20:35

matthiaskrgr reviewed Jun 13, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

oli-obk reviewed Jun 14, 2024

View reviewed changes

tgross35 force-pushed the f16-f128-const-eval branch 2 times, most recently from 80c32ca to df2f8f7 Compare June 14, 2024 17:30

RalfJung reviewed Jun 14, 2024

View reviewed changes

tgross35 added 2 commits June 14, 2024 12:43

Enable const evaluation for f16 and f128

c906d2e

This excludes casting, which needs more tests.

Make the unary operator FloatTy check exhaustive

eab5e8e

Add a spot that was missed in <rust-lang#121997>.

tgross35 force-pushed the f16-f128-const-eval branch from df2f8f7 to 0f76fe3 Compare June 14, 2024 17:46

tgross35 added 2 commits June 14, 2024 12:47

Add f16 and f128 support to Miri

5cb58ad

Remove f16 const eval crash test

e649042

Const eval negation was added. This test is now covered by Miri tests, and merged into an existing UI test. Fixes <rust-lang#124583>

tgross35 force-pushed the f16-f128-const-eval branch from 0f76fe3 to e649042 Compare June 14, 2024 17:47

RalfJung reviewed Jun 14, 2024

View reviewed changes

tgross35 changed the title ~~Add f16 and f128 const evaluation~~ Add f16 and f128 const eval for binary and unary operationations Jun 14, 2024

rustbot added the F-f16_and_f128 `#![feature(f16)]`, `#![feature(f128)]` label Jun 14, 2024

tgross35 mentioned this pull request Jun 14, 2024

Tracking Issue for f16 and f128 float types #116909

Open

92 tasks

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 Jun 14, 2024

matthiaskrgr mentioned this pull request Jun 15, 2024

Rollup of 10 pull requests #126513

Closed

matthiaskrgr mentioned this pull request Jun 15, 2024

Rollup of 9 pull requests #126518

Merged

bors merged commit 3775f2f into rust-lang:master Jun 15, 2024

rustbot added this to the 1.81.0 milestone Jun 15, 2024

tgross35 mentioned this pull request Jun 17, 2024

Add more constants, functions, and tests for f16 and f128 #126608

Merged

tgross35 deleted the f16-f128-const-eval branch June 18, 2024 04:16

This was referenced Jun 19, 2024

Resolve Clippy f16 and f128 unimplemented!/FIXMEs #126636

Merged

Add f16 and f128 const eval for casting operations #126718

Closed

Uh oh!

Conversation

tgross35 commented Jun 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tgross35 commented Jun 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

RalfJung commented Jun 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tgross35 commented Jun 13, 2024

Uh oh!

rustbot commented Jun 13, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

tgross35 commented Jun 13, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tgross35 Jun 14, 2024 • 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 commented Jun 14, 2024

Uh oh!

tgross35 commented Jun 14, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RalfJung commented Jun 14, 2024

Uh oh!

tgross35 commented Jun 14, 2024

Uh oh!

RalfJung commented Jun 14, 2024 via email

Uh oh!

bors commented Jun 14, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

tgross35 commented Jun 13, 2024 •

edited

Loading

tgross35 commented Jun 13, 2024 •

edited

Loading

RalfJung commented Jun 13, 2024 •

edited

Loading

tgross35 Jun 14, 2024 •

edited

Loading