From 42ac64efd234748c2163a65d9f19555e4cf9ed78 Mon Sep 17 00:00:00 2001 From: Kleis Auke Wolthuizen Date: Mon, 12 Aug 2024 08:08:41 +0200 Subject: [PATCH] Fix CI for targets that conditionally disable `f16` or `f128` support --- build.rs | 83 +------------------------------------- configure.rs | 84 +++++++++++++++++++++++++++++++++++++++ testcrate/build.rs | 45 ++++++++++++--------- testcrate/src/bench.rs | 2 +- testcrate/src/lib.rs | 4 +- testcrate/tests/addsub.rs | 4 +- testcrate/tests/cmp.rs | 2 +- testcrate/tests/conv.rs | 14 +++---- testcrate/tests/mul.rs | 4 +- 9 files changed, 127 insertions(+), 115 deletions(-) create mode 100644 configure.rs diff --git a/build.rs b/build.rs index 56716b90..08307ac5 100644 --- a/build.rs +++ b/build.rs @@ -1,44 +1,8 @@ use std::{collections::BTreeMap, env, path::PathBuf, sync::atomic::Ordering}; -#[allow(dead_code)] -struct Target { - triple: String, - os: String, - arch: String, - vendor: String, - env: String, - pointer_width: u8, - little_endian: bool, - features: Vec, -} - -impl Target { - fn from_env() -> Self { - let little_endian = match env::var("CARGO_CFG_TARGET_ENDIAN").unwrap().as_str() { - "little" => true, - "big" => false, - x => panic!("unknown endian {x}"), - }; +mod configure; - Self { - triple: env::var("TARGET").unwrap(), - os: env::var("CARGO_CFG_TARGET_OS").unwrap(), - arch: env::var("CARGO_CFG_TARGET_ARCH").unwrap(), - vendor: env::var("CARGO_CFG_TARGET_VENDOR").unwrap(), - env: env::var("CARGO_CFG_TARGET_ENV").unwrap(), - pointer_width: env::var("CARGO_CFG_TARGET_POINTER_WIDTH") - .unwrap() - .parse() - .unwrap(), - little_endian, - features: env::var("CARGO_CFG_TARGET_FEATURE") - .unwrap_or_default() - .split(",") - .map(ToOwned::to_owned) - .collect(), - } - } -} +use crate::configure::{configure_f16_f128, Target}; fn main() { println!("cargo:rerun-if-changed=build.rs"); @@ -261,49 +225,6 @@ fn configure_check_cfg() { println!("cargo::rustc-check-cfg=cfg(assert_no_panic)"); } -/// Configure whether or not `f16` and `f128` support should be enabled. -fn configure_f16_f128(target: &Target) { - // Set whether or not `f16` and `f128` are supported at a basic level by LLVM. This only means - // that the backend will not crash when using these types. This does not mean that the - // backend does the right thing, or that the platform doesn't have ABI bugs. - // - // We do this here rather than in `rust-lang/rust` because configuring via cargo features is - // not straightforward. - // - // Original source of this list: - // - let (f16_ok, f128_ok) = match target.arch.as_str() { - // `f16` and `f128` both crash - "arm64ec" => (false, false), - // `f16` crashes - "s390x" => (false, true), - // `f128` crashes - "mips64" | "mips64r6" => (true, false), - // `f128` crashes - "powerpc64" if &target.os == "aix" => (true, false), - // `f128` crashes - "sparc" | "sparcv9" => (true, false), - // `f16` miscompiles - "wasm32" | "wasm64" => (false, true), - // Most everything else works as of LLVM 19 - _ => (true, true), - }; - - // If the feature is set, disable these types. - let disable_both = env::var_os("CARGO_FEATURE_NO_F16_F128").is_some(); - - println!("cargo::rustc-check-cfg=cfg(f16_enabled)"); - println!("cargo::rustc-check-cfg=cfg(f128_enabled)"); - - if f16_ok && !disable_both { - println!("cargo::rustc-cfg=f16_enabled"); - } - - if f128_ok && !disable_both { - println!("cargo::rustc-cfg=f128_enabled"); - } -} - #[cfg(feature = "c")] mod c { use std::collections::{BTreeMap, HashSet}; diff --git a/configure.rs b/configure.rs new file mode 100644 index 00000000..a94a0e38 --- /dev/null +++ b/configure.rs @@ -0,0 +1,84 @@ +use std::env; + +#[allow(dead_code)] +pub(crate) struct Target { + pub(crate) triple: String, + pub(crate) os: String, + pub(crate) arch: String, + pub(crate) vendor: String, + pub(crate) env: String, + pub(crate) pointer_width: u8, + pub(crate) little_endian: bool, + pub(crate) features: Vec, +} + +impl Target { + pub(crate) fn from_env() -> Self { + let little_endian = match env::var("CARGO_CFG_TARGET_ENDIAN").unwrap().as_str() { + "little" => true, + "big" => false, + x => panic!("unknown endian {x}"), + }; + + Self { + triple: env::var("TARGET").unwrap(), + os: env::var("CARGO_CFG_TARGET_OS").unwrap(), + arch: env::var("CARGO_CFG_TARGET_ARCH").unwrap(), + vendor: env::var("CARGO_CFG_TARGET_VENDOR").unwrap(), + env: env::var("CARGO_CFG_TARGET_ENV").unwrap(), + pointer_width: env::var("CARGO_CFG_TARGET_POINTER_WIDTH") + .unwrap() + .parse() + .unwrap(), + little_endian, + features: env::var("CARGO_CFG_TARGET_FEATURE") + .unwrap_or_default() + .split(",") + .map(ToOwned::to_owned) + .collect(), + } + } +} + +/// Configure whether or not `f16` and `f128` support should be enabled. +pub(crate) fn configure_f16_f128(target: &Target) { + // Set whether or not `f16` and `f128` are supported at a basic level by LLVM. This only means + // that the backend will not crash when using these types. This does not mean that the + // backend does the right thing, or that the platform doesn't have ABI bugs. + // + // We do this here rather than in `rust-lang/rust` because configuring via cargo features is + // not straightforward. + // + // Original source of this list: + // + let (f16_ok, f128_ok) = match target.arch.as_str() { + // `f16` and `f128` both crash + "arm64ec" => (false, false), + // `f16` crashes + "s390x" => (false, true), + // `f128` crashes + "mips64" | "mips64r6" => (true, false), + // `f128` crashes + "powerpc64" if &target.os == "aix" => (true, false), + // `f128` crashes + "sparc" | "sparcv9" => (true, false), + // `f16` miscompiles + "wasm32" | "wasm64" => (false, true), + // Most everything else works as of LLVM 19 + _ => (true, true), + }; + + // If the feature is set, disable these types. + let disable_both = env::var_os("CARGO_FEATURE_NO_F16_F128").is_some(); + + println!("cargo::rustc-check-cfg=cfg(f16_enabled)"); + println!("cargo::rustc-check-cfg=cfg(f128_enabled)"); + + if f16_ok && !disable_both { + println!("cargo::rustc-cfg=f16_enabled"); + } + + if f128_ok && !disable_both { + println!("cargo::rustc-cfg=f128_enabled"); + } +} diff --git a/testcrate/build.rs b/testcrate/build.rs index 74d74559..8b886dd0 100644 --- a/testcrate/build.rs +++ b/testcrate/build.rs @@ -1,4 +1,4 @@ -use std::{collections::HashSet, env}; +use std::collections::HashSet; /// Features to enable #[derive(Debug, PartialEq, Eq, Hash)] @@ -9,35 +9,39 @@ enum Feature { NoSysF16F128Convert, } +mod builtins_configure { + include!("../configure.rs"); +} + fn main() { - let target = env::var("TARGET").unwrap(); + let target = builtins_configure::Target::from_env(); let mut features = HashSet::new(); // These platforms do not have f128 symbols available in their system libraries, so // skip related tests. - if target.starts_with("arm-") - || target.contains("apple-darwin") - || target.contains("windows-msvc") + if target.arch == "arm" + || target.vendor == "apple" + || target.env == "msvc" // GCC and LLVM disagree on the ABI of `f16` and `f128` with MinGW. See // . - || target.contains("windows-gnu") + || (target.os == "windows" && target.env == "gnu") // FIXME(llvm): There is an ABI incompatibility between GCC and Clang on 32-bit x86. // See . - || target.starts_with("i686") + || target.arch == "i686" // 32-bit PowerPC and 64-bit LE gets code generated that Qemu cannot handle. See // . - || target.starts_with("powerpc-") - || target.starts_with("powerpc64le-") + || target.arch == "powerpc" + || target.arch == "powerpc64le" // FIXME: We get different results from the builtin functions. See // . - || target.starts_with("powerpc64-") + || target.arch == "powerpc64" { features.insert(Feature::NoSysF128); features.insert(Feature::NoSysF128IntConvert); features.insert(Feature::NoSysF16F128Convert); } - if target.starts_with("i586") || target.starts_with("i686") { + if target.arch == "i586" || target.arch == "i686" { // 32-bit x86 does not have `__fixunstfti`/`__fixtfti` but does have everything else features.insert(Feature::NoSysF128IntConvert); // FIXME: 32-bit x86 has a bug in `f128 -> f16` system libraries @@ -46,17 +50,18 @@ fn main() { // These platforms do not have f16 symbols available in their system libraries, so // skip related tests. Most of these are missing `f16 <-> f32` conversion routines. - if (target.starts_with("aarch64-") && target.contains("linux")) - || target.starts_with("arm") - || target.starts_with("powerpc-") - || target.starts_with("powerpc64-") - || target.starts_with("powerpc64le-") - || target.starts_with("i586-") - || target.contains("windows-") + if (target.arch == "aarch64" && target.os == "linux") + || target.arch == "arm" + || target.arch == "powerpc" + || target.arch == "powerpc64le" + || target.arch == "powerpc64le" + || target.arch == "i586" + || target.os == "windows" // Linking says "error: function signature mismatch: __extendhfsf2" and seems to // think the signature is either `(i32) -> f32` or `(f32) -> f32`. See // . - || target.starts_with("wasm") + || target.arch == "wasm32" + || target.arch == "wasm64" { features.insert(Feature::NoSysF16); features.insert(Feature::NoSysF16F128Convert); @@ -78,4 +83,6 @@ fn main() { println!("cargo:warning={warning}"); println!("cargo:rustc-cfg=feature=\"{name}\""); } + + builtins_configure::configure_f16_f128(&target); } diff --git a/testcrate/src/bench.rs b/testcrate/src/bench.rs index 5ab9bac8..018887e6 100644 --- a/testcrate/src/bench.rs +++ b/testcrate/src/bench.rs @@ -350,7 +350,7 @@ macro_rules! impl_testio { } } -#[cfg(not(feature = "no-f16-f128"))] +#[cfg(all(f16_enabled, f128_enabled))] impl_testio!(float f16, f128); impl_testio!(float f32, f64); impl_testio!(int i16, i32, i64, i128); diff --git a/testcrate/src/lib.rs b/testcrate/src/lib.rs index 5458c9ab..cc9e7393 100644 --- a/testcrate/src/lib.rs +++ b/testcrate/src/lib.rs @@ -13,8 +13,8 @@ //! Some floating point tests are disabled for specific architectures, because they do not have //! correct rounding. #![no_std] -#![cfg_attr(not(feature = "no-f16-f128"), feature(f128))] -#![cfg_attr(not(feature = "no-f16-f128"), feature(f16))] +#![cfg_attr(f128_enabled, feature(f128))] +#![cfg_attr(f16_enabled, feature(f16))] #![feature(isqrt)] pub mod bench; diff --git a/testcrate/tests/addsub.rs b/testcrate/tests/addsub.rs index f21f61ff..da286fbe 100644 --- a/testcrate/tests/addsub.rs +++ b/testcrate/tests/addsub.rs @@ -120,7 +120,7 @@ mod float_addsub { } } -#[cfg(not(feature = "no-f16-f128"))] +#[cfg(f128_enabled)] #[cfg(not(all(target_arch = "x86", not(target_feature = "sse"))))] #[cfg(not(any(target_arch = "powerpc", target_arch = "powerpc64")))] mod float_addsub_f128 { @@ -131,7 +131,7 @@ mod float_addsub_f128 { } } -#[cfg(not(feature = "no-f16-f128"))] +#[cfg(f128_enabled)] #[cfg(any(target_arch = "powerpc", target_arch = "powerpc64"))] mod float_addsub_f128_ppc { use super::*; diff --git a/testcrate/tests/cmp.rs b/testcrate/tests/cmp.rs index e8a0eb16..486b1d1d 100644 --- a/testcrate/tests/cmp.rs +++ b/testcrate/tests/cmp.rs @@ -94,7 +94,7 @@ mod float_comparisons { } #[test] - #[cfg(not(feature = "no-f16-f128"))] + #[cfg(f128_enabled)] fn cmp_f128() { #[cfg(any(target_arch = "powerpc", target_arch = "powerpc64"))] use compiler_builtins::float::cmp::{ diff --git a/testcrate/tests/conv.rs b/testcrate/tests/conv.rs index e394183c..ce1f64e6 100644 --- a/testcrate/tests/conv.rs +++ b/testcrate/tests/conv.rs @@ -1,5 +1,5 @@ -#![cfg_attr(not(feature = "no-f16-f128"), feature(f16))] -#![cfg_attr(not(feature = "no-f16-f128"), feature(f128))] +#![cfg_attr(f128_enabled, feature(f128))] +#![cfg_attr(f16_enabled, feature(f16))] // makes configuration easier #![allow(unused_macros)] #![allow(unused_imports)] @@ -176,7 +176,7 @@ mod f_to_i { } #[test] - #[cfg(not(feature = "no-f16-f128"))] + #[cfg(f128_enabled)] fn f128_to_int() { #[cfg(any(target_arch = "powerpc", target_arch = "powerpc64"))] use compiler_builtins::float::conv::{ @@ -264,7 +264,7 @@ mod extend { f32 => f64, Single => Double, __extendsfdf2vfp, all(); } - #[cfg(not(feature = "no-f16-f128"))] + #[cfg(all(f16_enabled, f128_enabled))] #[cfg(not(any(target_arch = "powerpc", target_arch = "powerpc64")))] f_to_f! { extend, @@ -275,7 +275,7 @@ mod extend { f64 => f128, Double => Quad, __extenddftf2, not(feature = "no-sys-f128"); } - #[cfg(not(feature = "no-f16-f128"))] + #[cfg(f128_enabled)] #[cfg(any(target_arch = "powerpc", target_arch = "powerpc64"))] f_to_f! { extend, @@ -299,7 +299,7 @@ mod trunc { f64 => f32, Double => Single, __truncdfsf2vfp, all(); } - #[cfg(not(feature = "no-f16-f128"))] + #[cfg(all(f16_enabled, f128_enabled))] #[cfg(not(any(target_arch = "powerpc", target_arch = "powerpc64")))] f_to_f! { trunc, @@ -310,7 +310,7 @@ mod trunc { f128 => f64, Quad => Double, __trunctfdf2, not(feature = "no-sys-f128"); } - #[cfg(not(feature = "no-f16-f128"))] + #[cfg(f128_enabled)] #[cfg(any(target_arch = "powerpc", target_arch = "powerpc64"))] f_to_f! { trunc, diff --git a/testcrate/tests/mul.rs b/testcrate/tests/mul.rs index 90144bb0..5b9e9b6d 100644 --- a/testcrate/tests/mul.rs +++ b/testcrate/tests/mul.rs @@ -131,7 +131,7 @@ mod float_mul { } } -#[cfg(not(feature = "no-f16-f128"))] +#[cfg(f128_enabled)] #[cfg(not(all(target_arch = "x86", not(target_feature = "sse"))))] #[cfg(not(any(target_arch = "powerpc", target_arch = "powerpc64")))] mod float_mul_f128 { @@ -145,7 +145,7 @@ mod float_mul_f128 { } } -#[cfg(not(feature = "no-f16-f128"))] +#[cfg(f128_enabled)] #[cfg(any(target_arch = "powerpc", target_arch = "powerpc64"))] mod float_mul_f128_ppc { use super::*;