Add `f16` and `f128` by tgross35 · Pull Request #114607 · rust-lang/rust

tgross35 · 2023-08-08T03:52:12Z

Preliminary implementation of rust-lang/rfcs#3453 (still a work in progress)

This introduces four new features:

f16: lang feature, basic support for 16-bit floating point numbers
f128: same as above for 128-bit
f16_math: libs feature, gates math that relies on intrinsics
f128_math: libs feature, same as above

The math features are gated separately because it seems like LLVM has some bugs with the intrinsics. F16 should work but f128 is completely unusable llvm/llvm-project#44744.

I have found this calculator helpful if anyone is validating the data: http://weitz.de/ieee/

Tracking issue: #116909

Edit: I have started merging this in portions, see #116909 (comment)

r? ghost
@rustbot label +T-lang +T-libs-api

library/core/src/num/f128.rs

compiler/rustc_hir_analysis/src/check/intrinsic.rs

compiler/rustc_lint/src/types.rs

compiler/rustc_abi/src/lib.rs

bjorn3 · 2023-08-08T16:25:36Z

compiler/rustc_mir_build/src/build/mod.rs

+//     num.parse().ok()
+// }
+
+// FIXME: bootstrap `f16` parsing via `f32`


Suggested change

// FIXME: bootstrap `f16` parsing via `f32`

// FIXME: bootstrap `f128` parsing via `f64`

Also please don't remove this until both cg_clif and cg_gcc support the new types. Otherwise they can't bootstrap rustc anymore.

bjorn3 · 2023-08-08T16:29:12Z

library/core/src/fmt/float.rs

+#[cfg(not(bootstrap))]
+floating! { f16 }
+#[cfg(not(bootstrap))]
+floating! { f128 }


These methods are not #[inline] so they will completely break building the standard library with cg_gcc and cg_clif until these backends support f16 and f128.

What is the correct way to fix this? I have also just been using unimplemented!() in clif and gcc to make things compiile, I guess those should just throw errors instead?

(down the line of course, I'm still on the very first stage of getting this to work)

#[inline] functions are only codegened when actually used. So making them #[inline] at least temporarily would be an option.

I guess those should just throw errors instead?

That would replace an ICE with an error when compiling libcore. Both prevent successful compilation.

cg_clif and cg_gcc don't yet have CI tests yet right? Could turn them on while I'm working on this if so

Try cherry-picking #112701 and then remove https://github.com/rust-lang/rust/pull/112701/files#diff-8479eab02701e686aedb15b567dc8fc31220c6e4efb9565ccc9d662b7fee2214R3057-R3063 Once that is done if you set codegen-backends in config.toml to include llvm and cranelift (llvm first to avoid building rustc itself with cranelift), you can test it with ./x.py test compiler/rustc_codegen_cranelift.

Do these mini_core errors usually show up in CI? I don't think I'm getting them on local https://github.com/rust-lang/rust/actions/runs/5806522313/job/15739585197?pr=114607#step:24:3221

Because #112701 hasn't landed yet it is possible for PR's in this repo to break cg_clif's testsuite. I fixed that issue just today in bjorn3/rustc_codegen_cranelift@3deb6c6. Opened #114666 to sync it back to this repo.

library/core/src/num/f128.rs

tgross35 · 2023-08-08T20:27:39Z

@rustbot label -A-testsuite -T-bootstrap -T-infra

jendrikw · 2024-02-02T17:59:12Z

I'll try to continue this in my fork at https://github.com/jendrikw/rust/tree/f16-f128-impl over the next few weeks. Not sure what the best way to track the progress would be.

tgross35 · 2024-02-03T08:43:57Z

Hey, that would be awesome. These are the main things remaining:

Parsing and formatting. This is mostly working for f16 except for an edge case, which I think probably needs a debugger to step through and see what is going on.
The feature gate, really need somebody from the compiler team to check the implementation.
compiler-builtins needs updates to add missing intrinsics, can't even compile on aarch64 until this is done (which is what slowed my development). This is probably honestly the easiest point to jump in if you're up for it since it doesn't require any coordination Unable to find long double symbols on aarch64-apple-darwin compiler-builtins#567

Just so you are aware I haven't abandoned this and still plan to get it over the line, just switched to working on the related LLVM bug for a bit. But any help moving this forward is greatly appreciated, any of these single tasks would be pretty huge.

jendrikw · 2024-02-04T21:40:08Z

Hey @tgross35,

from the commit message of f769f48 I'm assuming these are squashed before pushing. Do you happen to have the original commits somewhere and could push them to your fork? That would be helpful as I'm trying to split the very large PR into multiple smaller ones.

tgross35 · 2024-02-05T00:24:41Z

Respectfully, I am still active on this so please don’t submit new PRs with my existing work. I can handle splitting it up, I was looking for guidance on how to proceed or for work that could apply on top of what is already here.

jendrikw · 2024-02-05T11:45:15Z

I understand. If you don't mind, I'll focus on compiler-builtins and open an PR there, if you don't mind.

…r=compiler-errors Add stubs in IR and ABI for `f16` and `f128` This is the very first step toward the changes in rust-lang#114607 and the [`f16` and `f128` RFC](https://rust-lang.github.io/rfcs/3453-f16-and-f128.html). It adds the types to `rustc_type_ir::FloatTy` and `rustc_abi::Primitive`, and just propagates those out as `unimplemented!` stubs where necessary. These types do not parse yet so there is no feature gate, and it should be okay to use `unimplemented!`. The next steps will probably be AST support with parsing and the feature gate. r? `@compiler-errors` cc `@Nilstrieb` suggested breaking the PR up in rust-lang#120645 (comment)

…r-errors Add stubs in IR and ABI for `f16` and `f128` This is the very first step toward the changes in rust-lang/rust#114607 and the [`f16` and `f128` RFC](https://rust-lang.github.io/rfcs/3453-f16-and-f128.html). It adds the types to `rustc_type_ir::FloatTy` and `rustc_abi::Primitive`, and just propagates those out as `unimplemented!` stubs where necessary. These types do not parse yet so there is no feature gate, and it should be okay to use `unimplemented!`. The next steps will probably be AST support with parsing and the feature gate. r? `@compiler-errors` cc `@Nilstrieb` suggested breaking the PR up in rust-lang/rust#120645 (comment)

…, r=compiler-errors `f16` and `f128` step 2: intrinsics Continuation of rust-lang#121728, another portion of rust-lang#114607. This PR adds `f16` and `f128` intrinsics, and hooks them up to both HIR and LLVM. This is all still unexposed to the frontend, which will probably be the next step. Also update itanium mangling per `@rcvalle's` in https://github.com/rust-lang/rust/pull/121728/files#r1506570300, and fix a typo from step 1. Once these types are usable in code, I will add the codegen tests from rust-lang#114607 (codegen is passing on that branch) This does add more `unimplemented!`s to Clippy, but I still don't think we can do better until library support is added. r? `@compiler-errors` cc `@Nilstrieb` `@rustbot` label +T-compiler +F-f16_and_f128

Rollup merge of rust-lang#121841 - tgross35:f16-f128-step2-intrinsics, r=compiler-errors `f16` and `f128` step 2: intrinsics Continuation of rust-lang#121728, another portion of rust-lang#114607. This PR adds `f16` and `f128` intrinsics, and hooks them up to both HIR and LLVM. This is all still unexposed to the frontend, which will probably be the next step. Also update itanium mangling per `@rcvalle's` in https://github.com/rust-lang/rust/pull/121728/files#r1506570300, and fix a typo from step 1. Once these types are usable in code, I will add the codegen tests from rust-lang#114607 (codegen is passing on that branch) This does add more `unimplemented!`s to Clippy, but I still don't think we can do better until library support is added. r? `@compiler-errors` cc `@Nilstrieb` `@rustbot` label +T-compiler +F-f16_and_f128

tgross35 · 2024-03-04T21:55:23Z

Work is in progress elsewhere, not much sense in keeping this open

…r-errors Add stubs in IR and ABI for `f16` and `f128` This is the very first step toward the changes in rust-lang/rust#114607 and the [`f16` and `f128` RFC](https://rust-lang.github.io/rfcs/3453-f16-and-f128.html). It adds the types to `rustc_type_ir::FloatTy` and `rustc_abi::Primitive`, and just propagates those out as `unimplemented!` stubs where necessary. These types do not parse yet so there is no feature gate, and it should be okay to use `unimplemented!`. The next steps will probably be AST support with parsing and the feature gate. r? `@compiler-errors` cc `@Nilstrieb` suggested breaking the PR up in rust-lang/rust#120645 (comment)

…te, r=compiler-errors `f16` and `f128` step 3: compiler support & feature gate Continuation of rust-lang#121841, another portion of rust-lang#114607 This PR exposes the new types to the world and adds a feature gate. Marking this as a draft because I need some feedback on where I did the feature gate check. It also does not yet catch type via suffixed literals (so the feature gate test will fail, probably some others too because I haven't belssed). If there is a better place to check all types after resolution, I can do that. If not, I figure maybe I can add a second gate location in AST when it checks numeric suffixes. Unfortunately I still don't think there is much testing to be done for correctness (codegen tests or parsed value checks) until we have basic library support. I think that will be the next step. Tracking issue: rust-lang#116909 r? `@compiler-errors` cc `@Nilstrieb` `@rustbot` label +F-f16_and_f128

…te, r=compiler-errors `f16` and `f128` step 3: compiler support & feature gate Continuation of rust-lang#121841, another portion of rust-lang#114607 This PR exposes the new types to the world and adds a feature gate. Marking this as a draft because I need some feedback on where I did the feature gate check. It also does not yet catch type via suffixed literals (so the feature gate test will fail, probably some others too because I haven't belssed). If there is a better place to check all types after resolution, I can do that. If not, I figure maybe I can add a second gate location in AST when it checks numeric suffixes. Unfortunately I still don't think there is much testing to be done for correctness (codegen tests or parsed value checks) until we have basic library support. I think that will be the next step. Tracking issue: rust-lang#116909 r? ``@compiler-errors`` cc ``@Nilstrieb`` ``@rustbot`` label +F-f16_and_f128

…te, r=compiler-errors `f16` and `f128` step 3: compiler support & feature gate Continuation of rust-lang#121841, another portion of rust-lang#114607 This PR exposes the new types to the world and adds a feature gate. Marking this as a draft because I need some feedback on where I did the feature gate check. It also does not yet catch type via suffixed literals (so the feature gate test will fail, probably some others too because I haven't belssed). If there is a better place to check all types after resolution, I can do that. If not, I figure maybe I can add a second gate location in AST when it checks numeric suffixes. Unfortunately I still don't think there is much testing to be done for correctness (codegen tests or parsed value checks) until we have basic library support. I think that will be the next step. Tracking issue: rust-lang#116909 r? ```@compiler-errors``` cc ```@Nilstrieb``` ```@rustbot``` label +F-f16_and_f128

…r-errors Add stubs in IR and ABI for `f16` and `f128` This is the very first step toward the changes in rust-lang/rust#114607 and the [`f16` and `f128` RFC](https://rust-lang.github.io/rfcs/3453-f16-and-f128.html). It adds the types to `rustc_type_ir::FloatTy` and `rustc_abi::Primitive`, and just propagates those out as `unimplemented!` stubs where necessary. These types do not parse yet so there is no feature gate, and it should be okay to use `unimplemented!`. The next steps will probably be AST support with parsing and the feature gate. r? `@compiler-errors` cc `@Nilstrieb` suggested breaking the PR up in rust-lang/rust#120645 (comment)

…ler-errors `f16` and `f128` step 2: intrinsics Continuation of rust-lang/rust#121728, another portion of rust-lang/rust#114607. This PR adds `f16` and `f128` intrinsics, and hooks them up to both HIR and LLVM. This is all still unexposed to the frontend, which will probably be the next step. Also update itanium mangling per `@rcvalle's` in https://github.com/rust-lang/rust/pull/121728/files#r1506570300, and fix a typo from step 1. Once these types are usable in code, I will add the codegen tests from #114607 (codegen is passing on that branch) This does add more `unimplemented!`s to Clippy, but I still don't think we can do better until library support is added. r? `@compiler-errors` cc `@Nilstrieb` `@rustbot` label +T-compiler +F-f16_and_f128

…, r=compiler-errors,petrochenkov `f16` and `f128` step 3: compiler support & feature gate Continuation of rust-lang#121841, another portion of rust-lang#114607 This PR exposes the new types to the world and adds a feature gate. Marking this as a draft because I need some feedback on where I did the feature gate check. It also does not yet catch type via suffixed literals (so the feature gate test will fail, probably some others too because I haven't belssed). If there is a better place to check all types after resolution, I can do that. If not, I figure maybe I can add a second gate location in AST when it checks numeric suffixes. Unfortunately I still don't think there is much testing to be done for correctness (codegen tests or parsed value checks) until we have basic library support. I think that will be the next step. Tracking issue: rust-lang#116909 r? `@compiler-errors` cc `@Nilstrieb` `@rustbot` label +F-f16_and_f128

…ler-errors,petrochenkov `f16` and `f128` step 3: compiler support & feature gate Continuation of rust-lang/rust#121841, another portion of rust-lang/rust#114607 This PR exposes the new types to the world and adds a feature gate. Marking this as a draft because I need some feedback on where I did the feature gate check. It also does not yet catch type via suffixed literals (so the feature gate test will fail, probably some others too because I haven't belssed). If there is a better place to check all types after resolution, I can do that. If not, I figure maybe I can add a second gate location in AST when it checks numeric suffixes. Unfortunately I still don't think there is much testing to be done for correctness (codegen tests or parsed value checks) until we have basic library support. I think that will be the next step. Tracking issue: rust-lang/rust#116909 r? `@compiler-errors` cc `@Nilstrieb` `@rustbot` label +F-f16_and_f128

…r=Amanieu `f16` and `f128` step 4: basic library support This is the next step after rust-lang#121926, another portion of rust-lang#114607 Tracking issue: rust-lang#116909 This PR adds the most basic operations to `f16` and `f128` that get lowered as LLVM intrinsics. This is a very small step but it seemed reasonable enough to add unopinionated basic operations before the larger modules that are built on top of them. r? `@Amanieu` since you were pretty involved in the RFC cc `@compiler-errors` `@rustbot` label +T-libs-api +S-blocked +F-f16_and_f128

…r=Amanieu `f16` and `f128` step 4: basic library support This is the next step after rust-lang#121926, another portion of rust-lang#114607 Tracking issue: rust-lang#116909 This PR adds the most basic operations to `f16` and `f128` that get lowered as LLVM intrinsics. This is a very small step but it seemed reasonable enough to add unopinionated basic operations before the larger modules that are built on top of them. r? ``@Amanieu`` since you were pretty involved in the RFC cc ``@compiler-errors`` ``@rustbot`` label +T-libs-api +S-blocked +F-f16_and_f128

This comment has been minimized.

Sign in to view

tgross35 force-pushed the f16-f128-impl branch from 0138507 to f33f672 Compare August 8, 2023 04:03

This comment has been minimized.

Sign in to view

ecnelises reviewed Aug 8, 2023

View reviewed changes

library/core/src/num/f128.rs Outdated Show resolved Hide resolved

Noratrieb reviewed Aug 8, 2023

View reviewed changes

compiler/rustc_hir_analysis/src/check/intrinsic.rs Outdated Show resolved Hide resolved

compiler/rustc_lint/src/types.rs Outdated Show resolved Hide resolved

CraftSpider reviewed Aug 8, 2023

View reviewed changes

compiler/rustc_abi/src/lib.rs Outdated Show resolved Hide resolved

compiler/rustc_abi/src/lib.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

bjorn3 reviewed Aug 8, 2023

View reviewed changes

This comment has been minimized.

Sign in to view

tgross35 commented Aug 8, 2023

View reviewed changes

library/core/src/num/f128.rs Show resolved Hide resolved

rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Aug 8, 2023

This comment has been minimized.

Sign in to view

jendrikw mentioned this pull request Feb 4, 2024

[WIP] continue f16 and f128 impl #120645

Closed

tgross35 mentioned this pull request Feb 28, 2024

Add stubs in IR and ABI for f16 and f128 #121728

Merged

tgross35 mentioned this pull request Mar 1, 2024

f16 and f128 step 2: intrinsics #121841

Merged

tgross35 mentioned this pull request Mar 3, 2024

f16 and f128 step 3: compiler support & feature gate #121926

Merged

compiler-errors added S-waiting-on-author 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 Mar 4, 2024

tgross35 closed this Mar 4, 2024

tgross35 mentioned this pull request Mar 14, 2024

f16 and f128 step 4: basic library support #122470

Merged

tgross35 mentioned this pull request Jun 26, 2024

Remove f16 and f128 ICE paths from smir #126983

Merged

	// FIXME: bootstrap `f16` parsing via `f32`
	// FIXME: bootstrap `f128` parsing via `f64`

Uh oh!

Conversation

tgross35 commented Aug 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

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!

Choose a reason for hiding this comment

Uh oh!

bjorn3 Aug 8, 2023 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

Uh oh!

This comment has been minimized.

tgross35 commented Aug 8, 2023

Uh oh!

jendrikw commented Feb 2, 2024

Uh oh!

tgross35 commented Feb 3, 2024

Uh oh!

jendrikw commented Feb 4, 2024

Uh oh!

tgross35 commented Feb 5, 2024

Uh oh!

jendrikw commented Feb 5, 2024

Uh oh!

tgross35 commented Mar 4, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

20 participants

tgross35 commented Aug 8, 2023 •

edited

Loading

bjorn3 Aug 8, 2023 •

edited

Loading