From 6b97e039e4fc90500b03e7676b2c9d8a07105f78 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Tue, 31 Mar 2026 17:50:35 -0400 Subject: [PATCH] c-b: Replace the `no-asm` feature with an opposite-polarity `arch` Make `libm` and `compiler-builtins` use the same configuration: * By default there is an `arch` feature which allows `core::arch` and inline assembly. * Disabling the `arch` feature turns these off. Positive feature flags are easier to reason about than negative, so this will allow for some build script cleanup as well. --- .../compiler-builtins/builtins-shim/Cargo.toml | 11 +++++------ .../compiler-builtins/builtins-test/Cargo.toml | 12 ++++-------- library/compiler-builtins/ci/bench-icount.sh | 2 +- library/compiler-builtins/ci/miri.sh | 5 +++-- library/compiler-builtins/ci/run.sh | 8 ++++---- .../compiler-builtins/Cargo.toml | 11 +++++------ .../compiler-builtins/compiler-builtins/build.rs | 2 +- .../src/int/specialized_div_rem/mod.rs | 16 ++++++++-------- .../compiler-builtins/src/mem/mod.rs | 5 +---- library/compiler-builtins/libm-test/Cargo.toml | 4 +++- 10 files changed, 35 insertions(+), 41 deletions(-) diff --git a/library/compiler-builtins/builtins-shim/Cargo.toml b/library/compiler-builtins/builtins-shim/Cargo.toml index 0ff2acdbbe6a..c940723a1ba5 100644 --- a/library/compiler-builtins/builtins-shim/Cargo.toml +++ b/library/compiler-builtins/builtins-shim/Cargo.toml @@ -39,17 +39,16 @@ test = false cc = { version = "1.2", optional = true } [features] -default = [] +default = ["arch"] + +# Enable architecture-specific features such as SIMD or assembly routines. If +# disabled, the generic version can be tested on any platform. +arch = [] # Enable compilation of C code in compiler-rt, filling in some more optimized # implementations and also filling in unimplemented intrinsics c = ["dep:cc"] -# For implementations where there is both a generic version and a platform- -# specific version, use the generic version. This is meant to enable testing -# the generic versions on all platforms. -no-asm = [] - # Flag this library as the unstable compiler-builtins lib. This must be enabled # when using as `std`'s dependency.' compiler-builtins = ["unmangled-names"] diff --git a/library/compiler-builtins/builtins-test/Cargo.toml b/library/compiler-builtins/builtins-test/Cargo.toml index 1b356b102c19..f1a5be415675 100644 --- a/library/compiler-builtins/builtins-test/Cargo.toml +++ b/library/compiler-builtins/builtins-test/Cargo.toml @@ -6,7 +6,7 @@ publish = false license = "MIT AND Apache-2.0 WITH LLVM-exception AND (MIT OR Apache-2.0)" [dependencies] -compiler_builtins = { workspace = true, features = ["unstable-public-internals"] } +compiler_builtins = { workspace = true, default-features = false, features = ["unstable-public-internals"] } # For fuzzing tests we want a deterministic seedable RNG. We also eliminate potential # problems with system RNGs on the variety of platforms this crate is tested on. @@ -24,9 +24,10 @@ gungraun = { workspace = true, optional = true } paste.workspace = true [features] -default = [] +# Defaults should match the defaults in compiler-builtins since we have that +# dependency with `default-features=false`. +default = ["compiler_builtins/arch"] c = ["compiler_builtins/c"] -no-asm = ["compiler_builtins/no-asm"] # Enable icount benchmarks (requires gungraun-runner and valgrind locally) icount = ["dep:gungraun"] @@ -36,11 +37,6 @@ icount = ["dep:gungraun"] benchmarking-reports = ["walltime", "criterion/plotters", "criterion/html_reports"] walltime = ["dep:criterion"] -# NOTE: benchmarks must be run with `--no-default-features` or with -# `-p builtins-test`, otherwise the default `compiler-builtins` feature -# of the `compiler_builtins` crate gets activated, resulting in linker -# errors. - [[bench]] name = "float_add" harness = false diff --git a/library/compiler-builtins/ci/bench-icount.sh b/library/compiler-builtins/ci/bench-icount.sh index e24a47fe07be..17b6997602da 100755 --- a/library/compiler-builtins/ci/bench-icount.sh +++ b/library/compiler-builtins/ci/bench-icount.sh @@ -79,7 +79,7 @@ function run_icount_benchmarks() { # Run once with softfloats, once with arch instructions enabled run_icount_benchmarks --features force-soft-floats -- --save-baseline=softfloat -run_icount_benchmarks -- --save-baseline=hardfloat +run_icount_benchmarks --features arch -- --save-baseline=hardfloat if [ "$failed" != "0" ]; then echo "One or more benchmarks failed" diff --git a/library/compiler-builtins/ci/miri.sh b/library/compiler-builtins/ci/miri.sh index aae474d88463..90b64934db0b 100755 --- a/library/compiler-builtins/ci/miri.sh +++ b/library/compiler-builtins/ci/miri.sh @@ -13,10 +13,11 @@ targets=( s390x-unknown-linux-gnu ) for target in "${targets[@]}"; do - # Only run the `mem` tests to avoid this taking too long. + # Only run the `mem` tests to avoid this taking too long. Disable default + # features to turn off `arch` and avoid inline assembly. cargo miri test \ --manifest-path builtins-test/Cargo.toml \ - --features no-asm \ + --no-default-features \ --target "$target" \ -- mem done diff --git a/library/compiler-builtins/ci/run.sh b/library/compiler-builtins/ci/run.sh index 5ddfbd038f5c..021bc6809e43 100755 --- a/library/compiler-builtins/ci/run.sh +++ b/library/compiler-builtins/ci/run.sh @@ -55,10 +55,10 @@ else "${test_builtins[@]}" --release "${test_builtins[@]}" --features c "${test_builtins[@]}" --features c --release - "${test_builtins[@]}" --features no-asm - "${test_builtins[@]}" --features no-asm --release "${test_builtins[@]}" --benches "${test_builtins[@]}" --benches --release + "${test_builtins[@]}" --no-default-features + "${test_builtins[@]}" --no-default-features --release # Validate that having a verbatim path for the target directory works # (trivial to regress using `/` in paths to build artifacts rather than @@ -85,8 +85,8 @@ symcheck_cb_args=(-- --package compiler_builtins --features compiler-builtins) "${symcheck[@]}" "${symcheck_cb_args[@]}" --release "${symcheck[@]}" "${symcheck_cb_args[@]}" --features c "${symcheck[@]}" "${symcheck_cb_args[@]}" --features c --release -"${symcheck[@]}" "${symcheck_cb_args[@]}" --features no-asm -"${symcheck[@]}" "${symcheck_cb_args[@]}" --features no-asm --release +"${symcheck[@]}" "${symcheck_cb_args[@]}" --no-default-features +"${symcheck[@]}" "${symcheck_cb_args[@]}" --no-default-features --release run_intrinsics_test() { build_args=(--verbose --manifest-path builtins-test-intrinsics/Cargo.toml) diff --git a/library/compiler-builtins/compiler-builtins/Cargo.toml b/library/compiler-builtins/compiler-builtins/Cargo.toml index 2299ae8e3aaf..aa4e8d2ab285 100644 --- a/library/compiler-builtins/compiler-builtins/Cargo.toml +++ b/library/compiler-builtins/compiler-builtins/Cargo.toml @@ -34,17 +34,16 @@ core = { path = "../../core", optional = true } cc = { version = "1.2", optional = true } [features] -default = [] +default = ["arch"] + +# Enable architecture-specific features such as SIMD or assembly routines. If +# disabled, the generic version can be tested on any platform. +arch = [] # Enable compilation of C code in compiler-rt, filling in some more optimized # implementations and also filling in unimplemented intrinsics c = ["dep:cc"] -# For implementations where there is both a generic version and a platform- -# specific version, use the generic version. This is meant to enable testing -# the generic versions on all platforms. -no-asm = [] - # Flag this library as the unstable compiler-builtins lib. This must be enabled # when using as `std`'s dependency.' compiler-builtins = ["dep:core", "unmangled-names"] diff --git a/library/compiler-builtins/compiler-builtins/build.rs b/library/compiler-builtins/compiler-builtins/build.rs index 3b1ad344e2a3..bdbc766de847 100644 --- a/library/compiler-builtins/compiler-builtins/build.rs +++ b/library/compiler-builtins/compiler-builtins/build.rs @@ -90,7 +90,7 @@ fn configure_libm(target: &Target) { set_cfg("intrinsics_enabled", true); // The arch module may contain assembly. - set_cfg("arch_enabled", !cfg!(feature = "no-asm")); + set_cfg("arch_enabled", cfg!(feature = "arch")); let opt = !matches!(target.opt_level.as_str(), "0" | "1"); set_cfg("optimizations_enabled", opt); diff --git a/library/compiler-builtins/compiler-builtins/src/int/specialized_div_rem/mod.rs b/library/compiler-builtins/compiler-builtins/src/int/specialized_div_rem/mod.rs index 5ffe1f59b4db..902c191d6528 100644 --- a/library/compiler-builtins/compiler-builtins/src/int/specialized_div_rem/mod.rs +++ b/library/compiler-builtins/compiler-builtins/src/int/specialized_div_rem/mod.rs @@ -144,7 +144,7 @@ fn u64_by_u64_div_rem(duo: u64, div: u64) -> (u64, u64) { target_family = "wasm", not(any(target_pointer_width = "16", target_pointer_width = "32")), ), - not(all(not(feature = "no-asm"), target_arch = "x86_64")), + not(all(feature = "arch", target_arch = "x86_64")), not(any(target_arch = "sparc", target_arch = "sparc64")) ))] impl_trifecta!( @@ -165,7 +165,7 @@ fn u64_by_u64_div_rem(duo: u64, div: u64) -> (u64, u64) { target_family = "wasm", not(any(target_pointer_width = "16", target_pointer_width = "32")), )), - not(all(not(feature = "no-asm"), target_arch = "x86_64")), + not(all(feature = "arch", target_arch = "x86_64")), not(any(target_arch = "sparc", target_arch = "sparc64")) ))] impl_delegate!( @@ -186,7 +186,7 @@ fn u64_by_u64_div_rem(duo: u64, div: u64) -> (u64, u64) { /// /// If the quotient does not fit in a `u64`, a floating point exception occurs. /// If `div == 0`, then a division by zero exception occurs. -#[cfg(all(not(feature = "no-asm"), target_arch = "x86_64"))] +#[cfg(all(feature = "arch", target_arch = "x86_64"))] #[inline] unsafe fn u128_by_u64_div_rem(duo: u128, div: u64) -> (u64, u64) { let duo_lo = duo as u64; @@ -208,7 +208,7 @@ unsafe fn u128_by_u64_div_rem(duo: u128, div: u64) -> (u64, u64) { } // use `asymmetric` instead of `trifecta` on x86_64 -#[cfg(all(not(feature = "no-asm"), target_arch = "x86_64"))] +#[cfg(all(feature = "arch", target_arch = "x86_64"))] impl_asymmetric!( u128_div_rem, zero_div_fn, @@ -237,7 +237,7 @@ fn u32_by_u32_div_rem(duo: u32, div: u32) -> (u32, u32) { // When not on x86 and the pointer width is not 64, use `delegate` since the division size is larger // than register size. #[cfg(all( - not(all(not(feature = "no-asm"), target_arch = "x86")), + not(all(feature = "arch", target_arch = "x86")), not(target_pointer_width = "64") ))] impl_delegate!( @@ -254,7 +254,7 @@ fn u32_by_u32_div_rem(duo: u32, div: u32) -> (u32, u32) { // When not on x86 and the pointer width is 64, use `binary_long`. #[cfg(all( - not(all(not(feature = "no-asm"), target_arch = "x86")), + not(all(feature = "arch", target_arch = "x86")), target_pointer_width = "64" ))] impl_binary_long!( @@ -272,7 +272,7 @@ fn u32_by_u32_div_rem(duo: u32, div: u32) -> (u32, u32) { /// /// If the quotient does not fit in a `u32`, a floating point exception occurs. /// If `div == 0`, then a division by zero exception occurs. -#[cfg(all(not(feature = "no-asm"), target_arch = "x86"))] +#[cfg(all(feature = "arch", target_arch = "x86"))] #[inline] unsafe fn u64_by_u32_div_rem(duo: u64, div: u32) -> (u32, u32) { let duo_lo = duo as u32; @@ -294,7 +294,7 @@ unsafe fn u64_by_u32_div_rem(duo: u64, div: u32) -> (u32, u32) { } // use `asymmetric` instead of `delegate` on x86 -#[cfg(all(not(feature = "no-asm"), target_arch = "x86"))] +#[cfg(all(feature = "arch", target_arch = "x86"))] impl_asymmetric!( u64_div_rem, zero_div_fn, diff --git a/library/compiler-builtins/compiler-builtins/src/mem/mod.rs b/library/compiler-builtins/compiler-builtins/src/mem/mod.rs index a227f60a2949..ac41cd33416f 100644 --- a/library/compiler-builtins/compiler-builtins/src/mem/mod.rs +++ b/library/compiler-builtins/compiler-builtins/src/mem/mod.rs @@ -4,10 +4,7 @@ #![allow(unsafe_op_in_unsafe_fn)] // memcpy/memmove/memset have optimized implementations on some architectures -#[cfg_attr( - all(not(feature = "no-asm"), target_arch = "x86_64"), - path = "x86_64.rs" -)] +#[cfg_attr(all(feature = "arch", target_arch = "x86_64"), path = "x86_64.rs")] mod impls; intrinsics! { diff --git a/library/compiler-builtins/libm-test/Cargo.toml b/library/compiler-builtins/libm-test/Cargo.toml index abc9bc6f75e1..9314c26831eb 100644 --- a/library/compiler-builtins/libm-test/Cargo.toml +++ b/library/compiler-builtins/libm-test/Cargo.toml @@ -36,7 +36,9 @@ rand = { workspace = true, optional = true } libtest-mimic.workspace = true [features] -default = ["build-mpfr", "unstable-float"] +# Defaults should match the defaults in compiler-builtins since we have that +# dependency with `default-features=false`. +default = ["build-mpfr", "unstable-float", "compiler_builtins/arch"] # Propagated from libm because this affects which functions we test. unstable-float = ["libm/unstable-float", "rug?/nightly-float"]