-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Add f16
formatting and parsing
#127013
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
base: master
Are you sure you want to change the base?
Add f16
formatting and parsing
#127013
Conversation
This comment has been minimized.
This comment has been minimized.
@rustbot label +rla-silenced |
@rustbot label +F-f16_and_f128 |
This will need #127510 |
☔ The latest upstream changes (presumably #127020) made this pull request unmergeable. Please resolve the merge conflicts. |
7c3f9c1
to
f3ebeb3
Compare
Update: I'm really just waiting on #128083 to bump stage0, managing |
☔ The latest upstream changes (presumably #128360) made this pull request unmergeable. Please resolve the merge conflicts. |
2eaa479
to
422c52e
Compare
21ffabc
to
2098f01
Compare
404089f
to
3636530
Compare
I don't think I've ever used this and I have no idea how it works 😆 any improvements to running test-float-parse via bootstrap are welcome though, I wired it up just enough to run tests in CI. |
I'm not very familiar with the CI system; can you link me to where you wired it up? |
|
Doesn't CI use it? |
It does not AFAIK. It was mostly added to help local developers. #109933 I don't think it's been worked on for a while. |
What's the system for registering CI commands? |
The CI goes through some shell wrappers What do you want to do with "registering CI commands"? CI also goes through bootstrap. Footnotes
|
c1f042b
to
18e5709
Compare
Thanks @speedy-lex for the tests, almost everything is passing (including test-float-parse!). I have a few
Bootstrap can be set up to build a path as a crate, rust/src/bootstrap/mk/Makefile.in Lines 51 to 55 in 91a0e16
|
@tgross35, I've figured out CI on my own already but thanks for commenting. Also, what else needs to be done before this is finished? |
There are a few small places where I added tests for consistency but didn’t yet update them, labeled After that this should be review-ready |
check_exact!(f(f16::MIN_POSITIVE) => b"6103515625 ", -4); | ||
check_exact!(f(minf16) => b"59604644775390625", -7); | ||
|
||
// todo: add tables from f32 and f64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the last thing remaining to do, I think it might take a bit of work to apply the algorithms from the Paxson paper to binary16. @speedy-lex have you touched this at all?
(I'm working on it, thinking the described algorithm would be good in test-float-parse)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't really looked at it since everything was so hard to find. Have you made any progress on it so far?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made some progress but wasn't getting very good results with basically a direct implementation of the described algorithm. It's pretty complex and probably needs some thought put into it to figure out what is wrong, but I think we should be okay just leaving this as a FIXME for now.
My scratch work for reference https://github.com/tgross35/float-paxson-format-test
6ba3e9d
to
3bb0af3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tgross35, you missed some func calls when you added the target_arch stuff.
☔ The latest upstream changes (presumably #139229) made this pull request unmergeable. Please resolve the merge conflicts. |
Use the existing Lemire (decimal -> float) and Dragon / Grisu algorithms (float -> decimal) to add support for `f16`. This allows updating the implementation for `Display` to the expected behavior for `Display` (currently it prints the a hex bitwise representation) and adds a `FromStr` implementation.
0c4ca7c
to
ddeae7b
Compare
I didn't manage to get the last group of tests implemented yet #127013 (comment), but everything else is passing so I think this should be mergable. r? libs |
ddeae7b
to
db05f28
Compare
Extend the existing tests for `f32` and `f64` with versions that include `f16`'s new printing and parsing implementations. Co-authored-by: Speedy_Lex <alex.ciocildau@gmail.com>
This requires a fix to the subnormal test to cap the maximum allowed value within the maximum mantissa.
db05f28
to
6f07075
Compare
@bors try |
Add `f16` formatting and parsing Use the same algorithms as for `f32` and `f64` to implement `f16` parsing and printing. try-job: aarch64-gnu
☀️ Try build successful - checks-actions |
Use the same algorithms as for
f32
andf64
to implementf16
parsing and printing.try-job: aarch64-gnu