Skip to content
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

Add f16 and f128 #114607

Closed
wants to merge 7 commits into from
Closed

Add f16 and f128 #114607

wants to merge 7 commits into from

Conversation

tgross35
Copy link
Contributor

@tgross35 tgross35 commented Aug 8, 2023

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

@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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Aug 8, 2023
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

compiler/rustc_hir_analysis/src/check/intrinsic.rs Outdated Show resolved Hide resolved
compiler/rustc_lint/src/types.rs Outdated Show resolved Hide resolved
compiler/rustc_abi/src/lib.rs Outdated Show resolved Hide resolved
compiler/rustc_abi/src/lib.rs Outdated Show resolved Hide resolved
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

// num.parse().ok()
// }

// FIXME: bootstrap `f16` parsing via `f32`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

#[cfg(not(bootstrap))]
floating! { f16 }
#[cfg(not(bootstrap))]
floating! { f128 }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#[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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@bjorn3 bjorn3 Aug 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@rust-log-analyzer

This comment has been minimized.

@rustbot 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
@rust-log-analyzer

This comment has been minimized.

@tgross35
Copy link
Contributor Author

tgross35 commented Aug 8, 2023

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

@rustbot 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. and removed 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
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Mar 2, 2024
…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)
Nadrieril added a commit to Nadrieril/rust that referenced this pull request Mar 2, 2024
…, 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
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 3, 2024
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
@compiler-errors 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
Copy link
Contributor Author

tgross35 commented Mar 4, 2024

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

@tgross35 tgross35 closed this Mar 4, 2024
antoyo pushed a commit to rust-lang/rustc_codegen_gcc that referenced this pull request Mar 5, 2024
…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)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 6, 2024
…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
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 6, 2024
…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
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 6, 2024
…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
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request Mar 7, 2024
…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)
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request Mar 7, 2024
…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
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 15, 2024
…, 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
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 16, 2024
…, 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
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Mar 17, 2024
…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
fmease added a commit to fmease/rust that referenced this pull request Apr 10, 2024
…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
fmease added a commit to fmease/rust that referenced this pull request Apr 10, 2024
…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
fmease added a commit to fmease/rust that referenced this pull request Apr 10, 2024
…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
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 11, 2024
…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
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 11, 2024
…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
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 11, 2024
Rollup merge of rust-lang#122470 - tgross35:f16-f128-step4-libs-min, 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
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 26, 2024
Remove `f16` and `f128` ICE paths from smir

Just chasing down some possible ICE paths. `@compiler-errors` mentioned in rust-lang#121728 (comment) that it is okay not to support these in smir, but this change seems pretty trivial?

r? `@celinval` since you reviewed rust-lang#114607 (review)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 26, 2024
Remove `f16` and `f128` ICE paths from smir

Just chasing down some possible ICE paths. ``@compiler-errors`` mentioned in rust-lang#121728 (comment) that it is okay not to support these in smir, but this change seems pretty trivial?

r? ``@celinval`` since you reviewed rust-lang#114607 (review)
jhpratt added a commit to jhpratt/rust that referenced this pull request Jun 27, 2024
Remove `f16` and `f128` ICE paths from smir

Just chasing down some possible ICE paths. ```@compiler-errors``` mentioned in rust-lang#121728 (comment) that it is okay not to support these in smir, but this change seems pretty trivial?

r? ```@celinval``` since you reviewed rust-lang#114607 (review)
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 27, 2024
Rollup merge of rust-lang#126983 - tgross35:f16-f128-smir, r=celinval

Remove `f16` and `f128` ICE paths from smir

Just chasing down some possible ICE paths. ```@compiler-errors``` mentioned in rust-lang#121728 (comment) that it is okay not to support these in smir, but this change seems pretty trivial?

r? ```@celinval``` since you reviewed rust-lang#114607 (review)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc F-f16_and_f128 `#![feature(f16)]`, `#![feature(f128)]` O-unix Operating system: Unix-like rla-silenced Silences rust-log-analyzer postings to the PR it's added on. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language 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. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.