Skip to content

ci: Don't set compiler-builtins-no-f16-f128 #743

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tgross35
Copy link
Contributor

Since rust-lang/rust be35d37d8b6c ("Use the compiler to determine whether or not to enable f16 and f128"), compiler-builtins relies on rustc to report whether or not f16 and f128 are supported, which is reported by the backend. This means that there should no longer be any need to set this config in CI.

Backend config:

let has_reliable_f16 = target_info.supports_target_dependent_type(CType::Float16);
let has_reliable_f128 = target_info.supports_target_dependent_type(CType::Float128);
TargetConfig {
target_features,
unstable_target_features,
// There are no known bugs with GCC support for f16 or f128
has_reliable_f16,
has_reliable_f16_math: has_reliable_f16,
has_reliable_f128,
has_reliable_f128_math: has_reliable_f128,
}

@antoyo
Copy link
Contributor

antoyo commented Jul 25, 2025

@tgross35
Copy link
Contributor Author

Yeah, that's the same error. Looks like GCC is reporting it supports f128 on m68k when it actually doesn't?

@tgross35
Copy link
Contributor Author

tgross35 commented Jul 25, 2025

Actually, I think the toolchain here is from before rust-lang/rust#143405 merged, meaning rust-lang/rust@be35d37 isn't available. Should have known since that is what we were looking at in #t-compiler/help > How to correctly disable `f16` and `f128` when bootstraping.

@tgross35 tgross35 force-pushed the remove-no-f16-f128 branch from e52a112 to 6a7ae71 Compare August 5, 2025 18:17
@tgross35
Copy link
Contributor Author

tgross35 commented Aug 5, 2025

Looks like this is getting stuck because running rustc in the build.rs for Cargo.toml doesn't account for codegen backend, and we can't check it because build scripts don't have RUSTFLAGS. So the compiler-builtins configure part is unfortunately apparently broken for non-llvm backends, and sadly this needs some Cargo tie in to fix it.

Started discussion at #t-cargo > Detecting `-Zcodegen-backend` from build.rs.

@tgross35 tgross35 force-pushed the remove-no-f16-f128 branch from b505ad1 to 4265f3b Compare August 5, 2025 22:21
This is in the compiler-builtins repository but has yet to be synced to
rust-lang/rust and then rustc_codegen_gcc. Once that happens, this patch
can be removed (it will no longer apply).

Link: rust-lang/compiler-builtins@87a66ec
Since rust-lang/rust be35d37d8b6c ("Use the compiler to determine
whether or not to enable f16 and f128"), `compiler-builtins` relies on
`rustc` to report whether or not `f16` and `f128` are supported, which
is reported by the backend. This means that there should no longer be
any need to set this config in CI.

Backend config: https://github.com/rust-lang/rustc_codegen_gcc/blob/f682d09eefc6700b9e5851ef193847959acf4fac/src/lib.rs#L499-L510
@tgross35 tgross35 force-pushed the remove-no-f16-f128 branch from 4265f3b to fe6f5ef Compare August 5, 2025 22:32
@tgross35
Copy link
Contributor Author

tgross35 commented Aug 5, 2025

🎉 it works!

The first commit is rust-lang/compiler-builtins@87a66ec. It's in the compiler-builtins repo now but will take a while to sync to rust-lang/rust then rust-lang/rustc_codegen_gcc, so using a patch unblocks this.

@tgross35 tgross35 marked this pull request as ready for review August 5, 2025 22:41
@tgross35 tgross35 requested a review from antoyo August 5, 2025 22:41
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.

2 participants