From 66f8906862fba51ffde9d9a73b527a29402bce93 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Mon, 28 Oct 2024 17:44:02 -0500 Subject: [PATCH 1/2] Move the existing "unstable" feature to "unstable-intrinsics" Currently there is a single feature called "unstable" that is used to control whether intrinsics may be called. In anticipation of adding other unstable features that we will want to control separately, create a new feature called "unstable-intrinsics" that is enabled by "unstable". Then move everything gated by "unstable" to "unstable-intrinsics". --- library/compiler-builtins/libm/Cargo.toml | 5 ++++- library/compiler-builtins/libm/ci/run.sh | 6 +++--- .../libm/crates/compiler-builtins-smoke-test/Cargo.toml | 2 ++ library/compiler-builtins/libm/src/lib.rs | 4 ++-- library/compiler-builtins/libm/src/math/mod.rs | 8 +++++--- 5 files changed, 16 insertions(+), 9 deletions(-) diff --git a/library/compiler-builtins/libm/Cargo.toml b/library/compiler-builtins/libm/Cargo.toml index aa6c08ddb011..de450468a472 100644 --- a/library/compiler-builtins/libm/Cargo.toml +++ b/library/compiler-builtins/libm/Cargo.toml @@ -18,7 +18,10 @@ default = [] # This tells the compiler to assume that a Nightly toolchain is being used and # that it should activate any useful Nightly things accordingly. -unstable = [] +unstable = ["unstable-intrinsics"] + +# Enable calls to functions in `core::intrinsics` +unstable-intrinsics = [] # Used to prevent using any intrinsics or arch-specific code. force-soft-floats = [] diff --git a/library/compiler-builtins/libm/ci/run.sh b/library/compiler-builtins/libm/ci/run.sh index f61fff8433e3..f1ca4b0cb109 100755 --- a/library/compiler-builtins/libm/ci/run.sh +++ b/library/compiler-builtins/libm/ci/run.sh @@ -49,7 +49,7 @@ fi if [ "${BUILD_ONLY:-}" = "1" ]; then cmd="cargo build --target $target --package libm" $cmd - $cmd --features 'unstable' + $cmd --features "unstable-intrinsics" echo "can't run tests on $target" else @@ -60,6 +60,6 @@ else $cmd --release # unstable with a feature - $cmd --features 'unstable' - $cmd --release --features 'unstable' + $cmd --features "unstable-intrinsics" + $cmd --release --features "unstable-intrinsics" fi diff --git a/library/compiler-builtins/libm/crates/compiler-builtins-smoke-test/Cargo.toml b/library/compiler-builtins/libm/crates/compiler-builtins-smoke-test/Cargo.toml index 8d084ee341e9..2aa7c83718b4 100644 --- a/library/compiler-builtins/libm/crates/compiler-builtins-smoke-test/Cargo.toml +++ b/library/compiler-builtins/libm/crates/compiler-builtins-smoke-test/Cargo.toml @@ -10,6 +10,8 @@ test = false bench = false [features] +# Duplicated from libm's Cargo.toml unstable = [] +unstable-intrinsics = [] checked = [] force-soft-floats = [] diff --git a/library/compiler-builtins/libm/src/lib.rs b/library/compiler-builtins/libm/src/lib.rs index 6d95fa1737b3..1305d35abf7d 100644 --- a/library/compiler-builtins/libm/src/lib.rs +++ b/library/compiler-builtins/libm/src/lib.rs @@ -1,7 +1,7 @@ //! libm in pure Rust #![no_std] -#![cfg_attr(feature = "unstable", allow(internal_features))] -#![cfg_attr(feature = "unstable", feature(core_intrinsics))] +#![cfg_attr(feature = "unstable-intrinsics", allow(internal_features))] +#![cfg_attr(feature = "unstable-intrinsics", feature(core_intrinsics))] #![allow(clippy::assign_op_pattern)] #![allow(clippy::deprecated_cfg_attr)] #![allow(clippy::eq_op)] diff --git a/library/compiler-builtins/libm/src/math/mod.rs b/library/compiler-builtins/libm/src/math/mod.rs index 85c9fc5bf156..17b9e6b4cf67 100644 --- a/library/compiler-builtins/libm/src/math/mod.rs +++ b/library/compiler-builtins/libm/src/math/mod.rs @@ -60,14 +60,14 @@ macro_rules! i { // the time of this writing this is only used in a few places, and once // rust-lang/rust#72751 is fixed then this macro will no longer be necessary and // the native `/` operator can be used and panics won't be codegen'd. -#[cfg(any(debug_assertions, not(feature = "unstable")))] +#[cfg(any(debug_assertions, not(feature = "unstable-intrinsics")))] macro_rules! div { ($a:expr, $b:expr) => { $a / $b }; } -#[cfg(all(not(debug_assertions), feature = "unstable"))] +#[cfg(all(not(debug_assertions), feature = "unstable-intrinsics"))] macro_rules! div { ($a:expr, $b:expr) => { unsafe { core::intrinsics::unchecked_div($a, $b) } @@ -76,7 +76,9 @@ macro_rules! div { macro_rules! llvm_intrinsically_optimized { (#[cfg($($clause:tt)*)] $e:expr) => { - #[cfg(all(feature = "unstable", not(feature = "force-soft-floats"), $($clause)*))] + #[cfg(all( + feature = "unstable-intrinsics", not(feature = "force-soft-floats"), $($clause)* + ))] { if true { // thwart the dead code lint $e From e6f7053c2e2d7fe2de0cf5582bfd529c2cd3b5a7 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Mon, 28 Oct 2024 18:15:19 -0500 Subject: [PATCH 2/2] Replace `feature = "unstable-intrinsics"` with `intrinsics_enabled` We currently have a non-additive feature, "force-soft-floats", and we will need to gain another "no-f16-f128". This makes `cfg` usage in code somewhat confusing and redundant. Use `build.rs` to figure out if "unstable-intrinsics" is enabled while "force-soft-floats" is not enabled and if so, emit a cfg `intrinsics_enabled`. This is cleaner to use and should make adding more features easier to reason about. Also use this as an opportunity to eliminate the build.rs from the compiler-builtins test crate, replaced with the `[lints]` table in Cargo.toml. --- library/compiler-builtins/libm/Cargo.toml | 5 +++++ library/compiler-builtins/libm/build.rs | 15 ++++++++++++++- library/compiler-builtins/libm/ci/run.sh | 4 ++++ .../compiler-builtins-smoke-test/Cargo.toml | 6 ++++++ .../crates/compiler-builtins-smoke-test/build.rs | 3 --- library/compiler-builtins/libm/src/lib.rs | 4 ++-- library/compiler-builtins/libm/src/math/mod.rs | 8 +++----- 7 files changed, 34 insertions(+), 11 deletions(-) delete mode 100644 library/compiler-builtins/libm/crates/compiler-builtins-smoke-test/build.rs diff --git a/library/compiler-builtins/libm/Cargo.toml b/library/compiler-builtins/libm/Cargo.toml index de450468a472..5e4565556b02 100644 --- a/library/compiler-builtins/libm/Cargo.toml +++ b/library/compiler-builtins/libm/Cargo.toml @@ -24,6 +24,11 @@ unstable = ["unstable-intrinsics"] unstable-intrinsics = [] # Used to prevent using any intrinsics or arch-specific code. +# +# HACK: this is a negative feature which is generally a bad idea in Cargo, but +# we need it to be able to forbid other features when this crate is used in +# Rust dependencies. Setting this overrides all features that may enable +# hard float operations. force-soft-floats = [] [workspace] diff --git a/library/compiler-builtins/libm/build.rs b/library/compiler-builtins/libm/build.rs index b683557e4af8..adb521407308 100644 --- a/library/compiler-builtins/libm/build.rs +++ b/library/compiler-builtins/libm/build.rs @@ -3,7 +3,6 @@ use std::env; fn main() { println!("cargo:rerun-if-changed=build.rs"); println!("cargo:rustc-check-cfg=cfg(assert_no_panic)"); - println!("cargo:rustc-check-cfg=cfg(feature, values(\"unstable\"))"); println!("cargo:rustc-check-cfg=cfg(feature, values(\"checked\"))"); @@ -14,4 +13,18 @@ fn main() { println!("cargo:rustc-cfg=assert_no_panic"); } } + + configure_intrinsics(); +} + +/// Simplify the feature logic for enabling intrinsics so code only needs to use +/// `cfg(intrinsics_enabled)`. +fn configure_intrinsics() { + println!("cargo:rustc-check-cfg=cfg(intrinsics_enabled)"); + + // Disabled by default; `unstable-intrinsics` enables again; `force-soft-floats` overrides + // to disable. + if cfg!(feature = "unstable-intrinsics") && !cfg!(feature = "force-soft-floats") { + println!("cargo:rustc-cfg=intrinsics_enabled"); + } } diff --git a/library/compiler-builtins/libm/ci/run.sh b/library/compiler-builtins/libm/ci/run.sh index f1ca4b0cb109..d3fc4ce240dd 100755 --- a/library/compiler-builtins/libm/ci/run.sh +++ b/library/compiler-builtins/libm/ci/run.sh @@ -46,6 +46,10 @@ if [ "$(uname -a)" = "Linux" ]; then extra_flags="$extra_flags --features libm-test/test-musl-serialized" fi +# Make sure we can build with overriding features. We test the indibidual +# features it controls separately. +cargo check --features "force-soft-floats" + if [ "${BUILD_ONLY:-}" = "1" ]; then cmd="cargo build --target $target --package libm" $cmd diff --git a/library/compiler-builtins/libm/crates/compiler-builtins-smoke-test/Cargo.toml b/library/compiler-builtins/libm/crates/compiler-builtins-smoke-test/Cargo.toml index 2aa7c83718b4..2a6c62961e36 100644 --- a/library/compiler-builtins/libm/crates/compiler-builtins-smoke-test/Cargo.toml +++ b/library/compiler-builtins/libm/crates/compiler-builtins-smoke-test/Cargo.toml @@ -15,3 +15,9 @@ unstable = [] unstable-intrinsics = [] checked = [] force-soft-floats = [] + +[lints.rust] +unexpected_cfgs = { level = "warn", check-cfg = [ + "cfg(assert_no_panic)", + "cfg(intrinsics_enabled)", +] } diff --git a/library/compiler-builtins/libm/crates/compiler-builtins-smoke-test/build.rs b/library/compiler-builtins/libm/crates/compiler-builtins-smoke-test/build.rs deleted file mode 100644 index 27d4a0e89d0b..000000000000 --- a/library/compiler-builtins/libm/crates/compiler-builtins-smoke-test/build.rs +++ /dev/null @@ -1,3 +0,0 @@ -fn main() { - println!("cargo::rustc-check-cfg=cfg(assert_no_panic)"); -} diff --git a/library/compiler-builtins/libm/src/lib.rs b/library/compiler-builtins/libm/src/lib.rs index 1305d35abf7d..98ac55988d0a 100644 --- a/library/compiler-builtins/libm/src/lib.rs +++ b/library/compiler-builtins/libm/src/lib.rs @@ -1,7 +1,7 @@ //! libm in pure Rust #![no_std] -#![cfg_attr(feature = "unstable-intrinsics", allow(internal_features))] -#![cfg_attr(feature = "unstable-intrinsics", feature(core_intrinsics))] +#![cfg_attr(intrinsics_enabled, allow(internal_features))] +#![cfg_attr(intrinsics_enabled, feature(core_intrinsics))] #![allow(clippy::assign_op_pattern)] #![allow(clippy::deprecated_cfg_attr)] #![allow(clippy::eq_op)] diff --git a/library/compiler-builtins/libm/src/math/mod.rs b/library/compiler-builtins/libm/src/math/mod.rs index 17b9e6b4cf67..9baa57fc8825 100644 --- a/library/compiler-builtins/libm/src/math/mod.rs +++ b/library/compiler-builtins/libm/src/math/mod.rs @@ -60,14 +60,14 @@ macro_rules! i { // the time of this writing this is only used in a few places, and once // rust-lang/rust#72751 is fixed then this macro will no longer be necessary and // the native `/` operator can be used and panics won't be codegen'd. -#[cfg(any(debug_assertions, not(feature = "unstable-intrinsics")))] +#[cfg(any(debug_assertions, not(intrinsics_enabled)))] macro_rules! div { ($a:expr, $b:expr) => { $a / $b }; } -#[cfg(all(not(debug_assertions), feature = "unstable-intrinsics"))] +#[cfg(all(not(debug_assertions), intrinsics_enabled))] macro_rules! div { ($a:expr, $b:expr) => { unsafe { core::intrinsics::unchecked_div($a, $b) } @@ -76,9 +76,7 @@ macro_rules! div { macro_rules! llvm_intrinsically_optimized { (#[cfg($($clause:tt)*)] $e:expr) => { - #[cfg(all( - feature = "unstable-intrinsics", not(feature = "force-soft-floats"), $($clause)* - ))] + #[cfg(all(intrinsics_enabled, not(feature = "force-soft-floats"), $($clause)*))] { if true { // thwart the dead code lint $e