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 builtins for f16/f128 float conversions #593

Merged
merged 1 commit into from
May 2, 2024

Conversation

beetrees
Copy link
Contributor

Adds builtins for float conversions to/from f16/f128, which will solve rust-lang/rust#123885. I also updated the README.md to reflect that f16/f128 are being added to Rust.

@tgross35 this should save you some work on #587.

I'm not sure if the new functions need the #[avr_skip] attribute like the existing extend/truncate builtins do, but thought it was better to add it and accidentally cause linker errors if the builtin is needed and not supplied by libgcc rather than not having the attribute and accidentally causing subtle ABI bugs if a special ABI is required on AVR. @Patryk27 who added the original #[avr_skip]s.

@Patryk27
Copy link
Contributor

Yeah, I'm not sure on AVR's ABI here, so probably better to keep those #[avr_skip] 🙂

@beetrees beetrees force-pushed the fix-f16-conv-c branch 2 times, most recently from 6dd8caf to 4878c32 Compare April 14, 2024 22:46
tgross35 added a commit to tgross35/compiler-builtins that referenced this pull request Apr 15, 2024
@tgross35
Copy link
Contributor

Should this also be setting COMPILER_RT_HAS_FLOAT16 for the C build, in case that winds up being used as a fallback in the future?

@tgross35
Copy link
Contributor

Since this will probably land before #587, you might have to add the feature gate that @bjorn3 requested #587 (comment)

@beetrees
Copy link
Contributor Author

I've added a no-f16-f128 feature (following the convention set by the existing no-asm feature). I've also added a COMPILER_RT_HAS_FLOAT16 define when compiler-rt is being built, with a comment explaining why it is there.

@tgross35
Copy link
Contributor

Thanks, that part looks great to me. Looks like tests might be missing? Like

// PowerPC tests are failing on LLVM 13: https://github.com/rust-lang/rust/issues/88520
#[cfg(not(target_arch = "powerpc64"))]
#[test]
fn float_extend() {
use compiler_builtins::float::extend::__extendsfdf2;
extend!(f32, f64, __extendsfdf2);
}
/
#[test]
fn float_trunc() {
use compiler_builtins::float::trunc::__truncdfsf2;
trunc!(f64, f32, __truncdfsf2);
}
#[cfg(target_arch = "arm")]
#[test]
fn float_trunc_arm() {
use compiler_builtins::float::trunc::__truncdfsf2vfp;
trunc!(f64, f32, __truncdfsf2vfp);
}

tgross35 added a commit to tgross35/compiler-builtins that referenced this pull request Apr 17, 2024
tgross35 added a commit to tgross35/compiler-builtins that referenced this pull request Apr 19, 2024
@beetrees beetrees force-pushed the fix-f16-conv-c branch 2 times, most recently from 201f826 to 11c1eed Compare April 21, 2024 15:55
@beetrees
Copy link
Contributor Author

As the builtins aren't present everywhere at the moment (and therefore comparing against the builtin compiler float conversions would cause test failures on platforms where they're currently unavailable or miscompiled), I've instead added tests comparing compiler_builtins float conversions to those in rustc_apfloat, the (heavily tested) soft float library ported from LLVM for use in rustc. I've also fixed a copy-paste error and some shift overflow bugs (that only show up now the destination type can be larger than 32 bits) in trunc.rs that I discovered by running the tests, and moved the float conversion tests from misc.rs to conv.rs.

@tgross35
Copy link
Contributor

I've instead added tests comparing compiler_builtins float conversions to those in rustc_apfloat

Great idea, I can probably do the same on #587. Everything here looks reasonable to my understanding, probably ready for a look from @Amanieu.

testcrate/tests/conv.rs Outdated Show resolved Hide resolved
@beetrees beetrees force-pushed the fix-f16-conv-c branch 2 times, most recently from 76e648a to efd5d7d Compare April 28, 2024 16:38
src/float/trunc.rs Outdated Show resolved Hide resolved
@@ -52,8 +52,10 @@ where
// destination format. We can convert by simply right-shifting with
// rounding and adjusting the exponent.
abs_result = (a_abs >> sign_bits_delta).cast();
let tmp = src_exp_bias.wrapping_sub(dst_exp_bias) << R::SIGNIFICAND_BITS;
abs_result = abs_result.wrapping_sub(tmp.cast());
// Cast before shifting to prevent overflow.
Copy link
Member

Choose a reason for hiding this comment

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

I'm having trouble understand how exactly this helps prevent overflows. Aren't you casting to a smaller size (u16) here instead of operating as a u32?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Amanieu This is indeed not an issue for truncation to f16. However, the __trunctfdf2 builtin also implemented in this PR truncates from f128 to f64, meaning R::Int is a u64 and R::SIGNIFICAND_BITS is 52.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I missed that. LGTM.

nicholasbishop added a commit to nicholasbishop/rust that referenced this pull request May 16, 2024
The `weak-intrinsics` feature was removed from compiler_builtins in
rust-lang/compiler-builtins#598, so dropped the
`compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some
builtins for f16/f128 were added. These don't work for all compiler
backends, so add a `compiler-builtins-no-f16-f128` feature and disable
it for cranelift and gcc.
nicholasbishop added a commit to nicholasbishop/rust that referenced this pull request May 27, 2024
The `weak-intrinsics` feature was removed from compiler_builtins in
rust-lang/compiler-builtins#598, so dropped the
`compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some
builtins for f16/f128 were added. These don't work for all compiler
backends, so add a `compiler-builtins-no-f16-f128` feature and disable
it for cranelift and gcc.
nicholasbishop added a commit to nicholasbishop/rust that referenced this pull request May 30, 2024
The `weak-intrinsics` feature was removed from compiler_builtins in
rust-lang/compiler-builtins#598, so dropped the
`compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some
builtins for f16/f128 were added. These don't work for all compiler
backends, so add a `compiler-builtins-no-f16-f128` feature and disable
it for cranelift and gcc.
nicholasbishop added a commit to nicholasbishop/rust that referenced this pull request May 30, 2024
The `weak-intrinsics` feature was removed from compiler_builtins in
rust-lang/compiler-builtins#598, so dropped the
`compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some
builtins for f16/f128 were added. These don't work for all compiler
backends, so add a `compiler-builtins-no-f16-f128` feature and disable
it for cranelift and gcc.
bors added a commit to rust-lang-ci/rust that referenced this pull request May 30, 2024
Update compiler_builtins to 0.1.112

The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc.
nicholasbishop added a commit to nicholasbishop/rust that referenced this pull request May 30, 2024
The `weak-intrinsics` feature was removed from compiler_builtins in
rust-lang/compiler-builtins#598, so dropped the
`compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some
builtins for f16/f128 were added. These don't work for all compiler
backends, so add a `compiler-builtins-no-f16-f128` feature and disable
it for cranelift and gcc.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 4, 2024
…iler-errors

Update compiler_builtins to 0.1.112

The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc.
nicholasbishop added a commit to nicholasbishop/rust that referenced this pull request Jun 4, 2024
The `weak-intrinsics` feature was removed from compiler_builtins in
rust-lang/compiler-builtins#598, so dropped the
`compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some
builtins for f16/f128 were added. These don't work for all compiler
backends, so add a `compiler-builtins-no-f16-f128` feature and disable
it for cranelift and gcc. Also disable it for LLVM targets that don't
support it.
nicholasbishop added a commit to nicholasbishop/rust that referenced this pull request Jun 4, 2024
The `weak-intrinsics` feature was removed from compiler_builtins in
rust-lang/compiler-builtins#598, so dropped the
`compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some
builtins for f16/f128 were added. These don't work for all compiler
backends, so add a `compiler-builtins-no-f16-f128` feature and disable
it for cranelift and gcc. Also disable it for LLVM targets that don't
support it.
nicholasbishop added a commit to nicholasbishop/rust that referenced this pull request Jun 8, 2024
The `weak-intrinsics` feature was removed from compiler_builtins in
rust-lang/compiler-builtins#598, so dropped the
`compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some
builtins for f16/f128 were added. These don't work for all compiler
backends, so add a `compiler-builtins-no-f16-f128` feature and disable
it for cranelift and gcc. Also disable it for LLVM targets that don't
support it.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 17, 2024
Update compiler_builtins to 0.1.112

The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc.
tgross35 pushed a commit to tgross35/rust that referenced this pull request Jun 21, 2024
The `weak-intrinsics` feature was removed from compiler_builtins in
rust-lang/compiler-builtins#598, so dropped the
`compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some
builtins for f16/f128 were added. These don't work for all compiler
backends, so add a `compiler-builtins-no-f16-f128` feature and disable
it for cranelift and gcc. Also disable it for LLVM targets that don't
support it.
nicholasbishop added a commit to nicholasbishop/rust that referenced this pull request Jun 25, 2024
The `weak-intrinsics` feature was removed from compiler_builtins in
rust-lang/compiler-builtins#598, so dropped the
`compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some
builtins for f16/f128 were added. These don't work for all compiler
backends, so add a `compiler-builtins-no-f16-f128` feature and disable
it for cranelift and gcc. Also disable it for LLVM targets that don't
support it.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 26, 2024
Update compiler_builtins to 0.1.112

The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 26, 2024
…anieu

Update compiler_builtins to 0.1.113

The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 26, 2024
…anieu

Update compiler_builtins to 0.1.113

The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc.
tgross35 pushed a commit to tgross35/rust that referenced this pull request Jul 10, 2024
The `weak-intrinsics` feature was removed from compiler_builtins in
rust-lang/compiler-builtins#598, so dropped the
`compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some
builtins for f16/f128 were added. These don't work for all compiler
backends, so add a `compiler-builtins-no-f16-f128` feature and disable
it for cranelift and gcc. Also disable it for LLVM targets that don't
support it.
tgross35 pushed a commit to tgross35/rust that referenced this pull request Jul 18, 2024
The `weak-intrinsics` feature was removed from compiler_builtins in
rust-lang/compiler-builtins#598, so dropped the
`compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some
builtins for f16/f128 were added. These don't work for all compiler
backends, so add a `compiler-builtins-no-f16-f128` feature and disable
it for cranelift and gcc. Also disable it for LLVM targets that don't
support it.
nicholasbishop added a commit to nicholasbishop/rust that referenced this pull request Jul 29, 2024
The `weak-intrinsics` feature was removed from compiler_builtins in
rust-lang/compiler-builtins#598, so dropped the
`compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some
builtins for f16/f128 were added. These don't work for all compiler
backends, so add a `compiler-builtins-no-f16-f128` feature and disable
it for cranelift and gcc. Also disable it for LLVM targets that don't
support it.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 29, 2024
…ss35

Update compiler_builtins to 0.1.114

The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc.
Mrmaxmeier pushed a commit to Mrmaxmeier/rust that referenced this pull request Jul 29, 2024
The `weak-intrinsics` feature was removed from compiler_builtins in
rust-lang/compiler-builtins#598, so dropped the
`compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some
builtins for f16/f128 were added. These don't work for all compiler
backends, so add a `compiler-builtins-no-f16-f128` feature and disable
it for cranelift and gcc. Also disable it for LLVM targets that don't
support it.
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Jul 30, 2024
Update compiler_builtins to 0.1.114

The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc.
bjorn3 pushed a commit to rust-lang/rustc_codegen_cranelift that referenced this pull request Aug 2, 2024
The `weak-intrinsics` feature was removed from compiler_builtins in
rust-lang/compiler-builtins#598, so dropped the
`compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some
builtins for f16/f128 were added. These don't work for all compiler
backends, so add a `compiler-builtins-no-f16-f128` feature and disable
it for cranelift and gcc. Also disable it for LLVM targets that don't
support it.
bjorn3 pushed a commit to rust-lang/rustc_codegen_cranelift that referenced this pull request Aug 2, 2024
Update compiler_builtins to 0.1.114

The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc.
GuillaumeGomez pushed a commit to GuillaumeGomez/rustc_codegen_gcc that referenced this pull request Aug 6, 2024
The `weak-intrinsics` feature was removed from compiler_builtins in
rust-lang/compiler-builtins#598, so dropped the
`compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some
builtins for f16/f128 were added. These don't work for all compiler
backends, so add a `compiler-builtins-no-f16-f128` feature and disable
it for cranelift and gcc. Also disable it for LLVM targets that don't
support it.
GuillaumeGomez pushed a commit to GuillaumeGomez/rustc_codegen_gcc that referenced this pull request Aug 6, 2024
Update compiler_builtins to 0.1.114

The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc.
GuillaumeGomez pushed a commit to GuillaumeGomez/rustc_codegen_gcc that referenced this pull request Aug 12, 2024
The `weak-intrinsics` feature was removed from compiler_builtins in
rust-lang/compiler-builtins#598, so dropped the
`compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some
builtins for f16/f128 were added. These don't work for all compiler
backends, so add a `compiler-builtins-no-f16-f128` feature and disable
it for cranelift and gcc. Also disable it for LLVM targets that don't
support it.
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Aug 13, 2024
Update compiler_builtins to 0.1.114

The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot.

In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants