Add f16 and f128 math functions by tgross35 · Pull Request #127027 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation60 Commits6 Checks12 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
This adds intrinsics and math functions for f16 and f128 floating point types. Support is quite limited and some things are broken so tests don't run on many platforms, but this provides a starting point.
try-job: aarch64-gnu
r? ghost
@rustbot label +F-f16_and_f128
Operating system: Unix-like
Status: Awaiting review from the assignee but also interested parties.
Relevant to the library team, which will review and decide on the PR/issue.
`#![feature(f16)]`, `#![feature(f128)]`
labels
This comment has been minimized.
Still getting a failure on f16 locally (aarch64) that I can't explain, 1.0f16.powi(1) is returning a wrong result. This happens in the unit tests but doesn't seem to replicate when I copy them to a separate file, making this difficult to debug.
May as well see if this shows up in CI.
@bors try
bors added a commit to rust-lang-ci/rust that referenced this pull request
Add f16 and f128 math functions
WIP
This will be pretty finicky among selection failures, the lowering bug, and lack of symbols. But might as well start somewhere.
Effectively blocked on const casting.
try-job: aarch64-gnu
Figured out how to reproduce, the incorrect value is only returned if optimizations are on 💀
@bors cancel
This comment was marked as outdated.
This comment has been minimized.
This comment was marked as outdated.
Status: This is awaiting some action (such as code changes or more information) from the author.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
This comment was marked as outdated.
bors added a commit to rust-lang-ci/rust that referenced this pull request
Add f16 and f128 math functions
WIP
This will be pretty finicky among selection failures, the lowering bug, and lack of symbols. But might as well start somewhere.
Checks will fail until rust-lang#126636 makes it to beta, which should be next week (July 19).
try-job: aarch64-gnu
This comment was marked as outdated.
There are probably some rough edges that I need to smooth out and some tests that need tweaking, but this PR is somewhat large so I think somebody can start taking a look at it. f16 pass on my aarch machine, I can't test f128 locally so I'm going via try jobs.
Note that this is still going to fail the check CI job until nightly clippy becomes beta next week.
r? @dtolnay
Since I know you have reviewed the math functions before. Feel free to reroll if you prefer.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
bors added a commit to rust-lang-ci/rust that referenced this pull request
Add f16 and f128 math functions
This adds intrinsics and math functions for f16 and f128 floating point types. Support is quite limited and some things are broken so tests don't run on many platforms, but this provides a starting point.
Checks will fail until rust-lang#126636 makes it to beta, which should be next week (July 19).
try-job: aarch64-gnu
This comment was marked as outdated.
Status: Blocked on something else such as an RFC or other implementation work.
and removed S-waiting-on-author
Status: This is awaiting some action (such as code changes or more information) from the author.
labels
Thank you for the review! Yes, the Clippy ICE fix should be be in beta Friday/Saturday so that should (hopefully) fix CI.
edit: update, the bootstrap bump has unfortunately just taken a while #128083.
This was referenced
Jul 18, 2024
Since LLVM <https://reviews.llvm.org/D99439> (4c7f820, "Update
@llvm.powi to handle different int sizes for the exponent"), the size of
the integer can be specified for the powi intrinsic. Make use of this
so it is more obvious that integer size is consistent across all float
types.
This feature is available since LLVM 13 (October 2021). Based on bootstrap we currently support >= 17.0, so there should be no support problems.
These already exist in the compiler. Expose them in core so we can add their library functions.
This adds missing functions for math operations on the new float types.
Platform support is pretty spotty at this point, since even platforms
with generally good support can be missing math functions.
std/build.rs is updated to reflect this.
min, max, and similar functions require external math routines. Add
these under the same gates as std math functions (reliable_f16_math
and reliable_f128_math).
Clarify what makes some operations not safe, and correct comment in the default branch ("not safe" -> "safe").
Due to a LLVM bug, f128 math functions link successfully but LLVM
chooses the wrong symbols (long double symbols rather than those for
binary128).
Since this is a notable problem that may surprise a number of users, add a note about it.
Link: llvm/llvm-project#44744
The stage0 bump took longer than expected but finally merged, so I was able to rebase. Since the last review I removed all the cfg(not(bootstrap)) and added a note in the docs about the x86 math bug, so users are aware of it.
One more try job to make sure nothing got broken:
@bors try
x86 CI passed and it looks like the aarch64 try job has tested the libraries already, so this should be about ready. Changes were trivial since the last look so
@bors r=dtolnay
📌 Commit d64bbb1 has been approved by dtolnay
It is now in the queue for this repository.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-blocked
Status: Blocked on something else such as an RFC or other implementation work.
labels
Mark reset master. I'll open a new PR.
Labels
`#![feature(f16)]`, `#![feature(f128)]`
This PR was explicitly merged by bors.
Operating system: Unix-like
Silences rust-log-analyzer postings to the PR it's added on.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Relevant to the library team, which will review and decide on the PR/issue.