Skip to content

disable f16 tests for the cranelift backend#520

Open
folkertdev wants to merge 1 commit intorust-lang:masterfrom
folkertdev:disable-f16-tests-for-cranelift
Open

disable f16 tests for the cranelift backend#520
folkertdev wants to merge 1 commit intorust-lang:masterfrom
folkertdev:disable-f16-tests-for-cranelift

Conversation

@folkertdev
Copy link
Copy Markdown
Contributor

@folkertdev folkertdev commented Apr 16, 2026

fixes #519

@bjorn3 does this look like it would work?

bjorn3 added a commit to rust-lang/rustc_codegen_cranelift that referenced this pull request Apr 16, 2026
bjorn3 added a commit to rust-lang/rustc_codegen_cranelift that referenced this pull request Apr 16, 2026
@bjorn3
Copy link
Copy Markdown
Member

bjorn3 commented Apr 16, 2026

@folkertdev
Copy link
Copy Markdown
Contributor Author

So actually it looks like the failure is in proptest? And there we certainly don't want to cfg on f16 math given that we want to stabilize f16 soon.

The libc implementation seems doable to implement, actually?

https://github.com/llvm/llvm-project/blob/b96818f2e0118fcbf908b0a168ca1d23190ac295/libc/src/__support/FPUtil/NearestIntegerOperations.h#L54-L105

@bjorn3
Copy link
Copy Markdown
Member

bjorn3 commented Apr 16, 2026

And there we certainly don't want to cfg on f16 math given that we want to stabilize f16 soon.

There are still a bunch of cases where the LLVM backend also says it doesn't support f16 math. So either way cfg(target_has_reliable_f16_math) is necessary to make it possible to test portable-simd on those targets at all.

@folkertdev
Copy link
Copy Markdown
Contributor Author

I believe that is because we still support older LLVM versions?

https://github.com/rust-lang/rust/blob/e8e4541ff19649d95afab52fdde2c2eaa6829965/compiler/rustc_codegen_llvm/src/llvm_util.rs#L373-L389

With LLVM 22 f16 should work everywhere as far as I know. Unless we're running into this case here

        (Arch::X86_64, Os::Windows) if *target_env == Env::Gnu && *target_abi != CfgAbi::Llvm => {

@bjorn3
Copy link
Copy Markdown
Member

bjorn3 commented Apr 16, 2026

To unblock the cg_clif rustc update, I have for now added a patch that remove all f16 support in portable-simd again.

Comment thread crates/core_simd/tests/f16_ops.rs Outdated
Comment on lines +12 to +14
// FIXME: the cranelift backend runs these tests for x86_64-pc-windows-gnu
// where it does not have full support for f16 yet.
#[cfg(target_has_reliable_f16_math)]
Copy link
Copy Markdown

@tgross35 tgross35 Apr 16, 2026

Choose a reason for hiding this comment

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

I'd drop this comment, it's not a cranelift-specific issue and the target_has_reliable_f16_math is effectively already a fixme.

View changes since the review

@tgross35
Copy link
Copy Markdown

At this time I'd still consider it more or less a requirement to gate anything that may codegen behind cfg(target_has_reliable_f16). We still have these edge cases and they're kind of bootstrapped by the fact that the cfg is used pretty consistently (out of necessity) like here. It will be at least ~4 months until our minimum LLVM version is high enough to drop the cfg completely.

relevant for the cranelift backend, possibly other cases/targets too
@folkertdev folkertdev force-pushed the disable-f16-tests-for-cranelift branch from 1f19d30 to f40da91 Compare April 16, 2026 22:38
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.

Missing cfg(target_has_reliable_f16_math) check for f16 tests

3 participants