From 11c15229557cb489a5604f13e26a51099cc05314 Mon Sep 17 00:00:00 2001 From: beetrees Date: Wed, 21 May 2025 18:11:11 +0100 Subject: [PATCH 01/28] Enable `__powitf2` on MSVC --- library/compiler-builtins/builtins-test/tests/float_pow.rs | 2 -- library/compiler-builtins/compiler-builtins/src/float/pow.rs | 2 -- 2 files changed, 4 deletions(-) diff --git a/library/compiler-builtins/builtins-test/tests/float_pow.rs b/library/compiler-builtins/builtins-test/tests/float_pow.rs index 8209543e666a0..0e8ae88e83eff 100644 --- a/library/compiler-builtins/builtins-test/tests/float_pow.rs +++ b/library/compiler-builtins/builtins-test/tests/float_pow.rs @@ -58,8 +58,6 @@ pow! { } #[cfg(f128_enabled)] -// FIXME(f16_f128): MSVC cannot build these until `__divtf3` is available in nightly. -#[cfg(not(target_env = "msvc"))] #[cfg(not(any(target_arch = "powerpc", target_arch = "powerpc64")))] pow! { f128, 1e-36, __powitf2, not(feature = "no-sys-f128"); diff --git a/library/compiler-builtins/compiler-builtins/src/float/pow.rs b/library/compiler-builtins/compiler-builtins/src/float/pow.rs index 45a4ad9049de4..6997a9c213c59 100644 --- a/library/compiler-builtins/compiler-builtins/src/float/pow.rs +++ b/library/compiler-builtins/compiler-builtins/src/float/pow.rs @@ -32,8 +32,6 @@ intrinsics! { #[ppc_alias = __powikf2] #[cfg(f128_enabled)] - // FIXME(f16_f128): MSVC cannot build these until `__divtf3` is available in nightly. - #[cfg(not(target_env = "msvc"))] pub extern "C" fn __powitf2(a: f128, b: i32) -> f128 { pow(a, b) } From 7966f1b556f39882b3e451533a74a3b24a34cb05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1lyi=20L=C5=91rinc?= Date: Sat, 10 May 2025 08:36:28 +0000 Subject: [PATCH 02/28] fixed typo in readme --- library/compiler-builtins/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/compiler-builtins/README.md b/library/compiler-builtins/README.md index 3130ff7b77d9d..177bce624e0a1 100644 --- a/library/compiler-builtins/README.md +++ b/library/compiler-builtins/README.md @@ -5,7 +5,7 @@ This repository contains two main crates: * `compiler-builtins`: symbols that the compiler expects to be available at link time * `libm`: a Rust implementation of C math libraries, used to provide - implementations in `ocre`. + implementations in `core`. More details are at [compiler-builtins/README.md](compiler-builtins/README.md) and [libm/README.md](libm/README.md). From db21837095f18a76d1fecb7d55547688a6b19647 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Tue, 22 Apr 2025 19:35:21 -0400 Subject: [PATCH 03/28] libm: Clean up unused files These were deleted during refactoring in 0a2dc5d9 ("Combine the source files for more generic implementations") but got added back by accident in 54bac411 ("refactor: Move the libm crate to a subdirectory"). Remove them again here. --- .../libm/src/math/copysignf.rs | 8 ---- .../libm/src/math/copysignf128.rs | 8 ---- .../libm/src/math/copysignf16.rs | 8 ---- .../compiler-builtins/libm/src/math/fabsf.rs | 39 ------------------- .../libm/src/math/fabsf128.rs | 31 --------------- .../libm/src/math/fabsf16.rs | 31 --------------- .../compiler-builtins/libm/src/math/fdimf.rs | 12 ------ .../libm/src/math/fdimf128.rs | 12 ------ .../libm/src/math/fdimf16.rs | 12 ------ .../compiler-builtins/libm/src/math/floorf.rs | 13 ------- .../libm/src/math/floorf128.rs | 7 ---- .../libm/src/math/floorf16.rs | 7 ---- .../compiler-builtins/libm/src/math/fmodf.rs | 5 --- .../libm/src/math/fmodf128.rs | 5 --- .../libm/src/math/fmodf16.rs | 5 --- .../compiler-builtins/libm/src/math/ldexpf.rs | 4 -- .../libm/src/math/ldexpf128.rs | 4 -- .../libm/src/math/ldexpf16.rs | 4 -- .../compiler-builtins/libm/src/math/roundf.rs | 5 --- .../libm/src/math/roundf128.rs | 5 --- .../libm/src/math/roundf16.rs | 5 --- .../libm/src/math/scalbnf.rs | 4 -- .../libm/src/math/scalbnf128.rs | 4 -- .../libm/src/math/scalbnf16.rs | 4 -- .../compiler-builtins/libm/src/math/sqrtf.rs | 15 ------- .../libm/src/math/sqrtf128.rs | 5 --- .../libm/src/math/sqrtf16.rs | 11 ------ .../compiler-builtins/libm/src/math/truncf.rs | 23 ----------- .../libm/src/math/truncf128.rs | 7 ---- .../libm/src/math/truncf16.rs | 7 ---- 30 files changed, 310 deletions(-) delete mode 100644 library/compiler-builtins/libm/src/math/copysignf.rs delete mode 100644 library/compiler-builtins/libm/src/math/copysignf128.rs delete mode 100644 library/compiler-builtins/libm/src/math/copysignf16.rs delete mode 100644 library/compiler-builtins/libm/src/math/fabsf.rs delete mode 100644 library/compiler-builtins/libm/src/math/fabsf128.rs delete mode 100644 library/compiler-builtins/libm/src/math/fabsf16.rs delete mode 100644 library/compiler-builtins/libm/src/math/fdimf.rs delete mode 100644 library/compiler-builtins/libm/src/math/fdimf128.rs delete mode 100644 library/compiler-builtins/libm/src/math/fdimf16.rs delete mode 100644 library/compiler-builtins/libm/src/math/floorf.rs delete mode 100644 library/compiler-builtins/libm/src/math/floorf128.rs delete mode 100644 library/compiler-builtins/libm/src/math/floorf16.rs delete mode 100644 library/compiler-builtins/libm/src/math/fmodf.rs delete mode 100644 library/compiler-builtins/libm/src/math/fmodf128.rs delete mode 100644 library/compiler-builtins/libm/src/math/fmodf16.rs delete mode 100644 library/compiler-builtins/libm/src/math/ldexpf.rs delete mode 100644 library/compiler-builtins/libm/src/math/ldexpf128.rs delete mode 100644 library/compiler-builtins/libm/src/math/ldexpf16.rs delete mode 100644 library/compiler-builtins/libm/src/math/roundf.rs delete mode 100644 library/compiler-builtins/libm/src/math/roundf128.rs delete mode 100644 library/compiler-builtins/libm/src/math/roundf16.rs delete mode 100644 library/compiler-builtins/libm/src/math/scalbnf.rs delete mode 100644 library/compiler-builtins/libm/src/math/scalbnf128.rs delete mode 100644 library/compiler-builtins/libm/src/math/scalbnf16.rs delete mode 100644 library/compiler-builtins/libm/src/math/sqrtf.rs delete mode 100644 library/compiler-builtins/libm/src/math/sqrtf128.rs delete mode 100644 library/compiler-builtins/libm/src/math/sqrtf16.rs delete mode 100644 library/compiler-builtins/libm/src/math/truncf.rs delete mode 100644 library/compiler-builtins/libm/src/math/truncf128.rs delete mode 100644 library/compiler-builtins/libm/src/math/truncf16.rs diff --git a/library/compiler-builtins/libm/src/math/copysignf.rs b/library/compiler-builtins/libm/src/math/copysignf.rs deleted file mode 100644 index 8b9bed4c0c427..0000000000000 --- a/library/compiler-builtins/libm/src/math/copysignf.rs +++ /dev/null @@ -1,8 +0,0 @@ -/// Sign of Y, magnitude of X (f32) -/// -/// Constructs a number with the magnitude (absolute value) of its -/// first argument, `x`, and the sign of its second argument, `y`. -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn copysignf(x: f32, y: f32) -> f32 { - super::generic::copysign(x, y) -} diff --git a/library/compiler-builtins/libm/src/math/copysignf128.rs b/library/compiler-builtins/libm/src/math/copysignf128.rs deleted file mode 100644 index 7bd81d42b2e9a..0000000000000 --- a/library/compiler-builtins/libm/src/math/copysignf128.rs +++ /dev/null @@ -1,8 +0,0 @@ -/// Sign of Y, magnitude of X (f128) -/// -/// Constructs a number with the magnitude (absolute value) of its -/// first argument, `x`, and the sign of its second argument, `y`. -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn copysignf128(x: f128, y: f128) -> f128 { - super::generic::copysign(x, y) -} diff --git a/library/compiler-builtins/libm/src/math/copysignf16.rs b/library/compiler-builtins/libm/src/math/copysignf16.rs deleted file mode 100644 index 8206586860102..0000000000000 --- a/library/compiler-builtins/libm/src/math/copysignf16.rs +++ /dev/null @@ -1,8 +0,0 @@ -/// Sign of Y, magnitude of X (f16) -/// -/// Constructs a number with the magnitude (absolute value) of its -/// first argument, `x`, and the sign of its second argument, `y`. -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn copysignf16(x: f16, y: f16) -> f16 { - super::generic::copysign(x, y) -} diff --git a/library/compiler-builtins/libm/src/math/fabsf.rs b/library/compiler-builtins/libm/src/math/fabsf.rs deleted file mode 100644 index e5820a26c5238..0000000000000 --- a/library/compiler-builtins/libm/src/math/fabsf.rs +++ /dev/null @@ -1,39 +0,0 @@ -/// Absolute value (magnitude) (f32) -/// -/// Calculates the absolute value (magnitude) of the argument `x`, -/// by direct manipulation of the bit representation of `x`. -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn fabsf(x: f32) -> f32 { - select_implementation! { - name: fabsf, - use_arch: all(target_arch = "wasm32", intrinsics_enabled), - args: x, - } - - super::generic::fabs(x) -} - -// PowerPC tests are failing on LLVM 13: https://github.com/rust-lang/rust/issues/88520 -#[cfg(not(target_arch = "powerpc64"))] -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn sanity_check() { - assert_eq!(fabsf(-1.0), 1.0); - assert_eq!(fabsf(2.8), 2.8); - } - - /// The spec: https://en.cppreference.com/w/cpp/numeric/math/fabs - #[test] - fn spec_tests() { - assert!(fabsf(f32::NAN).is_nan()); - for f in [0.0, -0.0].iter().copied() { - assert_eq!(fabsf(f), 0.0); - } - for f in [f32::INFINITY, f32::NEG_INFINITY].iter().copied() { - assert_eq!(fabsf(f), f32::INFINITY); - } - } -} diff --git a/library/compiler-builtins/libm/src/math/fabsf128.rs b/library/compiler-builtins/libm/src/math/fabsf128.rs deleted file mode 100644 index 46429ca494033..0000000000000 --- a/library/compiler-builtins/libm/src/math/fabsf128.rs +++ /dev/null @@ -1,31 +0,0 @@ -/// Absolute value (magnitude) (f128) -/// -/// Calculates the absolute value (magnitude) of the argument `x`, -/// by direct manipulation of the bit representation of `x`. -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn fabsf128(x: f128) -> f128 { - super::generic::fabs(x) -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn sanity_check() { - assert_eq!(fabsf128(-1.0), 1.0); - assert_eq!(fabsf128(2.8), 2.8); - } - - /// The spec: https://en.cppreference.com/w/cpp/numeric/math/fabs - #[test] - fn spec_tests() { - assert!(fabsf128(f128::NAN).is_nan()); - for f in [0.0, -0.0].iter().copied() { - assert_eq!(fabsf128(f), 0.0); - } - for f in [f128::INFINITY, f128::NEG_INFINITY].iter().copied() { - assert_eq!(fabsf128(f), f128::INFINITY); - } - } -} diff --git a/library/compiler-builtins/libm/src/math/fabsf16.rs b/library/compiler-builtins/libm/src/math/fabsf16.rs deleted file mode 100644 index eee42ac6a3c60..0000000000000 --- a/library/compiler-builtins/libm/src/math/fabsf16.rs +++ /dev/null @@ -1,31 +0,0 @@ -/// Absolute value (magnitude) (f16) -/// -/// Calculates the absolute value (magnitude) of the argument `x`, -/// by direct manipulation of the bit representation of `x`. -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn fabsf16(x: f16) -> f16 { - super::generic::fabs(x) -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn sanity_check() { - assert_eq!(fabsf16(-1.0), 1.0); - assert_eq!(fabsf16(2.8), 2.8); - } - - /// The spec: https://en.cppreference.com/w/cpp/numeric/math/fabs - #[test] - fn spec_tests() { - assert!(fabsf16(f16::NAN).is_nan()); - for f in [0.0, -0.0].iter().copied() { - assert_eq!(fabsf16(f), 0.0); - } - for f in [f16::INFINITY, f16::NEG_INFINITY].iter().copied() { - assert_eq!(fabsf16(f), f16::INFINITY); - } - } -} diff --git a/library/compiler-builtins/libm/src/math/fdimf.rs b/library/compiler-builtins/libm/src/math/fdimf.rs deleted file mode 100644 index 367ef517c63be..0000000000000 --- a/library/compiler-builtins/libm/src/math/fdimf.rs +++ /dev/null @@ -1,12 +0,0 @@ -/// Positive difference (f32) -/// -/// Determines the positive difference between arguments, returning: -/// * x - y if x > y, or -/// * +0 if x <= y, or -/// * NAN if either argument is NAN. -/// -/// A range error may occur. -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn fdimf(x: f32, y: f32) -> f32 { - super::generic::fdim(x, y) -} diff --git a/library/compiler-builtins/libm/src/math/fdimf128.rs b/library/compiler-builtins/libm/src/math/fdimf128.rs deleted file mode 100644 index 6f3d1d0ff1d54..0000000000000 --- a/library/compiler-builtins/libm/src/math/fdimf128.rs +++ /dev/null @@ -1,12 +0,0 @@ -/// Positive difference (f128) -/// -/// Determines the positive difference between arguments, returning: -/// * x - y if x > y, or -/// * +0 if x <= y, or -/// * NAN if either argument is NAN. -/// -/// A range error may occur. -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn fdimf128(x: f128, y: f128) -> f128 { - super::generic::fdim(x, y) -} diff --git a/library/compiler-builtins/libm/src/math/fdimf16.rs b/library/compiler-builtins/libm/src/math/fdimf16.rs deleted file mode 100644 index 37bd688581797..0000000000000 --- a/library/compiler-builtins/libm/src/math/fdimf16.rs +++ /dev/null @@ -1,12 +0,0 @@ -/// Positive difference (f16) -/// -/// Determines the positive difference between arguments, returning: -/// * x - y if x > y, or -/// * +0 if x <= y, or -/// * NAN if either argument is NAN. -/// -/// A range error may occur. -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn fdimf16(x: f16, y: f16) -> f16 { - super::generic::fdim(x, y) -} diff --git a/library/compiler-builtins/libm/src/math/floorf.rs b/library/compiler-builtins/libm/src/math/floorf.rs deleted file mode 100644 index 16957b7f35573..0000000000000 --- a/library/compiler-builtins/libm/src/math/floorf.rs +++ /dev/null @@ -1,13 +0,0 @@ -/// Floor (f32) -/// -/// Finds the nearest integer less than or equal to `x`. -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn floorf(x: f32) -> f32 { - select_implementation! { - name: floorf, - use_arch: all(target_arch = "wasm32", intrinsics_enabled), - args: x, - } - - return super::generic::floor(x); -} diff --git a/library/compiler-builtins/libm/src/math/floorf128.rs b/library/compiler-builtins/libm/src/math/floorf128.rs deleted file mode 100644 index 9a9fe4151152b..0000000000000 --- a/library/compiler-builtins/libm/src/math/floorf128.rs +++ /dev/null @@ -1,7 +0,0 @@ -/// Floor (f128) -/// -/// Finds the nearest integer less than or equal to `x`. -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn floorf128(x: f128) -> f128 { - return super::generic::floor(x); -} diff --git a/library/compiler-builtins/libm/src/math/floorf16.rs b/library/compiler-builtins/libm/src/math/floorf16.rs deleted file mode 100644 index f9b868e04109d..0000000000000 --- a/library/compiler-builtins/libm/src/math/floorf16.rs +++ /dev/null @@ -1,7 +0,0 @@ -/// Floor (f16) -/// -/// Finds the nearest integer less than or equal to `x`. -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn floorf16(x: f16) -> f16 { - return super::generic::floor(x); -} diff --git a/library/compiler-builtins/libm/src/math/fmodf.rs b/library/compiler-builtins/libm/src/math/fmodf.rs deleted file mode 100644 index 4e95696e20d63..0000000000000 --- a/library/compiler-builtins/libm/src/math/fmodf.rs +++ /dev/null @@ -1,5 +0,0 @@ -/// Calculate the remainder of `x / y`, the precise result of `x - trunc(x / y) * y`. -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn fmodf(x: f32, y: f32) -> f32 { - super::generic::fmod(x, y) -} diff --git a/library/compiler-builtins/libm/src/math/fmodf128.rs b/library/compiler-builtins/libm/src/math/fmodf128.rs deleted file mode 100644 index ff0e0493e26b6..0000000000000 --- a/library/compiler-builtins/libm/src/math/fmodf128.rs +++ /dev/null @@ -1,5 +0,0 @@ -/// Calculate the remainder of `x / y`, the precise result of `x - trunc(x / y) * y`. -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn fmodf128(x: f128, y: f128) -> f128 { - super::generic::fmod(x, y) -} diff --git a/library/compiler-builtins/libm/src/math/fmodf16.rs b/library/compiler-builtins/libm/src/math/fmodf16.rs deleted file mode 100644 index 11972a7de4ff0..0000000000000 --- a/library/compiler-builtins/libm/src/math/fmodf16.rs +++ /dev/null @@ -1,5 +0,0 @@ -/// Calculate the remainder of `x / y`, the precise result of `x - trunc(x / y) * y`. -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn fmodf16(x: f16, y: f16) -> f16 { - super::generic::fmod(x, y) -} diff --git a/library/compiler-builtins/libm/src/math/ldexpf.rs b/library/compiler-builtins/libm/src/math/ldexpf.rs deleted file mode 100644 index 95b27fc49d28e..0000000000000 --- a/library/compiler-builtins/libm/src/math/ldexpf.rs +++ /dev/null @@ -1,4 +0,0 @@ -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn ldexpf(x: f32, n: i32) -> f32 { - super::scalbnf(x, n) -} diff --git a/library/compiler-builtins/libm/src/math/ldexpf128.rs b/library/compiler-builtins/libm/src/math/ldexpf128.rs deleted file mode 100644 index b35277d15fbae..0000000000000 --- a/library/compiler-builtins/libm/src/math/ldexpf128.rs +++ /dev/null @@ -1,4 +0,0 @@ -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn ldexpf128(x: f128, n: i32) -> f128 { - super::scalbnf128(x, n) -} diff --git a/library/compiler-builtins/libm/src/math/ldexpf16.rs b/library/compiler-builtins/libm/src/math/ldexpf16.rs deleted file mode 100644 index 8de6cffd69987..0000000000000 --- a/library/compiler-builtins/libm/src/math/ldexpf16.rs +++ /dev/null @@ -1,4 +0,0 @@ -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn ldexpf16(x: f16, n: i32) -> f16 { - super::scalbnf16(x, n) -} diff --git a/library/compiler-builtins/libm/src/math/roundf.rs b/library/compiler-builtins/libm/src/math/roundf.rs deleted file mode 100644 index b5d7c9d693e71..0000000000000 --- a/library/compiler-builtins/libm/src/math/roundf.rs +++ /dev/null @@ -1,5 +0,0 @@ -/// Round `x` to the nearest integer, breaking ties away from zero. -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn roundf(x: f32) -> f32 { - super::generic::round(x) -} diff --git a/library/compiler-builtins/libm/src/math/roundf128.rs b/library/compiler-builtins/libm/src/math/roundf128.rs deleted file mode 100644 index fc3164929fe4f..0000000000000 --- a/library/compiler-builtins/libm/src/math/roundf128.rs +++ /dev/null @@ -1,5 +0,0 @@ -/// Round `x` to the nearest integer, breaking ties away from zero. -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn roundf128(x: f128) -> f128 { - super::generic::round(x) -} diff --git a/library/compiler-builtins/libm/src/math/roundf16.rs b/library/compiler-builtins/libm/src/math/roundf16.rs deleted file mode 100644 index 8b356eaabeecd..0000000000000 --- a/library/compiler-builtins/libm/src/math/roundf16.rs +++ /dev/null @@ -1,5 +0,0 @@ -/// Round `x` to the nearest integer, breaking ties away from zero. -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn roundf16(x: f16) -> f16 { - super::generic::round(x) -} diff --git a/library/compiler-builtins/libm/src/math/scalbnf.rs b/library/compiler-builtins/libm/src/math/scalbnf.rs deleted file mode 100644 index 57e7ba76f60b5..0000000000000 --- a/library/compiler-builtins/libm/src/math/scalbnf.rs +++ /dev/null @@ -1,4 +0,0 @@ -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn scalbnf(x: f32, n: i32) -> f32 { - super::generic::scalbn(x, n) -} diff --git a/library/compiler-builtins/libm/src/math/scalbnf128.rs b/library/compiler-builtins/libm/src/math/scalbnf128.rs deleted file mode 100644 index c1d2b48558568..0000000000000 --- a/library/compiler-builtins/libm/src/math/scalbnf128.rs +++ /dev/null @@ -1,4 +0,0 @@ -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn scalbnf128(x: f128, n: i32) -> f128 { - super::generic::scalbn(x, n) -} diff --git a/library/compiler-builtins/libm/src/math/scalbnf16.rs b/library/compiler-builtins/libm/src/math/scalbnf16.rs deleted file mode 100644 index 2209e1a179566..0000000000000 --- a/library/compiler-builtins/libm/src/math/scalbnf16.rs +++ /dev/null @@ -1,4 +0,0 @@ -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn scalbnf16(x: f16, n: i32) -> f16 { - super::generic::scalbn(x, n) -} diff --git a/library/compiler-builtins/libm/src/math/sqrtf.rs b/library/compiler-builtins/libm/src/math/sqrtf.rs deleted file mode 100644 index c28a705e378e6..0000000000000 --- a/library/compiler-builtins/libm/src/math/sqrtf.rs +++ /dev/null @@ -1,15 +0,0 @@ -/// The square root of `x` (f32). -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn sqrtf(x: f32) -> f32 { - select_implementation! { - name: sqrtf, - use_arch: any( - all(target_arch = "aarch64", target_feature = "neon"), - all(target_arch = "wasm32", intrinsics_enabled), - target_feature = "sse2" - ), - args: x, - } - - super::generic::sqrt(x) -} diff --git a/library/compiler-builtins/libm/src/math/sqrtf128.rs b/library/compiler-builtins/libm/src/math/sqrtf128.rs deleted file mode 100644 index eaef6ae0c1c85..0000000000000 --- a/library/compiler-builtins/libm/src/math/sqrtf128.rs +++ /dev/null @@ -1,5 +0,0 @@ -/// The square root of `x` (f128). -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn sqrtf128(x: f128) -> f128 { - return super::generic::sqrt(x); -} diff --git a/library/compiler-builtins/libm/src/math/sqrtf16.rs b/library/compiler-builtins/libm/src/math/sqrtf16.rs deleted file mode 100644 index 7bedb7f8bbb6b..0000000000000 --- a/library/compiler-builtins/libm/src/math/sqrtf16.rs +++ /dev/null @@ -1,11 +0,0 @@ -/// The square root of `x` (f16). -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn sqrtf16(x: f16) -> f16 { - select_implementation! { - name: sqrtf16, - use_arch: all(target_arch = "aarch64", target_feature = "fp16"), - args: x, - } - - return super::generic::sqrt(x); -} diff --git a/library/compiler-builtins/libm/src/math/truncf.rs b/library/compiler-builtins/libm/src/math/truncf.rs deleted file mode 100644 index 14533a2670632..0000000000000 --- a/library/compiler-builtins/libm/src/math/truncf.rs +++ /dev/null @@ -1,23 +0,0 @@ -/// Rounds the number toward 0 to the closest integral value (f32). -/// -/// This effectively removes the decimal part of the number, leaving the integral part. -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn truncf(x: f32) -> f32 { - select_implementation! { - name: truncf, - use_arch: all(target_arch = "wasm32", intrinsics_enabled), - args: x, - } - - super::generic::trunc(x) -} - -// PowerPC tests are failing on LLVM 13: https://github.com/rust-lang/rust/issues/88520 -#[cfg(not(target_arch = "powerpc64"))] -#[cfg(test)] -mod tests { - #[test] - fn sanity_check() { - assert_eq!(super::truncf(1.1), 1.0); - } -} diff --git a/library/compiler-builtins/libm/src/math/truncf128.rs b/library/compiler-builtins/libm/src/math/truncf128.rs deleted file mode 100644 index 9dccc0d0e9d7f..0000000000000 --- a/library/compiler-builtins/libm/src/math/truncf128.rs +++ /dev/null @@ -1,7 +0,0 @@ -/// Rounds the number toward 0 to the closest integral value (f128). -/// -/// This effectively removes the decimal part of the number, leaving the integral part. -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn truncf128(x: f128) -> f128 { - super::generic::trunc(x) -} diff --git a/library/compiler-builtins/libm/src/math/truncf16.rs b/library/compiler-builtins/libm/src/math/truncf16.rs deleted file mode 100644 index d7c3d225cf9b8..0000000000000 --- a/library/compiler-builtins/libm/src/math/truncf16.rs +++ /dev/null @@ -1,7 +0,0 @@ -/// Rounds the number toward 0 to the closest integral value (f16). -/// -/// This effectively removes the decimal part of the number, leaving the integral part. -#[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] -pub fn truncf16(x: f16) -> f16 { - super::generic::trunc(x) -} From 4c264c96ae30e45c503fbb24e452ea948c5229af Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Fri, 23 May 2025 17:26:39 +0000 Subject: [PATCH 04/28] Update `CmpResult` to use a pointer-sized return type As seen at [1], LLVM uses `long long` on LLP64 (to get a 64-bit integer matching pointer size) and `long` on everything else, with exceptions for AArch64 and AVR. Our current logic always uses an `i32`. This happens to work because LLVM uses 32-bit instructions to check the output on x86-64, but the GCC checks the full 64-bit register so garbage in the upper half leads to incorrect results. Update our return type to be `isize`, with exceptions for AArch64 and AVR. Fixes: https://github.com/rust-lang/compiler-builtins/issues/919 [1]: https://github.com/llvm/llvm-project/blob/0cf3c437c18ed27d9663d87804a9a15ff6874af2/compiler-rt/lib/builtins/fp_compare_impl.inc#L11-L27 --- .../builtins-test/benches/float_cmp.rs | 43 ++++++++++++------- .../builtins-test/src/bench.rs | 4 +- .../compiler-builtins/src/float/cmp.rs | 25 +++++++---- .../libm/src/math/support/mod.rs | 2 + 4 files changed, 48 insertions(+), 26 deletions(-) diff --git a/library/compiler-builtins/builtins-test/benches/float_cmp.rs b/library/compiler-builtins/builtins-test/benches/float_cmp.rs index 42d6652397dcf..87a89efb5a4fe 100644 --- a/library/compiler-builtins/builtins-test/benches/float_cmp.rs +++ b/library/compiler-builtins/builtins-test/benches/float_cmp.rs @@ -1,12 +1,23 @@ #![cfg_attr(f128_enabled, feature(f128))] use builtins_test::float_bench; -use compiler_builtins::float::cmp; +use compiler_builtins::float::cmp::{self, CmpResult}; use criterion::{Criterion, criterion_main}; /// `gt` symbols are allowed to return differing results, they just get compared /// to 0. -fn gt_res_eq(a: i32, b: i32) -> bool { +fn gt_res_eq(mut a: CmpResult, mut b: CmpResult) -> bool { + // FIXME: Our CmpResult used to be `i32`, but GCC/LLVM expect `isize`. on 64-bit platforms, + // this means the top half of the word may be garbage if built with an old version of + // `compiler-builtins`, so add a hack around this. + // + // This can be removed once a version of `compiler-builtins` with the return type fix makes + // it upstream. + if size_of::() == 8 { + a = a as i32 as CmpResult; + b = b as i32 as CmpResult; + } + let a_lt_0 = a <= 0; let b_lt_0 = b <= 0; (a_lt_0 && b_lt_0) || (!a_lt_0 && !b_lt_0) @@ -14,14 +25,14 @@ fn gt_res_eq(a: i32, b: i32) -> bool { float_bench! { name: cmp_f32_gt, - sig: (a: f32, b: f32) -> i32, + sig: (a: f32, b: f32) -> CmpResult, crate_fn: cmp::__gtsf2, sys_fn: __gtsf2, sys_available: all(), output_eq: gt_res_eq, asm: [ #[cfg(target_arch = "x86_64")] { - let ret: i32; + let ret: CmpResult; asm!( "xor {ret:e}, {ret:e}", "ucomiss {a}, {b}", @@ -36,7 +47,7 @@ float_bench! { }; #[cfg(target_arch = "aarch64")] { - let ret: i32; + let ret: CmpResult; asm!( "fcmp {a:s}, {b:s}", "cset {ret:w}, gt", @@ -53,13 +64,13 @@ float_bench! { float_bench! { name: cmp_f32_unord, - sig: (a: f32, b: f32) -> i32, + sig: (a: f32, b: f32) -> CmpResult, crate_fn: cmp::__unordsf2, sys_fn: __unordsf2, sys_available: all(), asm: [ #[cfg(target_arch = "x86_64")] { - let ret: i32; + let ret: CmpResult; asm!( "xor {ret:e}, {ret:e}", "ucomiss {a}, {b}", @@ -74,7 +85,7 @@ float_bench! { }; #[cfg(target_arch = "aarch64")] { - let ret: i32; + let ret: CmpResult; asm!( "fcmp {a:s}, {b:s}", "cset {ret:w}, vs", @@ -91,14 +102,14 @@ float_bench! { float_bench! { name: cmp_f64_gt, - sig: (a: f64, b: f64) -> i32, + sig: (a: f64, b: f64) -> CmpResult, crate_fn: cmp::__gtdf2, sys_fn: __gtdf2, sys_available: all(), output_eq: gt_res_eq, asm: [ #[cfg(target_arch = "x86_64")] { - let ret: i32; + let ret: CmpResult; asm!( "xor {ret:e}, {ret:e}", "ucomisd {a}, {b}", @@ -113,7 +124,7 @@ float_bench! { }; #[cfg(target_arch = "aarch64")] { - let ret: i32; + let ret: CmpResult; asm!( "fcmp {a:d}, {b:d}", "cset {ret:w}, gt", @@ -130,13 +141,13 @@ float_bench! { float_bench! { name: cmp_f64_unord, - sig: (a: f64, b: f64) -> i32, + sig: (a: f64, b: f64) -> CmpResult, crate_fn: cmp::__unorddf2, sys_fn: __unorddf2, sys_available: all(), asm: [ #[cfg(target_arch = "x86_64")] { - let ret: i32; + let ret: CmpResult; asm!( "xor {ret:e}, {ret:e}", "ucomisd {a}, {b}", @@ -151,7 +162,7 @@ float_bench! { }; #[cfg(target_arch = "aarch64")] { - let ret: i32; + let ret: CmpResult; asm!( "fcmp {a:d}, {b:d}", "cset {ret:w}, vs", @@ -168,7 +179,7 @@ float_bench! { float_bench! { name: cmp_f128_gt, - sig: (a: f128, b: f128) -> i32, + sig: (a: f128, b: f128) -> CmpResult, crate_fn: cmp::__gttf2, crate_fn_ppc: cmp::__gtkf2, sys_fn: __gttf2, @@ -180,7 +191,7 @@ float_bench! { float_bench! { name: cmp_f128_unord, - sig: (a: f128, b: f128) -> i32, + sig: (a: f128, b: f128) -> CmpResult, crate_fn: cmp::__unordtf2, crate_fn_ppc: cmp::__unordkf2, sys_fn: __unordtf2, diff --git a/library/compiler-builtins/builtins-test/src/bench.rs b/library/compiler-builtins/builtins-test/src/bench.rs index 2348f6bc97379..0987185670ee3 100644 --- a/library/compiler-builtins/builtins-test/src/bench.rs +++ b/library/compiler-builtins/builtins-test/src/bench.rs @@ -358,8 +358,8 @@ impl_testio!(float f16); impl_testio!(float f32, f64); #[cfg(f128_enabled)] impl_testio!(float f128); -impl_testio!(int i16, i32, i64, i128); -impl_testio!(int u16, u32, u64, u128); +impl_testio!(int i8, i16, i32, i64, i128, isize); +impl_testio!(int u8, u16, u32, u64, u128, usize); impl_testio!((float, int)(f32, i32)); impl_testio!((float, int)(f64, i32)); #[cfg(f128_enabled)] diff --git a/library/compiler-builtins/compiler-builtins/src/float/cmp.rs b/library/compiler-builtins/compiler-builtins/src/float/cmp.rs index 296952821cb46..f1e54dc1c83d1 100644 --- a/library/compiler-builtins/compiler-builtins/src/float/cmp.rs +++ b/library/compiler-builtins/compiler-builtins/src/float/cmp.rs @@ -2,14 +2,23 @@ use crate::float::Float; use crate::int::MinInt; - -// https://github.com/llvm/llvm-project/blob/1e6ba3cd2fe96be00b6ed6ba28b3d9f9271d784d/compiler-rt/lib/builtins/fp_compare_impl.inc#L22 -#[cfg(target_arch = "avr")] -pub type CmpResult = i8; - -// https://github.com/llvm/llvm-project/blob/1e6ba3cd2fe96be00b6ed6ba28b3d9f9271d784d/compiler-rt/lib/builtins/fp_compare_impl.inc#L25 -#[cfg(not(target_arch = "avr"))] -pub type CmpResult = i32; +use crate::support::cfg_if; + +// Taken from LLVM config: +// https://github.com/llvm/llvm-project/blob/0cf3c437c18ed27d9663d87804a9a15ff6874af2/compiler-rt/lib/builtins/fp_compare_impl.inc#L11-L27 +cfg_if! { + if #[cfg(any(target_arch = "aarch64", target_arch = "arm64ec"))] { + // Aarch64 uses `int` rather than a pointer-sized value. + pub type CmpResult = i32; + } else if #[cfg(target_arch = "avr")] { + // AVR uses a single byte. + pub type CmpResult = i8; + } else { + // In compiler-rt, LLP64 ABIs use `long long` and everything else uses `long`. In effect, + // this means the return value is always pointer-sized. + pub type CmpResult = isize; + } +} #[derive(Clone, Copy)] enum Result { diff --git a/library/compiler-builtins/libm/src/math/support/mod.rs b/library/compiler-builtins/libm/src/math/support/mod.rs index a4f596ab8442f..2771cfd32c8b5 100644 --- a/library/compiler-builtins/libm/src/math/support/mod.rs +++ b/library/compiler-builtins/libm/src/math/support/mod.rs @@ -11,6 +11,8 @@ mod int_traits; #[allow(unused_imports)] pub use big::{i256, u256}; +#[allow(unused_imports)] +pub(crate) use cfg_if; pub use env::{FpResult, Round, Status}; #[allow(unused_imports)] pub use float_traits::{DFloat, Float, HFloat, IntTy}; From c04f133858c3208525ae8fe47057fb157c18d606 Mon Sep 17 00:00:00 2001 From: Dario Damiani <154735680+D-Dario0@users.noreply.github.com> Date: Wed, 28 May 2025 20:48:05 +0200 Subject: [PATCH 05/28] Typo in README.md Link to Apache License changed from htps:// to https:// --- library/compiler-builtins/libm/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/compiler-builtins/libm/README.md b/library/compiler-builtins/libm/README.md index 349e892dfcf9c..77608db3d0d78 100644 --- a/library/compiler-builtins/libm/README.md +++ b/library/compiler-builtins/libm/README.md @@ -34,7 +34,7 @@ Usage is under the MIT license, available at ### Contribution Contributions are licensed under both the MIT license and the Apache License, -Version 2.0, available at . Unless +Version 2.0, available at . Unless you explicitly state otherwise, any contribution intentionally submitted for inclusion in the work by you, as defined in the Apache-2.0 license, shall be dual licensed as mentioned, without any additional terms or conditions. From 5978b8b875f6c53af9ca4c72f4ea5fcf74d55c84 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Wed, 28 May 2025 21:08:41 +0000 Subject: [PATCH 06/28] aarch64: Add a note saying why we use `frintx` rather than `frintn` --- library/compiler-builtins/libm/src/math/arch/aarch64.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/library/compiler-builtins/libm/src/math/arch/aarch64.rs b/library/compiler-builtins/libm/src/math/arch/aarch64.rs index 020bb731cdc7a..8896804b50403 100644 --- a/library/compiler-builtins/libm/src/math/arch/aarch64.rs +++ b/library/compiler-builtins/libm/src/math/arch/aarch64.rs @@ -30,6 +30,12 @@ pub fn fmaf(mut x: f32, y: f32, z: f32) -> f32 { x } +// NB: `frintx` is technically the correct instruction for C's `rint`. However, in Rust (and LLVM +// by default), `rint` is identical to `roundeven` (no fpenv interaction) so we use the +// side-effect-free `frintn`. +// +// In general, C code that calls Rust's libm should assume that fpenv is ignored. + pub fn rint(mut x: f64) -> f64 { // SAFETY: `frintn` is available with neon and has no side effects. // From 851aa05aa0addf41507967952f1ac2d183751682 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Mon, 21 Apr 2025 09:35:55 +0000 Subject: [PATCH 07/28] cleanup: Reuse `MinInt` and `Int` from `libm` in `compiler-builtins` Since the two crates are now in the same repo, it is easier to share code. Begin some deduplication with the integer traits. --- .../builtins-test/src/lib.rs | 78 ++++- .../compiler-builtins/src/float/add.rs | 22 +- .../compiler-builtins/src/float/conv.rs | 24 +- .../compiler-builtins/src/float/div.rs | 2 +- .../compiler-builtins/src/float/mul.rs | 2 +- .../compiler-builtins/src/float/traits.rs | 4 +- .../compiler-builtins/src/int/addsub.rs | 6 +- .../compiler-builtins/src/int/big.rs | 4 +- .../src/int/leading_zeros.rs | 64 ++-- .../src/int/trailing_zeros.rs | 25 +- .../compiler-builtins/src/int/traits.rs | 273 +----------------- .../libm/src/math/support/int_traits.rs | 9 + 12 files changed, 168 insertions(+), 345 deletions(-) diff --git a/library/compiler-builtins/builtins-test/src/lib.rs b/library/compiler-builtins/builtins-test/src/lib.rs index c596ac2138076..f1673133be27d 100644 --- a/library/compiler-builtins/builtins-test/src/lib.rs +++ b/library/compiler-builtins/builtins-test/src/lib.rs @@ -40,6 +40,75 @@ pub const N: u32 = if cfg!(target_arch = "x86_64") && !cfg!(debug_assertions) { 10_000 }; +/// Additional constants that determine how the integer gets fuzzed. +trait FuzzInt: MinInt { + /// LUT used for maximizing the space covered and minimizing the computational cost of fuzzing + /// in `builtins-test`. For example, Self = u128 produces [0,1,2,7,8,15,16,31,32,63,64,95,96, + /// 111,112,119,120,125,126,127]. + const FUZZ_LENGTHS: [u8; 20] = make_fuzz_lengths(Self::BITS); + + /// The number of entries of `FUZZ_LENGTHS` actually used. The maximum is 20 for u128. + const FUZZ_NUM: usize = { + let log2 = Self::BITS.ilog2() as usize; + if log2 == 3 { + // case for u8 + 6 + } else { + // 3 entries on each extreme, 2 in the middle, and 4 for each scale of intermediate + // boundaries. + 8 + (4 * (log2 - 4)) + } + }; +} + +impl FuzzInt for I where I: MinInt {} + +const fn make_fuzz_lengths(bits: u32) -> [u8; 20] { + let mut v = [0u8; 20]; + v[0] = 0; + v[1] = 1; + v[2] = 2; // important for parity and the iX::MIN case when reversed + let mut i = 3; + + // No need for any more until the byte boundary, because there should be no algorithms + // that are sensitive to anything not next to byte boundaries after 2. We also scale + // in powers of two, which is important to prevent u128 corner tests from getting too + // big. + let mut l = 8; + loop { + if l >= ((bits / 2) as u8) { + break; + } + // get both sides of the byte boundary + v[i] = l - 1; + i += 1; + v[i] = l; + i += 1; + l *= 2; + } + + if bits != 8 { + // add the lower side of the middle boundary + v[i] = ((bits / 2) - 1) as u8; + i += 1; + } + + // We do not want to jump directly from the Self::BITS/2 boundary to the Self::BITS + // boundary because of algorithms that split the high part up. We reverse the scaling + // as we go to Self::BITS. + let mid = i; + let mut j = 1; + loop { + v[i] = (bits as u8) - (v[mid - j]) - 1; + if j == mid { + break; + } + i += 1; + j += 1; + } + v +} + /// Random fuzzing step. When run several times, it results in excellent fuzzing entropy such as: /// 11110101010101011110111110011111 /// 10110101010100001011101011001010 @@ -92,10 +161,9 @@ fn fuzz_step(rng: &mut Xoshiro128StarStar, x: &mut I) { macro_rules! edge_cases { ($I:ident, $case:ident, $inner:block) => { for i0 in 0..$I::FUZZ_NUM { - let mask_lo = (!$I::UnsignedInt::ZERO).wrapping_shr($I::FUZZ_LENGTHS[i0] as u32); + let mask_lo = (!$I::Unsigned::ZERO).wrapping_shr($I::FUZZ_LENGTHS[i0] as u32); for i1 in i0..I::FUZZ_NUM { - let mask_hi = - (!$I::UnsignedInt::ZERO).wrapping_shl($I::FUZZ_LENGTHS[i1 - i0] as u32); + let mask_hi = (!$I::Unsigned::ZERO).wrapping_shl($I::FUZZ_LENGTHS[i1 - i0] as u32); let $case = I::from_unsigned(mask_lo & mask_hi); $inner } @@ -107,7 +175,7 @@ macro_rules! edge_cases { /// edge cases, followed by a more random fuzzer that runs `n` times. pub fn fuzz(n: u32, mut f: F) where - ::UnsignedInt: Int, + ::Unsigned: Int, { // edge case tester. Calls `f` 210 times for u128. // zero gets skipped by the loop @@ -128,7 +196,7 @@ where /// The same as `fuzz`, except `f` has two inputs. pub fn fuzz_2(n: u32, f: F) where - ::UnsignedInt: Int, + ::Unsigned: Int, { // Check cases where the first and second inputs are zero. Both call `f` 210 times for `u128`. edge_cases!(I, case, { diff --git a/library/compiler-builtins/compiler-builtins/src/float/add.rs b/library/compiler-builtins/compiler-builtins/src/float/add.rs index 0426c9cc44fbb..43e3ae931ffa4 100644 --- a/library/compiler-builtins/compiler-builtins/src/float/add.rs +++ b/library/compiler-builtins/compiler-builtins/src/float/add.rs @@ -1,5 +1,5 @@ use crate::float::Float; -use crate::int::{CastInto, Int, MinInt}; +use crate::int::{CastFrom, CastInto, Int, MinInt}; /// Returns `a + b` fn add(a: F, b: F) -> F @@ -12,7 +12,7 @@ where let one = F::Int::ONE; let zero = F::Int::ZERO; - let bits = F::BITS.cast(); + let bits: F::Int = F::BITS.cast(); let significand_bits = F::SIG_BITS; let max_exponent = F::EXP_SAT; @@ -115,9 +115,10 @@ where let align = a_exponent.wrapping_sub(b_exponent).cast(); if align != MinInt::ZERO { if align < bits { - let sticky = - F::Int::from_bool(b_significand << bits.wrapping_sub(align).cast() != MinInt::ZERO); - b_significand = (b_significand >> align.cast()) | sticky; + let sticky = F::Int::from_bool( + b_significand << u32::cast_from(bits.wrapping_sub(align)) != MinInt::ZERO, + ); + b_significand = (b_significand >> u32::cast_from(align)) | sticky; } else { b_significand = one; // sticky; b is known to be non-zero. } @@ -132,8 +133,8 @@ where // If partial cancellation occured, we need to left-shift the result // and adjust the exponent: if a_significand < implicit_bit << 3 { - let shift = - a_significand.leading_zeros() as i32 - (implicit_bit << 3).leading_zeros() as i32; + let shift = a_significand.leading_zeros() as i32 + - (implicit_bit << 3u32).leading_zeros() as i32; a_significand <<= shift; a_exponent -= shift; } @@ -159,9 +160,10 @@ where // Result is denormal before rounding; the exponent is zero and we // need to shift the significand. let shift = (1 - a_exponent).cast(); - let sticky = - F::Int::from_bool((a_significand << bits.wrapping_sub(shift).cast()) != MinInt::ZERO); - a_significand = (a_significand >> shift.cast()) | sticky; + let sticky = F::Int::from_bool( + (a_significand << u32::cast_from(bits.wrapping_sub(shift))) != MinInt::ZERO, + ); + a_significand = (a_significand >> u32::cast_from(shift)) | sticky; a_exponent = 0; } diff --git a/library/compiler-builtins/compiler-builtins/src/float/conv.rs b/library/compiler-builtins/compiler-builtins/src/float/conv.rs index f5427a11390fc..9d732f2cdbc9e 100644 --- a/library/compiler-builtins/compiler-builtins/src/float/conv.rs +++ b/library/compiler-builtins/compiler-builtins/src/float/conv.rs @@ -72,7 +72,7 @@ mod int_to_float { F: Float, I: Int, F::Int: CastFrom, - Conv: Fn(I::UnsignedInt) -> F::Int, + Conv: Fn(I::Unsigned) -> F::Int, { let sign_bit = F::Int::cast_from(i >> (I::BITS - 1)) << (F::BITS - 1); F::from_bits(conv(i.unsigned_abs()) | sign_bit) @@ -313,10 +313,10 @@ intrinsics! { fn float_to_unsigned_int(f: F) -> U where F: Float, - U: Int, + U: Int, F::Int: CastInto, F::Int: CastFrom, - F::Int: CastInto, + F::Int: CastInto, u32: CastFrom, { float_to_int_inner::(f.to_bits(), |i: U| i, || U::MAX) @@ -327,8 +327,8 @@ fn float_to_signed_int(f: F) -> I where F: Float, I: Int + Neg, - I::UnsignedInt: Int, - F::Int: CastInto, + I::Unsigned: Int, + F::Int: CastInto, F::Int: CastFrom, u32: CastFrom, { @@ -355,27 +355,27 @@ where I: Int, FnFoo: FnOnce(I) -> I, FnOob: FnOnce() -> I, - I::UnsignedInt: Int, - F::Int: CastInto, + I::Unsigned: Int, + F::Int: CastInto, F::Int: CastFrom, u32: CastFrom, { let int_max_exp = F::EXP_BIAS + I::MAX.ilog2() + 1; - let foobar = F::EXP_BIAS + I::UnsignedInt::BITS - 1; + let foobar = F::EXP_BIAS + I::Unsigned::BITS - 1; if fbits < F::ONE.to_bits() { // < 0 gets rounded to 0 I::ZERO } else if fbits < F::Int::cast_from(int_max_exp) << F::SIG_BITS { // >= 1, < integer max - let m_base = if I::UnsignedInt::BITS >= F::Int::BITS { - I::UnsignedInt::cast_from(fbits) << (I::BITS - F::SIG_BITS - 1) + let m_base = if I::Unsigned::BITS >= F::Int::BITS { + I::Unsigned::cast_from(fbits) << (I::BITS - F::SIG_BITS - 1) } else { - I::UnsignedInt::cast_from(fbits >> (F::SIG_BITS - I::BITS + 1)) + I::Unsigned::cast_from(fbits >> (F::SIG_BITS - I::BITS + 1)) }; // Set the implicit 1-bit. - let m: I::UnsignedInt = (I::UnsignedInt::ONE << (I::BITS - 1)) | m_base; + let m: I::Unsigned = (I::Unsigned::ONE << (I::BITS - 1)) | m_base; // Shift based on the exponent and bias. let s: u32 = (foobar) - u32::cast_from(fbits >> F::SIG_BITS); diff --git a/library/compiler-builtins/compiler-builtins/src/float/div.rs b/library/compiler-builtins/compiler-builtins/src/float/div.rs index 5df637c7e0f9b..3e4f0e20d1056 100644 --- a/library/compiler-builtins/compiler-builtins/src/float/div.rs +++ b/library/compiler-builtins/compiler-builtins/src/float/div.rs @@ -370,7 +370,7 @@ where let hi_corr: F::Int = corr_uq1 >> hw; // x_UQ0 * corr_UQ1 = (x_UQ0_hw * 2^HW) * (hi_corr * 2^HW + lo_corr) - corr_UQ1 - let mut x_uq0: F::Int = ((F::Int::from(x_uq0_hw) * hi_corr) << 1) + let mut x_uq0: F::Int = ((F::Int::from(x_uq0_hw) * hi_corr) << 1u32) .wrapping_add((F::Int::from(x_uq0_hw) * lo_corr) >> (hw - 1)) // 1 to account for the highest bit of corr_UQ1 can be 1 // 1 to account for possible carry diff --git a/library/compiler-builtins/compiler-builtins/src/float/mul.rs b/library/compiler-builtins/compiler-builtins/src/float/mul.rs index 7f1f19d9bd7fb..c811f140670e6 100644 --- a/library/compiler-builtins/compiler-builtins/src/float/mul.rs +++ b/library/compiler-builtins/compiler-builtins/src/float/mul.rs @@ -143,7 +143,7 @@ where // a zero of the appropriate sign. Mathematically there is no need to // handle this case separately, but we make it a special case to // simplify the shift logic. - let shift = one.wrapping_sub(product_exponent.cast()).cast(); + let shift: u32 = one.wrapping_sub(product_exponent.cast()).cast(); if shift >= bits { return F::from_bits(product_sign); } diff --git a/library/compiler-builtins/compiler-builtins/src/float/traits.rs b/library/compiler-builtins/compiler-builtins/src/float/traits.rs index 8ccaa7bcbd78a..a30d20900b1c4 100644 --- a/library/compiler-builtins/compiler-builtins/src/float/traits.rs +++ b/library/compiler-builtins/compiler-builtins/src/float/traits.rs @@ -20,10 +20,10 @@ pub trait Float: + ops::Rem { /// A uint of the same width as the float - type Int: Int; + type Int: Int; /// A int of the same width as the float - type SignedInt: Int + MinInt; + type SignedInt: Int + MinInt; /// An int capable of containing the exponent bits plus a sign bit. This is signed. type ExpInt: Int; diff --git a/library/compiler-builtins/compiler-builtins/src/int/addsub.rs b/library/compiler-builtins/compiler-builtins/src/int/addsub.rs index 1f84e8eb1e117..b2b21fc2c4401 100644 --- a/library/compiler-builtins/compiler-builtins/src/int/addsub.rs +++ b/library/compiler-builtins/compiler-builtins/src/int/addsub.rs @@ -22,7 +22,7 @@ impl UAddSub for u128 {} trait AddSub: Int where - ::UnsignedInt: UAddSub, + ::Unsigned: UAddSub, { fn add(self, other: Self) -> Self { Self::from_unsigned(self.unsigned().uadd(other.unsigned())) @@ -37,7 +37,7 @@ impl AddSub for i128 {} trait Addo: AddSub where - ::UnsignedInt: UAddSub, + ::Unsigned: UAddSub, { fn addo(self, other: Self) -> (Self, bool) { let sum = AddSub::add(self, other); @@ -50,7 +50,7 @@ impl Addo for u128 {} trait Subo: AddSub where - ::UnsignedInt: UAddSub, + ::Unsigned: UAddSub, { fn subo(self, other: Self) -> (Self, bool) { let sum = AddSub::sub(self, other); diff --git a/library/compiler-builtins/compiler-builtins/src/int/big.rs b/library/compiler-builtins/compiler-builtins/src/int/big.rs index 1402efb8ed419..8e06009090c09 100644 --- a/library/compiler-builtins/compiler-builtins/src/int/big.rs +++ b/library/compiler-builtins/compiler-builtins/src/int/big.rs @@ -45,7 +45,7 @@ impl i256 { impl MinInt for u256 { type OtherSign = i256; - type UnsignedInt = u256; + type Unsigned = u256; const SIGNED: bool = false; const BITS: u32 = 256; @@ -58,7 +58,7 @@ impl MinInt for u256 { impl MinInt for i256 { type OtherSign = u256; - type UnsignedInt = u256; + type Unsigned = u256; const SIGNED: bool = false; const BITS: u32 = 256; diff --git a/library/compiler-builtins/compiler-builtins/src/int/leading_zeros.rs b/library/compiler-builtins/compiler-builtins/src/int/leading_zeros.rs index 112f4d0361317..aa5cb39935ad8 100644 --- a/library/compiler-builtins/compiler-builtins/src/int/leading_zeros.rs +++ b/library/compiler-builtins/compiler-builtins/src/int/leading_zeros.rs @@ -9,11 +9,14 @@ pub use implementation::{leading_zeros_default, leading_zeros_riscv}; pub(crate) use implementation::{leading_zeros_default, leading_zeros_riscv}; mod implementation { - use crate::int::{CastInto, Int}; + use crate::int::{CastFrom, Int}; /// Returns the number of leading binary zeros in `x`. #[allow(dead_code)] - pub fn leading_zeros_default>(x: T) -> usize { + pub fn leading_zeros_default(x: I) -> usize + where + usize: CastFrom, + { // The basic idea is to test if the higher bits of `x` are zero and bisect the number // of leading zeros. It is possible for all branches of the bisection to use the same // code path by conditionally shifting the higher parts down to let the next bisection @@ -23,44 +26,48 @@ mod implementation { // because it simplifies the final bisection step. let mut x = x; // the number of potential leading zeros - let mut z = T::BITS as usize; + let mut z = I::BITS as usize; // a temporary - let mut t: T; + let mut t: I; - const { assert!(T::BITS <= 64) }; - if T::BITS >= 64 { + const { assert!(I::BITS <= 64) }; + if I::BITS >= 64 { t = x >> 32; - if t != T::ZERO { + if t != I::ZERO { z -= 32; x = t; } } - if T::BITS >= 32 { + if I::BITS >= 32 { t = x >> 16; - if t != T::ZERO { + if t != I::ZERO { z -= 16; x = t; } } - const { assert!(T::BITS >= 16) }; + const { assert!(I::BITS >= 16) }; t = x >> 8; - if t != T::ZERO { + if t != I::ZERO { z -= 8; x = t; } t = x >> 4; - if t != T::ZERO { + if t != I::ZERO { z -= 4; x = t; } t = x >> 2; - if t != T::ZERO { + if t != I::ZERO { z -= 2; x = t; } // the last two bisections are combined into one conditional t = x >> 1; - if t != T::ZERO { z - 2 } else { z - x.cast() } + if t != I::ZERO { + z - 2 + } else { + z - usize::cast_from(x) + } // We could potentially save a few cycles by using the LUT trick from // "https://embeddedgurus.com/state-space/2014/09/ @@ -82,10 +89,13 @@ mod implementation { /// Returns the number of leading binary zeros in `x`. #[allow(dead_code)] - pub fn leading_zeros_riscv>(x: T) -> usize { + pub fn leading_zeros_riscv(x: I) -> usize + where + usize: CastFrom, + { let mut x = x; // the number of potential leading zeros - let mut z = T::BITS; + let mut z = I::BITS; // a temporary let mut t: u32; @@ -97,11 +107,11 @@ mod implementation { // right). If we try to save an instruction by using `x < imm` for each bisection, we // have to shift `x` left and compare with powers of two approaching `usize::MAX + 1`, // but the immediate will never fit into 12 bits and never save an instruction. - const { assert!(T::BITS <= 64) }; - if T::BITS >= 64 { + const { assert!(I::BITS <= 64) }; + if I::BITS >= 64 { // If the upper 32 bits of `x` are not all 0, `t` is set to `1 << 5`, otherwise // `t` is set to 0. - t = ((x >= (T::ONE << 32)) as u32) << 5; + t = ((x >= (I::ONE << 32)) as u32) << 5; // If `t` was set to `1 << 5`, then the upper 32 bits are shifted down for the // next step to process. x >>= t; @@ -109,27 +119,27 @@ mod implementation { // leading zeros z -= t; } - if T::BITS >= 32 { - t = ((x >= (T::ONE << 16)) as u32) << 4; + if I::BITS >= 32 { + t = ((x >= (I::ONE << 16)) as u32) << 4; x >>= t; z -= t; } - const { assert!(T::BITS >= 16) }; - t = ((x >= (T::ONE << 8)) as u32) << 3; + const { assert!(I::BITS >= 16) }; + t = ((x >= (I::ONE << 8)) as u32) << 3; x >>= t; z -= t; - t = ((x >= (T::ONE << 4)) as u32) << 2; + t = ((x >= (I::ONE << 4)) as u32) << 2; x >>= t; z -= t; - t = ((x >= (T::ONE << 2)) as u32) << 1; + t = ((x >= (I::ONE << 2)) as u32) << 1; x >>= t; z -= t; - t = (x >= (T::ONE << 1)) as u32; + t = (x >= (I::ONE << 1)) as u32; x >>= t; z -= t; // All bits except the LSB are guaranteed to be zero for this final bisection step. // If `x != 0` then `x == 1` and subtracts one potential zero from `z`. - z as usize - x.cast() + z as usize - usize::cast_from(x) } } diff --git a/library/compiler-builtins/compiler-builtins/src/int/trailing_zeros.rs b/library/compiler-builtins/compiler-builtins/src/int/trailing_zeros.rs index c45d6b1cfe8d8..8f63c22c8f734 100644 --- a/library/compiler-builtins/compiler-builtins/src/int/trailing_zeros.rs +++ b/library/compiler-builtins/compiler-builtins/src/int/trailing_zeros.rs @@ -4,33 +4,38 @@ pub use implementation::trailing_zeros; pub(crate) use implementation::trailing_zeros; mod implementation { - use crate::int::{CastInto, Int}; + use crate::int::{CastFrom, Int}; /// Returns number of trailing binary zeros in `x`. #[allow(dead_code)] - pub fn trailing_zeros + CastInto + CastInto>(x: T) -> usize { + pub fn trailing_zeros(x: I) -> usize + where + u32: CastFrom, + u16: CastFrom, + u8: CastFrom, + { let mut x = x; let mut r: u32 = 0; let mut t: u32; - const { assert!(T::BITS <= 64) }; - if T::BITS >= 64 { - r += ((CastInto::::cast(x) == 0) as u32) << 5; // if (x has no 32 small bits) t = 32 else 0 + const { assert!(I::BITS <= 64) }; + if I::BITS >= 64 { + r += ((u32::cast_from(x) == 0) as u32) << 5; // if (x has no 32 small bits) t = 32 else 0 x >>= r; // remove 32 zero bits } - if T::BITS >= 32 { - t = ((CastInto::::cast(x) == 0) as u32) << 4; // if (x has no 16 small bits) t = 16 else 0 + if I::BITS >= 32 { + t = ((u16::cast_from(x) == 0) as u32) << 4; // if (x has no 16 small bits) t = 16 else 0 r += t; x >>= t; // x = [0 - 0xFFFF] + higher garbage bits } - const { assert!(T::BITS >= 16) }; - t = ((CastInto::::cast(x) == 0) as u32) << 3; + const { assert!(I::BITS >= 16) }; + t = ((u8::cast_from(x) == 0) as u32) << 3; x >>= t; // x = [0 - 0xFF] + higher garbage bits r += t; - let mut x: u8 = x.cast(); + let mut x: u8 = x.cast_lossy(); t = (((x & 0x0F) == 0) as u32) << 2; x >>= t; // x = [0 - 0xF] + higher garbage bits diff --git a/library/compiler-builtins/compiler-builtins/src/int/traits.rs b/library/compiler-builtins/compiler-builtins/src/int/traits.rs index 152cb2eee2ee6..b474df366eb56 100644 --- a/library/compiler-builtins/compiler-builtins/src/int/traits.rs +++ b/library/compiler-builtins/compiler-builtins/src/int/traits.rs @@ -1,275 +1,4 @@ -use core::ops; - -/// Minimal integer implementations needed on all integer types, including wide integers. -#[allow(dead_code)] -pub trait MinInt: - Copy - + core::fmt::Debug - + ops::BitOr - + ops::Not - + ops::Shl -{ - /// Type with the same width but other signedness - type OtherSign: MinInt; - /// Unsigned version of Self - type UnsignedInt: MinInt; - - /// If `Self` is a signed integer - const SIGNED: bool; - - /// The bitwidth of the int type - const BITS: u32; - - const ZERO: Self; - const ONE: Self; - const MIN: Self; - const MAX: Self; -} - -/// Trait for some basic operations on integers -#[allow(dead_code)] -pub trait Int: - MinInt - + PartialEq - + PartialOrd - + ops::AddAssign - + ops::SubAssign - + ops::BitAndAssign - + ops::BitOrAssign - + ops::BitXorAssign - + ops::ShlAssign - + ops::ShrAssign - + ops::Add - + ops::Sub - + ops::Mul - + ops::Div - + ops::Shr - + ops::BitXor - + ops::BitAnd -{ - /// LUT used for maximizing the space covered and minimizing the computational cost of fuzzing - /// in `builtins-test`. For example, Self = u128 produces [0,1,2,7,8,15,16,31,32,63,64,95,96, - /// 111,112,119,120,125,126,127]. - const FUZZ_LENGTHS: [u8; 20] = make_fuzz_lengths(::BITS); - - /// The number of entries of `FUZZ_LENGTHS` actually used. The maximum is 20 for u128. - const FUZZ_NUM: usize = { - let log2 = (::BITS - 1).count_ones() as usize; - if log2 == 3 { - // case for u8 - 6 - } else { - // 3 entries on each extreme, 2 in the middle, and 4 for each scale of intermediate - // boundaries. - 8 + (4 * (log2 - 4)) - } - }; - - fn unsigned(self) -> Self::UnsignedInt; - fn from_unsigned(unsigned: Self::UnsignedInt) -> Self; - fn unsigned_abs(self) -> Self::UnsignedInt; - - fn from_bool(b: bool) -> Self; - - /// Prevents the need for excessive conversions between signed and unsigned - fn logical_shr(self, other: u32) -> Self; - - /// Absolute difference between two integers. - fn abs_diff(self, other: Self) -> Self::UnsignedInt; - - // copied from primitive integers, but put in a trait - fn is_zero(self) -> bool; - fn wrapping_neg(self) -> Self; - fn wrapping_add(self, other: Self) -> Self; - fn wrapping_mul(self, other: Self) -> Self; - fn wrapping_sub(self, other: Self) -> Self; - fn wrapping_shl(self, other: u32) -> Self; - fn wrapping_shr(self, other: u32) -> Self; - fn rotate_left(self, other: u32) -> Self; - fn overflowing_add(self, other: Self) -> (Self, bool); - fn leading_zeros(self) -> u32; - fn ilog2(self) -> u32; -} - -pub(crate) const fn make_fuzz_lengths(bits: u32) -> [u8; 20] { - let mut v = [0u8; 20]; - v[0] = 0; - v[1] = 1; - v[2] = 2; // important for parity and the iX::MIN case when reversed - let mut i = 3; - - // No need for any more until the byte boundary, because there should be no algorithms - // that are sensitive to anything not next to byte boundaries after 2. We also scale - // in powers of two, which is important to prevent u128 corner tests from getting too - // big. - let mut l = 8; - loop { - if l >= ((bits / 2) as u8) { - break; - } - // get both sides of the byte boundary - v[i] = l - 1; - i += 1; - v[i] = l; - i += 1; - l *= 2; - } - - if bits != 8 { - // add the lower side of the middle boundary - v[i] = ((bits / 2) - 1) as u8; - i += 1; - } - - // We do not want to jump directly from the Self::BITS/2 boundary to the Self::BITS - // boundary because of algorithms that split the high part up. We reverse the scaling - // as we go to Self::BITS. - let mid = i; - let mut j = 1; - loop { - v[i] = (bits as u8) - (v[mid - j]) - 1; - if j == mid { - break; - } - i += 1; - j += 1; - } - v -} - -macro_rules! int_impl_common { - ($ty:ty) => { - fn from_bool(b: bool) -> Self { - b as $ty - } - - fn logical_shr(self, other: u32) -> Self { - Self::from_unsigned(self.unsigned().wrapping_shr(other)) - } - - fn is_zero(self) -> bool { - self == Self::ZERO - } - - fn wrapping_neg(self) -> Self { - ::wrapping_neg(self) - } - - fn wrapping_add(self, other: Self) -> Self { - ::wrapping_add(self, other) - } - - fn wrapping_mul(self, other: Self) -> Self { - ::wrapping_mul(self, other) - } - fn wrapping_sub(self, other: Self) -> Self { - ::wrapping_sub(self, other) - } - - fn wrapping_shl(self, other: u32) -> Self { - ::wrapping_shl(self, other) - } - - fn wrapping_shr(self, other: u32) -> Self { - ::wrapping_shr(self, other) - } - - fn rotate_left(self, other: u32) -> Self { - ::rotate_left(self, other) - } - - fn overflowing_add(self, other: Self) -> (Self, bool) { - ::overflowing_add(self, other) - } - - fn leading_zeros(self) -> u32 { - ::leading_zeros(self) - } - - fn ilog2(self) -> u32 { - ::ilog2(self) - } - }; -} - -macro_rules! int_impl { - ($ity:ty, $uty:ty) => { - impl MinInt for $uty { - type OtherSign = $ity; - type UnsignedInt = $uty; - - const BITS: u32 = ::ZERO.count_zeros(); - const SIGNED: bool = Self::MIN != Self::ZERO; - - const ZERO: Self = 0; - const ONE: Self = 1; - const MIN: Self = ::MIN; - const MAX: Self = ::MAX; - } - - impl Int for $uty { - fn unsigned(self) -> $uty { - self - } - - // It makes writing macros easier if this is implemented for both signed and unsigned - #[allow(clippy::wrong_self_convention)] - fn from_unsigned(me: $uty) -> Self { - me - } - - fn unsigned_abs(self) -> Self { - self - } - - fn abs_diff(self, other: Self) -> Self { - self.abs_diff(other) - } - - int_impl_common!($uty); - } - - impl MinInt for $ity { - type OtherSign = $uty; - type UnsignedInt = $uty; - - const BITS: u32 = ::ZERO.count_zeros(); - const SIGNED: bool = Self::MIN != Self::ZERO; - - const ZERO: Self = 0; - const ONE: Self = 1; - const MIN: Self = ::MIN; - const MAX: Self = ::MAX; - } - - impl Int for $ity { - fn unsigned(self) -> $uty { - self as $uty - } - - fn from_unsigned(me: $uty) -> Self { - me as $ity - } - - fn unsigned_abs(self) -> Self::UnsignedInt { - self.unsigned_abs() - } - - fn abs_diff(self, other: Self) -> $uty { - self.abs_diff(other) - } - - int_impl_common!($ity); - } - }; -} - -int_impl!(isize, usize); -int_impl!(i8, u8); -int_impl!(i16, u16); -int_impl!(i32, u32); -int_impl!(i64, u64); -int_impl!(i128, u128); +pub use crate::support::{Int, MinInt}; /// Trait for integers twice the bit width of another integer. This is implemented for all /// primitives except for `u8`, because there is not a smaller primitive. diff --git a/library/compiler-builtins/libm/src/math/support/int_traits.rs b/library/compiler-builtins/libm/src/math/support/int_traits.rs index 3ec1faba170cb..fa9e06066d744 100644 --- a/library/compiler-builtins/libm/src/math/support/int_traits.rs +++ b/library/compiler-builtins/libm/src/math/support/int_traits.rs @@ -78,6 +78,7 @@ pub trait Int: fn unsigned(self) -> Self::Unsigned; fn from_unsigned(unsigned: Self::Unsigned) -> Self; fn abs(self) -> Self; + fn unsigned_abs(self) -> Self::Unsigned; fn from_bool(b: bool) -> Self; @@ -203,6 +204,10 @@ macro_rules! int_impl { unimplemented!() } + fn unsigned_abs(self) -> Self { + unimplemented!() + } + // It makes writing macros easier if this is implemented for both signed and unsigned #[allow(clippy::wrong_self_convention)] fn from_unsigned(me: $uty) -> Self { @@ -242,6 +247,10 @@ macro_rules! int_impl { self.abs() } + fn unsigned_abs(self) -> Self::Unsigned { + self.unsigned_abs() + } + fn from_unsigned(me: $uty) -> Self { me as $ity } From 877feef541542972f1bfeefe14624a17f23cef8d Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Thu, 29 May 2025 03:51:43 +0000 Subject: [PATCH 08/28] Reuse `libm`'s `Caat` and `CastFrom` in `compiler-builtins` --- .../compiler-builtins/src/float/add.rs | 2 +- .../compiler-builtins/src/float/conv.rs | 6 +-- .../compiler-builtins/src/float/div.rs | 2 +- .../compiler-builtins/src/float/mul.rs | 2 +- .../compiler-builtins/src/float/trunc.rs | 2 +- .../src/int/trailing_zeros.rs | 6 +-- .../compiler-builtins/src/int/traits.rs | 43 +------------------ .../libm/src/math/support/int_traits.rs | 5 +++ 8 files changed, 16 insertions(+), 52 deletions(-) diff --git a/library/compiler-builtins/compiler-builtins/src/float/add.rs b/library/compiler-builtins/compiler-builtins/src/float/add.rs index 43e3ae931ffa4..0cc362f705b11 100644 --- a/library/compiler-builtins/compiler-builtins/src/float/add.rs +++ b/library/compiler-builtins/compiler-builtins/src/float/add.rs @@ -168,7 +168,7 @@ where } // Low three bits are round, guard, and sticky. - let a_significand_i32: i32 = a_significand.cast(); + let a_significand_i32: i32 = a_significand.cast_lossy(); let round_guard_sticky: i32 = a_significand_i32 & 0x7; // Shift the significand into place, and mask off the implicit bit. diff --git a/library/compiler-builtins/compiler-builtins/src/float/conv.rs b/library/compiler-builtins/compiler-builtins/src/float/conv.rs index 9d732f2cdbc9e..75ea7ce02424a 100644 --- a/library/compiler-builtins/compiler-builtins/src/float/conv.rs +++ b/library/compiler-builtins/compiler-builtins/src/float/conv.rs @@ -74,7 +74,7 @@ mod int_to_float { F::Int: CastFrom, Conv: Fn(I::Unsigned) -> F::Int, { - let sign_bit = F::Int::cast_from(i >> (I::BITS - 1)) << (F::BITS - 1); + let sign_bit = F::Int::cast_from_lossy(i >> (I::BITS - 1)) << (F::BITS - 1); F::from_bits(conv(i.unsigned_abs()) | sign_bit) } @@ -166,7 +166,7 @@ mod int_to_float { // Within the upper `F::BITS`, everything except for the signifcand // gets truncated - let d1: u32 = (i_m >> (u128::BITS - f32::BITS - f32::SIG_BITS - 1)).cast(); + let d1: u32 = (i_m >> (u128::BITS - f32::BITS - f32::SIG_BITS - 1)).cast_lossy(); // The entire rest of `i_m` gets truncated. Zero the upper `F::BITS` then just // check if it is nonzero. @@ -371,7 +371,7 @@ where let m_base = if I::Unsigned::BITS >= F::Int::BITS { I::Unsigned::cast_from(fbits) << (I::BITS - F::SIG_BITS - 1) } else { - I::Unsigned::cast_from(fbits >> (F::SIG_BITS - I::BITS + 1)) + I::Unsigned::cast_from_lossy(fbits >> (F::SIG_BITS - I::BITS + 1)) }; // Set the implicit 1-bit. diff --git a/library/compiler-builtins/compiler-builtins/src/float/div.rs b/library/compiler-builtins/compiler-builtins/src/float/div.rs index 3e4f0e20d1056..fc1fc085105a7 100644 --- a/library/compiler-builtins/compiler-builtins/src/float/div.rs +++ b/library/compiler-builtins/compiler-builtins/src/float/div.rs @@ -482,7 +482,7 @@ where let ret = quotient.wrapping_shr(u32::cast_from(res_exponent.wrapping_neg()) + 1); residual_lo = a_significand - .wrapping_shl(significand_bits.wrapping_add(CastInto::::cast(res_exponent))) + .wrapping_shl(significand_bits.wrapping_add(CastInto::::cast_lossy(res_exponent))) .wrapping_sub(ret.wrapping_mul(b_significand) << 1); ret }; diff --git a/library/compiler-builtins/compiler-builtins/src/float/mul.rs b/library/compiler-builtins/compiler-builtins/src/float/mul.rs index c811f140670e6..dbed3095cda5d 100644 --- a/library/compiler-builtins/compiler-builtins/src/float/mul.rs +++ b/library/compiler-builtins/compiler-builtins/src/float/mul.rs @@ -143,7 +143,7 @@ where // a zero of the appropriate sign. Mathematically there is no need to // handle this case separately, but we make it a special case to // simplify the shift logic. - let shift: u32 = one.wrapping_sub(product_exponent.cast()).cast(); + let shift: u32 = one.wrapping_sub(product_exponent.cast_lossy()).cast(); if shift >= bits { return F::from_bits(product_sign); } diff --git a/library/compiler-builtins/compiler-builtins/src/float/trunc.rs b/library/compiler-builtins/compiler-builtins/src/float/trunc.rs index ca8a0f368b563..93db5d8bbdeb1 100644 --- a/library/compiler-builtins/compiler-builtins/src/float/trunc.rs +++ b/library/compiler-builtins/compiler-builtins/src/float/trunc.rs @@ -50,7 +50,7 @@ where // The exponent of a is within the range of normal numbers in the // destination format. We can convert by simply right-shifting with // rounding and adjusting the exponent. - abs_result = (a_abs >> sig_bits_delta).cast(); + abs_result = (a_abs >> sig_bits_delta).cast_lossy(); // Cast before shifting to prevent overflow. let bias_diff: R::Int = src_exp_bias.wrapping_sub(dst_exp_bias).cast(); let tmp = bias_diff << R::SIG_BITS; diff --git a/library/compiler-builtins/compiler-builtins/src/int/trailing_zeros.rs b/library/compiler-builtins/compiler-builtins/src/int/trailing_zeros.rs index 8f63c22c8f734..1b0ae5b73ad24 100644 --- a/library/compiler-builtins/compiler-builtins/src/int/trailing_zeros.rs +++ b/library/compiler-builtins/compiler-builtins/src/int/trailing_zeros.rs @@ -20,18 +20,18 @@ mod implementation { const { assert!(I::BITS <= 64) }; if I::BITS >= 64 { - r += ((u32::cast_from(x) == 0) as u32) << 5; // if (x has no 32 small bits) t = 32 else 0 + r += ((u32::cast_from_lossy(x) == 0) as u32) << 5; // if (x has no 32 small bits) t = 32 else 0 x >>= r; // remove 32 zero bits } if I::BITS >= 32 { - t = ((u16::cast_from(x) == 0) as u32) << 4; // if (x has no 16 small bits) t = 16 else 0 + t = ((u16::cast_from_lossy(x) == 0) as u32) << 4; // if (x has no 16 small bits) t = 16 else 0 r += t; x >>= t; // x = [0 - 0xFFFF] + higher garbage bits } const { assert!(I::BITS >= 16) }; - t = ((u8::cast_from(x) == 0) as u32) << 3; + t = ((u8::cast_from_lossy(x) == 0) as u32) << 3; x >>= t; // x = [0 - 0xFF] + higher garbage bits r += t; diff --git a/library/compiler-builtins/compiler-builtins/src/int/traits.rs b/library/compiler-builtins/compiler-builtins/src/int/traits.rs index b474df366eb56..25b9718ad53fb 100644 --- a/library/compiler-builtins/compiler-builtins/src/int/traits.rs +++ b/library/compiler-builtins/compiler-builtins/src/int/traits.rs @@ -1,4 +1,4 @@ -pub use crate::support::{Int, MinInt}; +pub use crate::support::{CastFrom, CastInto, Int, MinInt}; /// Trait for integers twice the bit width of another integer. This is implemented for all /// primitives except for `u8`, because there is not a smaller primitive. @@ -97,44 +97,3 @@ impl_h_int!( i32 u32 i64, i64 u64 i128 ); - -/// Trait to express (possibly lossy) casting of integers -pub trait CastInto: Copy { - fn cast(self) -> T; -} - -pub trait CastFrom: Copy { - fn cast_from(value: T) -> Self; -} - -impl + Copy> CastFrom for T { - fn cast_from(value: U) -> Self { - value.cast() - } -} - -macro_rules! cast_into { - ($ty:ty) => { - cast_into!($ty; usize, isize, u8, i8, u16, i16, u32, i32, u64, i64, u128, i128); - }; - ($ty:ty; $($into:ty),*) => {$( - impl CastInto<$into> for $ty { - fn cast(self) -> $into { - self as $into - } - } - )*}; -} - -cast_into!(usize); -cast_into!(isize); -cast_into!(u8); -cast_into!(i8); -cast_into!(u16); -cast_into!(i16); -cast_into!(u32); -cast_into!(i32); -cast_into!(u64); -cast_into!(i64); -cast_into!(u128); -cast_into!(i128); diff --git a/library/compiler-builtins/libm/src/math/support/int_traits.rs b/library/compiler-builtins/libm/src/math/support/int_traits.rs index fa9e06066d744..716af748a9fb8 100644 --- a/library/compiler-builtins/libm/src/math/support/int_traits.rs +++ b/library/compiler-builtins/libm/src/math/support/int_traits.rs @@ -374,14 +374,19 @@ impl_h_int!( /// Trait to express (possibly lossy) casting of integers pub trait CastInto: Copy { /// By default, casts should be exact. + #[track_caller] fn cast(self) -> T; /// Call for casts that are expected to truncate. + /// + /// In practice, this is exactly the same as `cast`; the main difference is to document intent + /// in code. `cast` may panic in debug mode. fn cast_lossy(self) -> T; } pub trait CastFrom: Copy { /// By default, casts should be exact. + #[track_caller] fn cast_from(value: T) -> Self; /// Call for casts that are expected to truncate. From 97c0bebdf7094434c0fb2f941d7136c9c3ade986 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Wed, 28 May 2025 18:15:41 +0000 Subject: [PATCH 09/28] Remove unneeded C symbols These are now provided by `compiler-builtins`, so there is no need to also build the C versions. This was detected by checking for duplicate symbols and not excluding weak symbols (like CI currently does). --- library/compiler-builtins/compiler-builtins/build.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/library/compiler-builtins/compiler-builtins/build.rs b/library/compiler-builtins/compiler-builtins/build.rs index 90d98ec7ce940..d37fdc5df507a 100644 --- a/library/compiler-builtins/compiler-builtins/build.rs +++ b/library/compiler-builtins/compiler-builtins/build.rs @@ -555,7 +555,6 @@ mod c { if (target.arch == "aarch64" || target.arch == "arm64ec") && consider_float_intrinsics { sources.extend(&[ - ("__comparetf2", "comparetf2.c"), ("__fe_getround", "fp_mode.c"), ("__fe_raise_inexact", "fp_mode.c"), ]); @@ -570,11 +569,11 @@ mod c { } if target.arch == "mips64" { - sources.extend(&[("__netf2", "comparetf2.c"), ("__fe_getround", "fp_mode.c")]); + sources.extend(&[("__fe_getround", "fp_mode.c")]); } if target.arch == "loongarch64" { - sources.extend(&[("__netf2", "comparetf2.c"), ("__fe_getround", "fp_mode.c")]); + sources.extend(&[("__fe_getround", "fp_mode.c")]); } // Remove the assembly implementations that won't compile for the target From a63f4826cf2ccceeedff5fb19ba0cbffc1ef5d65 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Sat, 19 Apr 2025 07:38:43 +0000 Subject: [PATCH 10/28] Replace the `nm` symbol check with a Rust implementation This should be less error-prone and adaptable than the `nm` version, and have better cross-platform support without needing LLVM `nm` installed. --- library/compiler-builtins/Cargo.toml | 1 + library/compiler-builtins/ci/run.sh | 125 ++-------- .../crates/symbol-check/Cargo.toml | 13 + .../crates/symbol-check/src/main.rs | 231 ++++++++++++++++++ 4 files changed, 262 insertions(+), 108 deletions(-) create mode 100644 library/compiler-builtins/crates/symbol-check/Cargo.toml create mode 100644 library/compiler-builtins/crates/symbol-check/src/main.rs diff --git a/library/compiler-builtins/Cargo.toml b/library/compiler-builtins/Cargo.toml index b39ec8a25dac1..bc6b4bd29795e 100644 --- a/library/compiler-builtins/Cargo.toml +++ b/library/compiler-builtins/Cargo.toml @@ -6,6 +6,7 @@ members = [ "crates/libm-macros", "crates/musl-math-sys", "crates/panic-handler", + "crates/symbol-check", "crates/util", "libm", "libm-test", diff --git a/library/compiler-builtins/ci/run.sh b/library/compiler-builtins/ci/run.sh index 68d13c130bcce..cf3f7dfda1da2 100755 --- a/library/compiler-builtins/ci/run.sh +++ b/library/compiler-builtins/ci/run.sh @@ -47,87 +47,25 @@ else fi fi - -declare -a rlib_paths - -# Set the `rlib_paths` global array to a list of all compiler-builtins rlibs -update_rlib_paths() { - if [ -d /builtins-target ]; then - rlib_paths=( /builtins-target/"${target}"/debug/deps/libcompiler_builtins-*.rlib ) - else - rlib_paths=( target/"${target}"/debug/deps/libcompiler_builtins-*.rlib ) - fi -} - -# Remove any existing artifacts from previous tests that don't set #![compiler_builtins] -update_rlib_paths -rm -f "${rlib_paths[@]}" - -cargo build -p compiler_builtins --target "$target" -cargo build -p compiler_builtins --target "$target" --release -cargo build -p compiler_builtins --target "$target" --features c -cargo build -p compiler_builtins --target "$target" --features c --release -cargo build -p compiler_builtins --target "$target" --features no-asm -cargo build -p compiler_builtins --target "$target" --features no-asm --release -cargo build -p compiler_builtins --target "$target" --features no-f16-f128 -cargo build -p compiler_builtins --target "$target" --features no-f16-f128 --release - -PREFIX=${target//unknown-/}- -case "$target" in - armv7-*) - PREFIX=arm-linux-gnueabihf- - ;; - thumb*) - PREFIX=arm-none-eabi- - ;; - *86*-*) - PREFIX= - ;; -esac - -NM=$(find "$(rustc --print sysroot)" \( -name llvm-nm -o -name llvm-nm.exe \) ) -if [ "$NM" = "" ]; then - NM="${PREFIX}nm" -fi - -# i686-pc-windows-gnu tools have a dependency on some DLLs, so run it with -# rustup run to ensure that those are in PATH. -TOOLCHAIN="$(rustup show active-toolchain | sed 's/ (default)//')" -if [[ "$TOOLCHAIN" == *i686-pc-windows-gnu ]]; then - NM="rustup run $TOOLCHAIN $NM" -fi - -# Look out for duplicated symbols when we include the compiler-rt (C) implementation -update_rlib_paths -for rlib in "${rlib_paths[@]}"; do - set +x - echo "================================================================" - echo "checking $rlib for duplicate symbols" - echo "================================================================" - set -x - - duplicates_found=0 - - # NOTE On i586, It's normal that the get_pc_thunk symbol appears several - # times so ignore it - $NM -g --defined-only "$rlib" 2>&1 | - sort | - uniq -d | - grep -v __x86.get_pc_thunk --quiet | - grep 'T __' && duplicates_found=1 - - if [ "$duplicates_found" != 0 ]; then - echo "error: found duplicate symbols" - exit 1 - else - echo "success; no duplicate symbols found" - fi -done - -rm -f "${rlib_paths[@]}" +# Ensure there are no duplicate symbols or references to `core` when +# `compiler-builtins` is built with various features. Symcheck invokes Cargo to +# build with the arguments we provide it, then validates the built artifacts. +symcheck=(cargo run -p symbol-check --release) +[[ "$target" = "wasm"* ]] && symcheck+=(--features wasm) +symcheck+=(-- build-and-check) + +"${symcheck[@]}" -p compiler_builtins --target "$target" +"${symcheck[@]}" -p compiler_builtins --target "$target" --release +"${symcheck[@]}" -p compiler_builtins --target "$target" --features c +"${symcheck[@]}" -p compiler_builtins --target "$target" --features c --release +"${symcheck[@]}" -p compiler_builtins --target "$target" --features no-asm +"${symcheck[@]}" -p compiler_builtins --target "$target" --features no-asm --release +"${symcheck[@]}" -p compiler_builtins --target "$target" --features no-f16-f128 +"${symcheck[@]}" -p compiler_builtins --target "$target" --features no-f16-f128 --release build_intrinsics_test() { - cargo build \ + # symcheck also checks the results of builtins-test-intrinsics + "${symcheck[@]}" \ --target "$target" --verbose \ --manifest-path builtins-test-intrinsics/Cargo.toml "$@" } @@ -143,35 +81,6 @@ build_intrinsics_test --features c --release CARGO_PROFILE_DEV_LTO=true build_intrinsics_test CARGO_PROFILE_RELEASE_LTO=true build_intrinsics_test --release -# Ensure no references to any symbols from core -update_rlib_paths -for rlib in "${rlib_paths[@]}"; do - set +x - echo "================================================================" - echo "checking $rlib for references to core" - echo "================================================================" - set -x - - tmpdir="${CARGO_TARGET_DIR:-target}/tmp" - test -d "$tmpdir" || mkdir "$tmpdir" - defined="$tmpdir/defined_symbols.txt" - undefined="$tmpdir/defined_symbols.txt" - - $NM --quiet -U "$rlib" | grep 'T _ZN4core' | awk '{print $3}' | sort | uniq > "$defined" - $NM --quiet -u "$rlib" | grep 'U _ZN4core' | awk '{print $2}' | sort | uniq > "$undefined" - grep_has_results=0 - grep -v -F -x -f "$defined" "$undefined" && grep_has_results=1 - - if [ "$target" = "powerpc64-unknown-linux-gnu" ]; then - echo "FIXME: powerpc64 fails these tests" - elif [ "$grep_has_results" != 0 ]; then - echo "error: found unexpected references to core" - exit 1 - else - echo "success; no references to core found" - fi -done - # Test libm # Make sure a simple build works diff --git a/library/compiler-builtins/crates/symbol-check/Cargo.toml b/library/compiler-builtins/crates/symbol-check/Cargo.toml new file mode 100644 index 0000000000000..30969ee406ab4 --- /dev/null +++ b/library/compiler-builtins/crates/symbol-check/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "symbol-check" +version = "0.1.0" +edition = "2024" +publish = false + +[dependencies] +# FIXME: used as a git dependency since the latest release does not support wasm +object = { git = "https://github.com/gimli-rs/object.git", rev = "013fac75da56a684377af4151b8164b78c1790e0" } +serde_json = "1.0.140" + +[features] +wasm = ["object/wasm"] diff --git a/library/compiler-builtins/crates/symbol-check/src/main.rs b/library/compiler-builtins/crates/symbol-check/src/main.rs new file mode 100644 index 0000000000000..104505438cc07 --- /dev/null +++ b/library/compiler-builtins/crates/symbol-check/src/main.rs @@ -0,0 +1,231 @@ +//! Tool used by CI to inspect compiler-builtins archives and help ensure we won't run into any +//! linking errors. + +use std::collections::{BTreeMap, BTreeSet}; +use std::fs; +use std::io::{BufRead, BufReader}; +use std::path::{Path, PathBuf}; +use std::process::{Command, Stdio}; + +use object::read::archive::{ArchiveFile, ArchiveMember}; +use object::{Object, ObjectSymbol, Symbol, SymbolKind, SymbolScope, SymbolSection}; +use serde_json::Value; + +const CHECK_LIBRARIES: &[&str] = &["compiler_builtins", "builtins_test_intrinsics"]; +const CHECK_EXTENSIONS: &[Option<&str>] = &[Some("rlib"), Some("a"), Some("exe"), None]; + +const USAGE: &str = "Usage: + + symbol-check build-and-check CARGO_ARGS ... + +Cargo will get invoked with `CARGO_ARGS` and all output +`compiler_builtins*.rlib` files will be checked. +"; + +fn main() { + // Create a `&str` vec so we can match on it. + let args = std::env::args().collect::>(); + let args_ref = args.iter().map(String::as_str).collect::>(); + + match &args_ref[1..] { + ["build-and-check", rest @ ..] if !rest.is_empty() => { + let paths = exec_cargo_with_args(rest); + for path in paths { + println!("Checking {}", path.display()); + verify_no_duplicates(&path); + verify_core_symbols(&path); + } + } + _ => { + println!("{USAGE}"); + std::process::exit(1); + } + } +} + +/// Run `cargo build` with the provided additional arguments, collecting the list of created +/// libraries. +fn exec_cargo_with_args(args: &[&str]) -> Vec { + let mut cmd = Command::new("cargo") + .arg("build") + .arg("--message-format=json") + .args(args) + .stdout(Stdio::piped()) + .spawn() + .expect("failed to launch Cargo"); + + let stdout = cmd.stdout.take().unwrap(); + let reader = BufReader::new(stdout); + let mut check_files = Vec::new(); + + for line in reader.lines() { + let line = line.expect("failed to read line"); + println!("{line}"); // tee to stdout + + // Select only steps that create files + let j: Value = serde_json::from_str(&line).expect("failed to deserialize"); + if j["reason"] != "compiler-artifact" { + continue; + } + + // Find rlibs in the created file list that match our expected library names and + // extensions. + for fpath in j["filenames"].as_array().expect("filenames not an array") { + let path = fpath.as_str().expect("file name not a string"); + let path = PathBuf::from(path); + + if CHECK_EXTENSIONS.contains(&path.extension().map(|ex| ex.to_str().unwrap())) { + let fname = path.file_name().unwrap().to_str().unwrap(); + + if CHECK_LIBRARIES.iter().any(|lib| fname.contains(lib)) { + check_files.push(path); + } + } + } + } + + cmd.wait().expect("failed to wait on Cargo"); + + assert!(!check_files.is_empty(), "no compiler_builtins rlibs found"); + println!("Collected the following rlibs to check: {check_files:#?}"); + + check_files +} + +/// Information collected from `object`, for convenience. +#[expect(unused)] // only for printing +#[derive(Clone, Debug)] +struct SymInfo { + name: String, + kind: SymbolKind, + scope: SymbolScope, + section: SymbolSection, + is_undefined: bool, + is_global: bool, + is_local: bool, + is_weak: bool, + is_common: bool, + address: u64, + object: String, +} + +impl SymInfo { + fn new(sym: &Symbol, member: &ArchiveMember) -> Self { + Self { + name: sym.name().expect("missing name").to_owned(), + kind: sym.kind(), + scope: sym.scope(), + section: sym.section(), + is_undefined: sym.is_undefined(), + is_global: sym.is_global(), + is_local: sym.is_local(), + is_weak: sym.is_weak(), + is_common: sym.is_common(), + address: sym.address(), + object: String::from_utf8_lossy(member.name()).into_owned(), + } + } +} + +/// Ensure that the same global symbol isn't defined in multiple object files within an archive. +/// +/// Note that this will also locate cases where a symbol is weakly defined in more than one place. +/// Technically there are no linker errors that will come from this, but it keeps our binary more +/// straightforward and saves some distribution size. +fn verify_no_duplicates(path: &Path) { + let mut syms = BTreeMap::::new(); + let mut dups = Vec::new(); + let mut found_any = false; + + for_each_symbol(path, |symbol, member| { + // Only check defined globals + if !symbol.is_global() || symbol.is_undefined() { + return; + } + + let sym = SymInfo::new(&symbol, member); + + // x86-32 includes multiple copies of thunk symbols + if sym.name.starts_with("__x86.get_pc_thunk") { + return; + } + + // Windows has symbols for literal numeric constants, string literals, and MinGW pseudo- + // relocations. These are allowed to have repeated definitions. + let win_allowed_dup_pfx = ["__real@", "__xmm@", "??_C@_", ".refptr"]; + if win_allowed_dup_pfx + .iter() + .any(|pfx| sym.name.starts_with(pfx)) + { + return; + } + + match syms.get(&sym.name) { + Some(existing) => { + dups.push(sym); + dups.push(existing.clone()); + } + None => { + syms.insert(sym.name.clone(), sym); + } + } + + found_any = true; + }); + + assert!(found_any, "no symbols found"); + + if !dups.is_empty() { + dups.sort_unstable_by(|a, b| a.name.cmp(&b.name)); + panic!("found duplicate symbols: {dups:#?}"); + } + + println!(" success: no duplicate symbols found"); +} + +/// Ensure that there are no references to symbols from `core` that aren't also (somehow) defined. +fn verify_core_symbols(path: &Path) { + let mut defined = BTreeSet::new(); + let mut undefined = Vec::new(); + let mut has_symbols = false; + + for_each_symbol(path, |symbol, member| { + has_symbols = true; + + // Find only symbols from `core` + if !symbol.name().unwrap().contains("_ZN4core") { + return; + } + + let sym = SymInfo::new(&symbol, member); + if sym.is_undefined { + undefined.push(sym); + } else { + defined.insert(sym.name); + } + }); + + assert!(has_symbols, "no symbols found"); + + // Discard any symbols that are defined somewhere in the archive + undefined.retain(|sym| !defined.contains(&sym.name)); + + if !undefined.is_empty() { + undefined.sort_unstable_by(|a, b| a.name.cmp(&b.name)); + panic!("found undefined symbols from core: {undefined:#?}"); + } + + println!(" success: no undefined references to core found"); +} + +/// For a given archive path, do something with each symbol. +fn for_each_symbol(path: &Path, mut f: impl FnMut(Symbol, &ArchiveMember)) { + let data = fs::read(path).expect("reading file failed"); + let archive = ArchiveFile::parse(data.as_slice()).expect("archive parse failed"); + for member in archive.members() { + let member = member.expect("failed to access member"); + let obj_data = member.data(&*data).expect("failed to access object"); + let obj = object::File::parse(obj_data).expect("failed to parse object"); + obj.symbols().for_each(|sym| f(sym, &member)); + } +} From 8db9bd6a59c8c59b8d04ee647f071e81f79f6225 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Wed, 28 May 2025 19:59:16 +0000 Subject: [PATCH 11/28] Remove the now-unneeded llvm-tools-preview Since a working `nm` is no longer needed as part of CI, the rustup component can be removed. --- library/compiler-builtins/.github/workflows/main.yaml | 1 - library/compiler-builtins/crates/symbol-check/src/main.rs | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/library/compiler-builtins/.github/workflows/main.yaml b/library/compiler-builtins/.github/workflows/main.yaml index d13dd6b0f6462..567ad1205a5d3 100644 --- a/library/compiler-builtins/.github/workflows/main.yaml +++ b/library/compiler-builtins/.github/workflows/main.yaml @@ -119,7 +119,6 @@ jobs: rustup update "$channel" --no-self-update rustup default "$channel" rustup target add "${{ matrix.target }}" - rustup component add llvm-tools-preview - uses: taiki-e/install-action@nextest - uses: Swatinem/rust-cache@v2 with: diff --git a/library/compiler-builtins/crates/symbol-check/src/main.rs b/library/compiler-builtins/crates/symbol-check/src/main.rs index 104505438cc07..4e6417fdf9525 100644 --- a/library/compiler-builtins/crates/symbol-check/src/main.rs +++ b/library/compiler-builtins/crates/symbol-check/src/main.rs @@ -84,7 +84,7 @@ fn exec_cargo_with_args(args: &[&str]) -> Vec { } } - cmd.wait().expect("failed to wait on Cargo"); + assert!(cmd.wait().expect("failed to wait on Cargo").success()); assert!(!check_files.is_empty(), "no compiler_builtins rlibs found"); println!("Collected the following rlibs to check: {check_files:#?}"); From 151b1cb04777e1e9630045d52f14bc0e7adf5787 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Thu, 29 May 2025 15:40:05 +0000 Subject: [PATCH 12/28] Change `compiler-builtins` to edition 2024 Do the same for `builtins-test-intrinsics`. Mostly this means updating `extern` to `unsafe extern`, and fixing a few new Clippy lints. --- .../builtins-test-intrinsics/Cargo.toml | 2 +- .../builtins-test-intrinsics/src/main.rs | 6 ++++-- .../builtins-test/tests/aeabi_memclr.rs | 3 ++- .../builtins-test/tests/aeabi_memcpy.rs | 3 ++- .../builtins-test/tests/aeabi_memset.rs | 3 ++- .../compiler-builtins/Cargo.toml | 2 +- .../compiler-builtins/src/arm.rs | 7 +++++-- .../src/int/specialized_div_rem/mod.rs | 16 ++++++++-------- .../compiler-builtins/src/macros.rs | 4 ++-- .../compiler-builtins/src/probestack.rs | 4 +++- 10 files changed, 30 insertions(+), 20 deletions(-) diff --git a/library/compiler-builtins/builtins-test-intrinsics/Cargo.toml b/library/compiler-builtins/builtins-test-intrinsics/Cargo.toml index 6e10628a41b22..704de20c5ad64 100644 --- a/library/compiler-builtins/builtins-test-intrinsics/Cargo.toml +++ b/library/compiler-builtins/builtins-test-intrinsics/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "builtins-test-intrinsics" version = "0.1.0" -edition = "2021" +edition = "2024" publish = false license = "MIT OR Apache-2.0" diff --git a/library/compiler-builtins/builtins-test-intrinsics/src/main.rs b/library/compiler-builtins/builtins-test-intrinsics/src/main.rs index 1fa7b00916f77..96fe4a7385113 100644 --- a/library/compiler-builtins/builtins-test-intrinsics/src/main.rs +++ b/library/compiler-builtins/builtins-test-intrinsics/src/main.rs @@ -15,9 +15,10 @@ extern crate panic_handler; +// SAFETY: no definitions, only used for linking #[cfg(all(not(thumb), not(windows), not(target_arch = "wasm32")))] #[link(name = "c")] -extern "C" {} +unsafe extern "C" {} // Every function in this module maps will be lowered to an intrinsic by LLVM, if the platform // doesn't have native support for the operation used in the function. ARM has a naming convention @@ -663,10 +664,11 @@ pub fn _start() -> ! { loop {} } +// SAFETY: no definitions, only used for linking #[cfg(windows)] #[link(name = "kernel32")] #[link(name = "msvcrt")] -extern "C" {} +unsafe extern "C" {} // ARM targets need these symbols #[unsafe(no_mangle)] diff --git a/library/compiler-builtins/builtins-test/tests/aeabi_memclr.rs b/library/compiler-builtins/builtins-test/tests/aeabi_memclr.rs index bfd15a391aab2..0761feaffd9e2 100644 --- a/library/compiler-builtins/builtins-test/tests/aeabi_memclr.rs +++ b/library/compiler-builtins/builtins-test/tests/aeabi_memclr.rs @@ -24,7 +24,8 @@ macro_rules! panic { }; } -extern "C" { +// SAFETY: defined in compiler-builtins +unsafe extern "aapcs" { fn __aeabi_memclr4(dest: *mut u8, n: usize); fn __aeabi_memset4(dest: *mut u8, n: usize, c: u32); } diff --git a/library/compiler-builtins/builtins-test/tests/aeabi_memcpy.rs b/library/compiler-builtins/builtins-test/tests/aeabi_memcpy.rs index c892c5aba0f74..e76e712a246f1 100644 --- a/library/compiler-builtins/builtins-test/tests/aeabi_memcpy.rs +++ b/library/compiler-builtins/builtins-test/tests/aeabi_memcpy.rs @@ -22,7 +22,8 @@ macro_rules! panic { }; } -extern "C" { +// SAFETY: defined in compiler-builtins +unsafe extern "aapcs" { fn __aeabi_memcpy(dest: *mut u8, src: *const u8, n: usize); fn __aeabi_memcpy4(dest: *mut u8, src: *const u8, n: usize); } diff --git a/library/compiler-builtins/builtins-test/tests/aeabi_memset.rs b/library/compiler-builtins/builtins-test/tests/aeabi_memset.rs index 34ab3acc78c16..8f9f80f969ccb 100644 --- a/library/compiler-builtins/builtins-test/tests/aeabi_memset.rs +++ b/library/compiler-builtins/builtins-test/tests/aeabi_memset.rs @@ -24,7 +24,8 @@ macro_rules! panic { }; } -extern "C" { +// SAFETY: defined in compiler-builtins +unsafe extern "aapcs" { fn __aeabi_memset4(dest: *mut u8, n: usize, c: u32); } diff --git a/library/compiler-builtins/compiler-builtins/Cargo.toml b/library/compiler-builtins/compiler-builtins/Cargo.toml index d65a22152ef94..93eb3e01b3b8a 100644 --- a/library/compiler-builtins/compiler-builtins/Cargo.toml +++ b/library/compiler-builtins/compiler-builtins/Cargo.toml @@ -7,7 +7,7 @@ readme = "README.md" repository = "https://github.com/rust-lang/compiler-builtins" homepage = "https://github.com/rust-lang/compiler-builtins" documentation = "https://docs.rs/compiler_builtins" -edition = "2021" +edition = "2024" description = "Compiler intrinsics used by the Rust compiler." links = "compiler-rt" diff --git a/library/compiler-builtins/compiler-builtins/src/arm.rs b/library/compiler-builtins/compiler-builtins/src/arm.rs index a9107e3cdfda4..a7d84e49b3493 100644 --- a/library/compiler-builtins/compiler-builtins/src/arm.rs +++ b/library/compiler-builtins/compiler-builtins/src/arm.rs @@ -1,13 +1,16 @@ #![cfg(not(feature = "no-asm"))] // Interfaces used by naked trampolines. -extern "C" { +// SAFETY: these are defined in compiler-builtins +unsafe extern "C" { fn __udivmodsi4(a: u32, b: u32, rem: *mut u32) -> u32; fn __udivmoddi4(a: u64, b: u64, rem: *mut u64) -> u64; fn __divmoddi4(a: i64, b: i64, rem: *mut i64) -> i64; } -extern "aapcs" { +// SAFETY: these are defined in compiler-builtins +// FIXME(extern_custom), this isn't always the correct ABI +unsafe extern "aapcs" { // AAPCS is not always the correct ABI for these intrinsics, but we only use this to // forward another `__aeabi_` call so it doesn't matter. fn __aeabi_idiv(a: i32, b: i32) -> i32; 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 43f466e75ba4a..7841e4f33cd66 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 @@ -125,10 +125,10 @@ impl_normalization_shift!( /// dependencies. #[inline] fn u64_by_u64_div_rem(duo: u64, div: u64) -> (u64, u64) { - if let Some(quo) = duo.checked_div(div) { - if let Some(rem) = duo.checked_rem(div) { - return (quo, rem); - } + if let Some(quo) = duo.checked_div(div) + && let Some(rem) = duo.checked_rem(div) + { + return (quo, rem); } zero_div_fn() } @@ -227,10 +227,10 @@ impl_asymmetric!( #[inline] #[allow(dead_code)] fn u32_by_u32_div_rem(duo: u32, div: u32) -> (u32, u32) { - if let Some(quo) = duo.checked_div(div) { - if let Some(rem) = duo.checked_rem(div) { - return (quo, rem); - } + if let Some(quo) = duo.checked_div(div) + && let Some(rem) = duo.checked_rem(div) + { + return (quo, rem); } zero_div_fn() } diff --git a/library/compiler-builtins/compiler-builtins/src/macros.rs b/library/compiler-builtins/compiler-builtins/src/macros.rs index 22e0dd27f2f5a..203cd0949ac52 100644 --- a/library/compiler-builtins/compiler-builtins/src/macros.rs +++ b/library/compiler-builtins/compiler-builtins/src/macros.rs @@ -132,7 +132,7 @@ macro_rules! intrinsics { ) => ( #[cfg($name = "optimized-c")] pub $(unsafe $($empty)? )? extern $abi fn $name( $($argname: $ty),* ) $(-> $ret)? { - extern $abi { + unsafe extern $abi { fn $name($($argname: $ty),*) $(-> $ret)?; } unsafe { @@ -435,7 +435,7 @@ macro_rules! intrinsics { pub mod $name { #[unsafe(naked)] $(#[$($attr)*])* - #[cfg_attr(not(feature = "mangled-names"), no_mangle)] + #[cfg_attr(not(feature = "mangled-names"), unsafe(no_mangle))] #[cfg_attr(not(any(all(windows, target_env = "gnu"), target_os = "cygwin")), linkage = "weak")] pub unsafe extern $abi fn $name( $($argname: $ty),* ) $(-> $ret)? { $($body)* diff --git a/library/compiler-builtins/compiler-builtins/src/probestack.rs b/library/compiler-builtins/compiler-builtins/src/probestack.rs index 5b6abd21a1db3..c9070cf55c64a 100644 --- a/library/compiler-builtins/compiler-builtins/src/probestack.rs +++ b/library/compiler-builtins/compiler-builtins/src/probestack.rs @@ -49,7 +49,9 @@ // We only define stack probing for these architectures today. #![cfg(any(target_arch = "x86_64", target_arch = "x86"))] -extern "C" { +// SAFETY: defined in this module. +// FIXME(extern_custom): the ABI is not correct. +unsafe extern "C" { pub fn __rust_probestack(); } From af16553893152ba74b0391b02c31f083c2f519b1 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Thu, 29 May 2025 16:07:54 +0000 Subject: [PATCH 13/28] symcheck: Print the command to make reproducing errors easier --- .../crates/symbol-check/src/main.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/library/compiler-builtins/crates/symbol-check/src/main.rs b/library/compiler-builtins/crates/symbol-check/src/main.rs index 4e6417fdf9525..d83cd318d6a95 100644 --- a/library/compiler-builtins/crates/symbol-check/src/main.rs +++ b/library/compiler-builtins/crates/symbol-check/src/main.rs @@ -46,15 +46,16 @@ fn main() { /// Run `cargo build` with the provided additional arguments, collecting the list of created /// libraries. fn exec_cargo_with_args(args: &[&str]) -> Vec { - let mut cmd = Command::new("cargo") - .arg("build") + let mut cmd = Command::new("cargo"); + cmd.arg("build") .arg("--message-format=json") .args(args) - .stdout(Stdio::piped()) - .spawn() - .expect("failed to launch Cargo"); + .stdout(Stdio::piped()); - let stdout = cmd.stdout.take().unwrap(); + println!("running: {cmd:?}"); + let mut child = cmd.spawn().expect("failed to launch Cargo"); + + let stdout = child.stdout.take().unwrap(); let reader = BufReader::new(stdout); let mut check_files = Vec::new(); @@ -84,7 +85,7 @@ fn exec_cargo_with_args(args: &[&str]) -> Vec { } } - assert!(cmd.wait().expect("failed to wait on Cargo").success()); + assert!(child.wait().expect("failed to wait on Cargo").success()); assert!(!check_files.is_empty(), "no compiler_builtins rlibs found"); println!("Collected the following rlibs to check: {check_files:#?}"); From 7db8cf1fea87dd42082ac6eeb405e609628ac6b1 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Thu, 29 May 2025 17:37:35 +0000 Subject: [PATCH 14/28] Add benchmarks for float parsing and printing As part of this, the u256 benchmarks are reorganized to a group. --- .../libm-test/benches/icount.rs | 100 +++++++++++++++--- 1 file changed, 83 insertions(+), 17 deletions(-) diff --git a/library/compiler-builtins/libm-test/benches/icount.rs b/library/compiler-builtins/libm-test/benches/icount.rs index da8c6bfd15a03..4bebbc41ccd87 100644 --- a/library/compiler-builtins/libm-test/benches/icount.rs +++ b/library/compiler-builtins/libm-test/benches/icount.rs @@ -1,9 +1,11 @@ //! Benchmarks that use `iai-cachegrind` to be reasonably CI-stable. +#![feature(f16)] +#![feature(f128)] use std::hint::black_box; use iai_callgrind::{library_benchmark, library_benchmark_group, main}; -use libm::support::{HInt, u256}; +use libm::support::{HInt, Hexf, hf16, hf32, hf64, hf128, u256}; use libm_test::generate::spaced; use libm_test::{CheckBasis, CheckCtx, GeneratorKind, MathOp, OpRustArgs, TupleCall, op}; @@ -109,11 +111,6 @@ fn icount_bench_u128_widen_mul(cases: Vec<(u128, u128)>) { } } -library_benchmark_group!( - name = icount_bench_u128_widen_mul_group; - benchmarks = icount_bench_u128_widen_mul -); - #[library_benchmark] #[bench::linspace(setup_u256_add())] fn icount_bench_u256_add(cases: Vec<(u256, u256)>) { @@ -122,11 +119,6 @@ fn icount_bench_u256_add(cases: Vec<(u256, u256)>) { } } -library_benchmark_group!( - name = icount_bench_u256_add_group; - benchmarks = icount_bench_u256_add -); - #[library_benchmark] #[bench::linspace(setup_u256_shift())] fn icount_bench_u256_shr(cases: Vec<(u256, u32)>) { @@ -136,16 +128,90 @@ fn icount_bench_u256_shr(cases: Vec<(u256, u32)>) { } library_benchmark_group!( - name = icount_bench_u256_shr_group; - benchmarks = icount_bench_u256_shr + name = icount_bench_u128_group; + benchmarks = icount_bench_u128_widen_mul, icount_bench_u256_add, icount_bench_u256_shr +); + +#[library_benchmark] +#[bench::short("0x12.34p+8")] +#[bench::max("0x1.ffcp+15")] +fn icount_bench_hf16(s: &str) -> f16 { + black_box(hf16(s)) +} + +#[library_benchmark] +#[bench::short("0x12.34p+8")] +#[bench::max("0x1.fffffep+127")] +fn icount_bench_hf32(s: &str) -> f32 { + black_box(hf32(s)) +} + +#[library_benchmark] +#[bench::short("0x12.34p+8")] +#[bench::max("0x1.fffffffffffffp+1023")] +fn icount_bench_hf64(s: &str) -> f64 { + black_box(hf64(s)) +} + +#[library_benchmark] +#[bench::short("0x12.34p+8")] +#[bench::max("0x1.ffffffffffffffffffffffffffffp+16383")] +fn icount_bench_hf128(s: &str) -> f128 { + black_box(hf128(s)) +} + +library_benchmark_group!( + name = icount_bench_hf_parse_group; + benchmarks = + icount_bench_hf16, + icount_bench_hf32, + icount_bench_hf64, + icount_bench_hf128 +); + +#[library_benchmark] +#[bench::short(1.015625)] +#[bench::max(f16::MAX)] +fn icount_bench_print_hf16(x: f16) -> String { + black_box(Hexf(x).to_string()) +} + +#[library_benchmark] +#[bench::short(1.015625)] +#[bench::max(f32::MAX)] +fn icount_bench_print_hf32(x: f32) -> String { + black_box(Hexf(x).to_string()) +} + +#[library_benchmark] +#[bench::short(1.015625)] +#[bench::max(f64::MAX)] +fn icount_bench_print_hf64(x: f64) -> String { + black_box(Hexf(x).to_string()) +} + +#[library_benchmark] +#[bench::short(1.015625)] +#[bench::max(f128::MAX)] +fn icount_bench_print_hf128(x: f128) -> String { + black_box(Hexf(x).to_string()) +} + +library_benchmark_group!( + name = icount_bench_hf_print_group; + benchmarks = + icount_bench_print_hf16, + icount_bench_print_hf32, + icount_bench_print_hf64, + icount_bench_print_hf128 ); main!( library_benchmark_groups = - // u256-related benchmarks - icount_bench_u128_widen_mul_group, - icount_bench_u256_add_group, - icount_bench_u256_shr_group, + // Benchmarks not related to public libm math + icount_bench_u128_group, + icount_bench_hf_parse_group, + icount_bench_hf_print_group, // verify-apilist-start // verify-sorted-start icount_bench_acos_group, From c136fb7734716a078cd82a7364245128923eb7e4 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Wed, 19 Mar 2025 04:10:03 +0000 Subject: [PATCH 15/28] Run `builtins-test-intrinsics` when possible Currently we only build this, but it is possible to run the binary. Change the CI script to do so here. --- .../builtins-test-intrinsics/src/main.rs | 6 ++-- library/compiler-builtins/ci/run.sh | 30 ++++++++++++------- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/library/compiler-builtins/builtins-test-intrinsics/src/main.rs b/library/compiler-builtins/builtins-test-intrinsics/src/main.rs index 96fe4a7385113..66744a0817fe3 100644 --- a/library/compiler-builtins/builtins-test-intrinsics/src/main.rs +++ b/library/compiler-builtins/builtins-test-intrinsics/src/main.rs @@ -13,6 +13,8 @@ #![no_std] #![no_main] +// Ensure this `compiler_builtins` gets used, rather than the version injected from the sysroot. +extern crate compiler_builtins; extern crate panic_handler; // SAFETY: no definitions, only used for linking @@ -652,14 +654,14 @@ fn something_with_a_dtor(f: &dyn Fn()) { #[unsafe(no_mangle)] #[cfg(not(thumb))] -fn main(_argc: core::ffi::c_int, _argv: *const *const u8) -> core::ffi::c_int { +extern "C" fn main(_argc: core::ffi::c_int, _argv: *const *const u8) -> core::ffi::c_int { run(); 0 } #[unsafe(no_mangle)] #[cfg(thumb)] -pub fn _start() -> ! { +extern "C" fn _start() -> ! { run(); loop {} } diff --git a/library/compiler-builtins/ci/run.sh b/library/compiler-builtins/ci/run.sh index cf3f7dfda1da2..27b9686eac644 100755 --- a/library/compiler-builtins/ci/run.sh +++ b/library/compiler-builtins/ci/run.sh @@ -63,23 +63,33 @@ symcheck+=(-- build-and-check) "${symcheck[@]}" -p compiler_builtins --target "$target" --features no-f16-f128 "${symcheck[@]}" -p compiler_builtins --target "$target" --features no-f16-f128 --release -build_intrinsics_test() { - # symcheck also checks the results of builtins-test-intrinsics - "${symcheck[@]}" \ +run_intrinsics_test() { + args=( --target "$target" --verbose \ - --manifest-path builtins-test-intrinsics/Cargo.toml "$@" + --manifest-path builtins-test-intrinsics/Cargo.toml + ) + args+=( "$@" ) + + # symcheck also checks the results of builtins-test-intrinsics + "${symcheck[@]}" "${args[@]}" + + # FIXME: we get access violations on Windows, our entrypoint may need to + # be tweaked. + if [ "${BUILD_ONLY:-}" != "1" ] && ! [[ "$target" = *"windows"* ]]; then + cargo run "${args[@]}" + fi } # Verify that we haven't dropped any intrinsics/symbols -build_intrinsics_test -build_intrinsics_test --release -build_intrinsics_test --features c -build_intrinsics_test --features c --release +run_intrinsics_test +run_intrinsics_test --release +run_intrinsics_test --features c +run_intrinsics_test --features c --release # Verify that there are no undefined symbols to `panic` within our # implementations -CARGO_PROFILE_DEV_LTO=true build_intrinsics_test -CARGO_PROFILE_RELEASE_LTO=true build_intrinsics_test --release +CARGO_PROFILE_DEV_LTO=true run_intrinsics_test +CARGO_PROFILE_RELEASE_LTO=true run_intrinsics_test --release # Test libm From 3464b4b231e3e4a09e102696228fe950eb1672e5 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Thu, 29 May 2025 19:01:03 +0000 Subject: [PATCH 16/28] ci: Allow concurrency outside of pull requests When multiple merges to `master` happen before a CI run completes, the in-progress job is getting canceled. Fix this by using the commit sha for the group key if a pull request number is not available, rather than `github.ref` (which is always `refs/head/master` after merge). This should prevent jobs running on previous commits from getting cancelled, while still ensuring there is only ever one active run per pull request. --- library/compiler-builtins/.github/workflows/main.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/compiler-builtins/.github/workflows/main.yaml b/library/compiler-builtins/.github/workflows/main.yaml index 567ad1205a5d3..de433d8c74d52 100644 --- a/library/compiler-builtins/.github/workflows/main.yaml +++ b/library/compiler-builtins/.github/workflows/main.yaml @@ -5,7 +5,7 @@ on: concurrency: # Make sure that new pushes cancel running jobs - group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.sha }} cancel-in-progress: true env: From 9f84e9904fd2be7e6b2c339b16ac9c2e5da8778b Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Thu, 29 May 2025 18:06:43 +0000 Subject: [PATCH 17/28] Increase the benchmark rustc version to 2025-05-28 We may soon want to use some new nightly features in `compiler-builtins` and `libm`, specifically `cfg_target_has_reliable_f16_f128` which was added in the past few weeks. This will mean we need a newer toolchain for benchmarks to continue building. Bump to the current latest nightly so we are not blocked on this down the line. --- library/compiler-builtins/.github/workflows/main.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/compiler-builtins/.github/workflows/main.yaml b/library/compiler-builtins/.github/workflows/main.yaml index de433d8c74d52..8e89cb4729242 100644 --- a/library/compiler-builtins/.github/workflows/main.yaml +++ b/library/compiler-builtins/.github/workflows/main.yaml @@ -13,7 +13,7 @@ env: RUSTDOCFLAGS: -Dwarnings RUSTFLAGS: -Dwarnings RUST_BACKTRACE: full - BENCHMARK_RUSTC: nightly-2025-01-16 # Pin the toolchain for reproducable results + BENCHMARK_RUSTC: nightly-2025-05-28 # Pin the toolchain for reproducable results jobs: # Determine which tests should be run based on changed files. From 8edaa6e5c88e14df20b095016514d381c697e49a Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Thu, 29 May 2025 20:53:48 +0000 Subject: [PATCH 18/28] libm-test: Make `extensive` an attribute rather than a test type Currently we run logspace tests for extensive tests, but there isn't any reason we couldn't also run more kinds of tests more extensively (e.g. more edge cases, combine edge cases with logspace for multi-input functions, etc). As a first step toward making this possible, make `extensive` a new field in `CheckCtx`, and rename `QuickSpaced` to `Spaced`. --- .../libm-test/benches/icount.rs | 2 +- .../libm-test/examples/plot_domains.rs | 2 +- .../libm-test/src/run_cfg.rs | 74 ++++++++++++++----- .../libm-test/tests/compare_built_musl.rs | 2 +- .../libm-test/tests/multiprecision.rs | 2 +- .../libm-test/tests/z_extensive/run.rs | 3 +- 6 files changed, 60 insertions(+), 25 deletions(-) diff --git a/library/compiler-builtins/libm-test/benches/icount.rs b/library/compiler-builtins/libm-test/benches/icount.rs index 4bebbc41ccd87..a0928a29f9992 100644 --- a/library/compiler-builtins/libm-test/benches/icount.rs +++ b/library/compiler-builtins/libm-test/benches/icount.rs @@ -23,7 +23,7 @@ macro_rules! icount_benches { let mut ctx = CheckCtx::new( Op::IDENTIFIER, CheckBasis::None, - GeneratorKind::QuickSpaced + GeneratorKind::Spaced ); ctx.override_iterations(BENCH_ITER_ITEMS); let ret = spaced::get_test_cases::(&ctx).0.collect::>(); diff --git a/library/compiler-builtins/libm-test/examples/plot_domains.rs b/library/compiler-builtins/libm-test/examples/plot_domains.rs index 3563103b8cd7c..7331d454f2111 100644 --- a/library/compiler-builtins/libm-test/examples/plot_domains.rs +++ b/library/compiler-builtins/libm-test/examples/plot_domains.rs @@ -55,7 +55,7 @@ where Op: MathOp, Op::RustArgs: SpacedInput, { - let mut ctx = CheckCtx::new(Op::IDENTIFIER, CheckBasis::Mpfr, GeneratorKind::QuickSpaced); + let mut ctx = CheckCtx::new(Op::IDENTIFIER, CheckBasis::Mpfr, GeneratorKind::Spaced); plot_one_generator( out_dir, &ctx, diff --git a/library/compiler-builtins/libm-test/src/run_cfg.rs b/library/compiler-builtins/libm-test/src/run_cfg.rs index 3345a01d2de79..90f81195c8560 100644 --- a/library/compiler-builtins/libm-test/src/run_cfg.rs +++ b/library/compiler-builtins/libm-test/src/run_cfg.rs @@ -22,13 +22,38 @@ static EXTENSIVE_ITER_OVERRIDE: LazyLock> = LazyLock::new(|| { /// Specific tests that need to have a reduced amount of iterations to complete in a reasonable /// amount of time. -/// -/// Contains the itentifier+generator combo to match on, plus the factor to reduce by. -const EXTEMELY_SLOW_TESTS: &[(Identifier, GeneratorKind, u64)] = &[ - (Identifier::Fmodf128, GeneratorKind::QuickSpaced, 50), - (Identifier::Fmodf128, GeneratorKind::Extensive, 50), +const EXTREMELY_SLOW_TESTS: &[SlowTest] = &[ + SlowTest { + ident: Identifier::Fmodf128, + gen_kind: GeneratorKind::Spaced, + extensive: false, + reduce_factor: 50, + }, + SlowTest { + ident: Identifier::Fmodf128, + gen_kind: GeneratorKind::Spaced, + extensive: true, + reduce_factor: 50, + }, ]; +/// A pattern to match a `CheckCtx`, plus a factor to reduce by. +struct SlowTest { + ident: Identifier, + gen_kind: GeneratorKind, + extensive: bool, + reduce_factor: u64, +} + +impl SlowTest { + /// True if the test in `CheckCtx` should be reduced by `reduce_factor`. + fn matches_ctx(&self, ctx: &CheckCtx) -> bool { + self.ident == ctx.fn_ident + && self.gen_kind == ctx.gen_kind + && self.extensive == ctx.extensive + } +} + /// Maximum number of iterations to run for a single routine. /// /// The default value of one greater than `u32::MAX` allows testing single-argument `f32` routines @@ -54,6 +79,7 @@ pub struct CheckCtx { /// Source of truth for tests. pub basis: CheckBasis, pub gen_kind: GeneratorKind, + pub extensive: bool, /// If specified, this value will override the value returned by [`iteration_count`]. pub override_iterations: Option, } @@ -69,12 +95,19 @@ impl CheckCtx { base_name_str: fn_ident.base_name().as_str(), basis, gen_kind, + extensive: false, override_iterations: None, }; ret.ulp = crate::default_ulp(&ret); ret } + /// Configure that this is an extensive test. + pub fn extensive(mut self, extensive: bool) -> Self { + self.extensive = extensive; + self + } + /// The number of input arguments for this function. pub fn input_count(&self) -> usize { self.fn_ident.math_op().rust_sig.args.len() @@ -100,14 +133,17 @@ pub enum CheckBasis { /// and quantity. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum GeneratorKind { + /// Extremes, zeros, nonstandard numbers, etc. EdgeCases, - Extensive, - QuickSpaced, + /// Spaced by logarithm (floats) or linear (integers). + Spaced, + /// Test inputs from an RNG. Random, + /// A provided test case list. List, } -/// A list of all functions that should get extensive tests. +/// A list of all functions that should get extensive tests, as configured by environment variable. /// /// This also supports the special test name `all` to run all tests, as well as `all_f16`, /// `all_f32`, `all_f64`, and `all_f128` to run all tests for a specific float type. @@ -216,17 +252,17 @@ pub fn iteration_count(ctx: &CheckCtx, argnum: usize) -> u64 { let random_iter_count = domain_iter_count / 100; let mut total_iterations = match ctx.gen_kind { - GeneratorKind::QuickSpaced => domain_iter_count, + GeneratorKind::Spaced if ctx.extensive => extensive_max_iterations(), + GeneratorKind::Spaced => domain_iter_count, GeneratorKind::Random => random_iter_count, - GeneratorKind::Extensive => extensive_max_iterations(), GeneratorKind::EdgeCases | GeneratorKind::List => { unimplemented!("shoudn't need `iteration_count` for {:?}", ctx.gen_kind) } }; // Larger float types get more iterations. - if t_env.large_float_ty && ctx.gen_kind != GeneratorKind::Extensive { - if ctx.gen_kind == GeneratorKind::Extensive { + if t_env.large_float_ty { + if ctx.extensive { // Extensive already has a pretty high test count. total_iterations *= 2; } else { @@ -244,13 +280,13 @@ pub fn iteration_count(ctx: &CheckCtx, argnum: usize) -> u64 { } // Some tests are significantly slower than others and need to be further reduced. - if let Some((_id, _gen, scale)) = EXTEMELY_SLOW_TESTS + if let Some(slow) = EXTREMELY_SLOW_TESTS .iter() - .find(|(id, generator, _scale)| *id == ctx.fn_ident && *generator == ctx.gen_kind) + .find(|slow| slow.matches_ctx(ctx)) { // However, do not override if the extensive iteration count has been manually set. - if !(ctx.gen_kind == GeneratorKind::Extensive && EXTENSIVE_ITER_OVERRIDE.is_some()) { - total_iterations /= scale; + if !(ctx.extensive && EXTENSIVE_ITER_OVERRIDE.is_some()) { + total_iterations /= slow.reduce_factor; } } @@ -279,7 +315,7 @@ pub fn iteration_count(ctx: &CheckCtx, argnum: usize) -> u64 { let total = ntests.pow(t_env.input_count.try_into().unwrap()); let seed_msg = match ctx.gen_kind { - GeneratorKind::QuickSpaced | GeneratorKind::Extensive => String::new(), + GeneratorKind::Spaced => String::new(), GeneratorKind::Random => { format!( " using `{SEED_ENV}={}`", @@ -327,8 +363,8 @@ pub fn int_range(ctx: &CheckCtx, argnum: usize) -> RangeInclusive { let extensive_range = (-0xfff)..=0xfffff; match ctx.gen_kind { - GeneratorKind::Extensive => extensive_range, - GeneratorKind::QuickSpaced | GeneratorKind::Random => non_extensive_range, + _ if ctx.extensive => extensive_range, + GeneratorKind::Spaced | GeneratorKind::Random => non_extensive_range, GeneratorKind::EdgeCases => extensive_range, GeneratorKind::List => unimplemented!("shoudn't need range for {:?}", ctx.gen_kind), } diff --git a/library/compiler-builtins/libm-test/tests/compare_built_musl.rs b/library/compiler-builtins/libm-test/tests/compare_built_musl.rs index 6ccbb6f4c51d5..86f3b8b711ea7 100644 --- a/library/compiler-builtins/libm-test/tests/compare_built_musl.rs +++ b/library/compiler-builtins/libm-test/tests/compare_built_musl.rs @@ -65,7 +65,7 @@ macro_rules! musl_tests { $(#[$attr])* fn [< musl_quickspace_ $fn_name >]() { type Op = libm_test::op::$fn_name::Routine; - let ctx = CheckCtx::new(Op::IDENTIFIER, BASIS, GeneratorKind::QuickSpaced); + let ctx = CheckCtx::new(Op::IDENTIFIER, BASIS, GeneratorKind::Spaced); let cases = spaced::get_test_cases::(&ctx).0; musl_runner::(&ctx, cases, musl_math_sys::$fn_name); } diff --git a/library/compiler-builtins/libm-test/tests/multiprecision.rs b/library/compiler-builtins/libm-test/tests/multiprecision.rs index 80b2c78688ea7..60175ae615693 100644 --- a/library/compiler-builtins/libm-test/tests/multiprecision.rs +++ b/library/compiler-builtins/libm-test/tests/multiprecision.rs @@ -55,7 +55,7 @@ macro_rules! mp_tests { $(#[$attr])* fn [< mp_quickspace_ $fn_name >]() { type Op = libm_test::op::$fn_name::Routine; - let ctx = CheckCtx::new(Op::IDENTIFIER, BASIS, GeneratorKind::QuickSpaced); + let ctx = CheckCtx::new(Op::IDENTIFIER, BASIS, GeneratorKind::Spaced); let cases = spaced::get_test_cases::(&ctx).0; mp_runner::(&ctx, cases); } diff --git a/library/compiler-builtins/libm-test/tests/z_extensive/run.rs b/library/compiler-builtins/libm-test/tests/z_extensive/run.rs index 59c806ce73e24..f2ba6a4a0e3e6 100644 --- a/library/compiler-builtins/libm-test/tests/z_extensive/run.rs +++ b/library/compiler-builtins/libm-test/tests/z_extensive/run.rs @@ -17,7 +17,6 @@ use rayon::prelude::*; use spaced::SpacedInput; const BASIS: CheckBasis = CheckBasis::Mpfr; -const GEN_KIND: GeneratorKind = GeneratorKind::Extensive; /// Run the extensive test suite. pub fn run() { @@ -77,7 +76,7 @@ where Op::RustArgs: SpacedInput + Send, { let test_name = format!("mp_extensive_{}", Op::NAME); - let ctx = CheckCtx::new(Op::IDENTIFIER, BASIS, GEN_KIND); + let ctx = CheckCtx::new(Op::IDENTIFIER, BASIS, GeneratorKind::Spaced).extensive(true); let skip = skip_extensive_test(&ctx); let runner = move || { From 4d325e8b2cca0139370a809a76183a880db52b24 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Thu, 29 May 2025 21:22:47 +0000 Subject: [PATCH 19/28] ci: Allow for multiple icount benchmarks in the same run We don't actually need this for now, but eventually it would be nice to run icount benchmarks on multiple targets. Start tagging artifact names with the architecture, and allow passing `--tag` to `ci-util.py` in order to retrieve the correct one. --- .../.github/workflows/main.yaml | 12 ++++++++++-- library/compiler-builtins/ci/bench-icount.sh | 16 ++++++++++++++-- library/compiler-builtins/ci/ci-util.py | 17 +++++++++++++---- 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/library/compiler-builtins/.github/workflows/main.yaml b/library/compiler-builtins/.github/workflows/main.yaml index 8e89cb4729242..9f389d8b48579 100644 --- a/library/compiler-builtins/.github/workflows/main.yaml +++ b/library/compiler-builtins/.github/workflows/main.yaml @@ -195,8 +195,14 @@ jobs: benchmarks: name: Benchmarks - runs-on: ubuntu-24.04 timeout-minutes: 20 + strategy: + fail-fast: false + matrix: + include: + - target: x86_64-unknown-linux-gnu + os: ubuntu-24.04 + runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@master with: @@ -215,12 +221,14 @@ jobs: cargo binstall -y iai-callgrind-runner --version "$iai_version" sudo apt-get install valgrind - uses: Swatinem/rust-cache@v2 + with: + key: ${{ matrix.target }} - name: Run icount benchmarks env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} PR_NUMBER: ${{ github.event.pull_request.number }} - run: ./ci/bench-icount.sh + run: ./ci/bench-icount.sh ${{ matrix.target }} - name: Upload the benchmark baseline uses: actions/upload-artifact@v4 diff --git a/library/compiler-builtins/ci/bench-icount.sh b/library/compiler-builtins/ci/bench-icount.sh index 4d93e257a6cb0..5b6974fe42048 100755 --- a/library/compiler-builtins/ci/bench-icount.sh +++ b/library/compiler-builtins/ci/bench-icount.sh @@ -2,10 +2,21 @@ set -eux +target="${1:-}" + +if [ -z "$target" ]; then + host_target=$(rustc -vV | awk '/^host/ { print $2 }') + echo "Defaulted to host target $host_target" + target="$host_target" +fi + iai_home="iai-home" +# Use the arch as a tag to disambiguate artifacts +tag="$(echo "$target" | cut -d'-' -f1)" + # Download the baseline from master -./ci/ci-util.py locate-baseline --download --extract +./ci/ci-util.py locate-baseline --download --extract --tag "$tag" # Run benchmarks once function run_icount_benchmarks() { @@ -44,6 +55,7 @@ function run_icount_benchmarks() { # If this is for a pull request, ignore regressions if specified. ./ci/ci-util.py check-regressions --home "$iai_home" --allow-pr-override "$PR_NUMBER" else + # Disregard regressions after merge ./ci/ci-util.py check-regressions --home "$iai_home" || true fi } @@ -53,6 +65,6 @@ run_icount_benchmarks --features force-soft-floats -- --save-baseline=softfloat run_icount_benchmarks -- --save-baseline=hardfloat # Name and tar the new baseline -name="baseline-icount-$(date -u +'%Y%m%d%H%M')-${GITHUB_SHA:0:12}" +name="baseline-icount-$tag-$(date -u +'%Y%m%d%H%M')-${GITHUB_SHA:0:12}" echo "BASELINE_NAME=$name" >>"$GITHUB_ENV" tar cJf "$name.tar.xz" "$iai_home" diff --git a/library/compiler-builtins/ci/ci-util.py b/library/compiler-builtins/ci/ci-util.py index d785b2e9e1d80..6c8b439805b44 100755 --- a/library/compiler-builtins/ci/ci-util.py +++ b/library/compiler-builtins/ci/ci-util.py @@ -28,11 +28,14 @@ Calculate a matrix of which functions had source change, print that as a JSON object. - locate-baseline [--download] [--extract] + locate-baseline [--download] [--extract] [--tag TAG] Locate the most recent benchmark baseline available in CI and, if flags specify, download and extract it. Never exits with nonzero status if downloading fails. + `--tag` can be specified to look for artifacts with a specific tag, such as + for a specific architecture. + Note that `--extract` will overwrite files in `iai-home`. check-regressions [--home iai-home] [--allow-pr-override pr_number] @@ -50,7 +53,7 @@ GIT = ["git", "-C", REPO_ROOT] DEFAULT_BRANCH = "master" WORKFLOW_NAME = "CI" # Workflow that generates the benchmark artifacts -ARTIFACT_GLOB = "baseline-icount*" +ARTIFACT_PREFIX = "baseline-icount*" # Place this in a PR body to skip regression checks (must be at the start of a line). REGRESSION_DIRECTIVE = "ci: allow-regressions" # Place this in a PR body to skip extensive tests @@ -278,6 +281,7 @@ def locate_baseline(flags: list[str]) -> None: download = False extract = False + tag = "" while len(flags) > 0: match flags[0]: @@ -285,6 +289,9 @@ def locate_baseline(flags: list[str]) -> None: download = True case "--extract": extract = True + case "--tag": + tag = flags[1] + flags = flags[1:] case _: eprint(USAGE) exit(1) @@ -333,8 +340,10 @@ def locate_baseline(flags: list[str]) -> None: eprint("skipping download step") return + artifact_glob = f"{ARTIFACT_PREFIX}{f"-{tag}" if tag else ""}*" + sp.run( - ["gh", "run", "download", str(job_id), f"--pattern={ARTIFACT_GLOB}"], + ["gh", "run", "download", str(job_id), f"--pattern={artifact_glob}"], check=False, ) @@ -344,7 +353,7 @@ def locate_baseline(flags: list[str]) -> None: # Find the baseline with the most recent timestamp. GH downloads the files to e.g. # `some-dirname/some-dirname.tar.xz`, so just glob the whole thing together. - candidate_baselines = glob(f"{ARTIFACT_GLOB}/{ARTIFACT_GLOB}") + candidate_baselines = glob(f"{artifact_glob}/{artifact_glob}") if len(candidate_baselines) == 0: eprint("no possible baseline directories found") return From 3c35505fe9c1a0f2c15c780709ed9812f8a332f0 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 29 May 2025 22:08:24 +0000 Subject: [PATCH 20/28] chore: release --- .../compiler-builtins/CHANGELOG.md | 15 +++++++++++++++ .../compiler-builtins/Cargo.toml | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/library/compiler-builtins/compiler-builtins/CHANGELOG.md b/library/compiler-builtins/compiler-builtins/CHANGELOG.md index a7c01c463ca68..880e56c443e3e 100644 --- a/library/compiler-builtins/compiler-builtins/CHANGELOG.md +++ b/library/compiler-builtins/compiler-builtins/CHANGELOG.md @@ -7,6 +7,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.1.160](https://github.com/rust-lang/compiler-builtins/compare/compiler_builtins-v0.1.159...compiler_builtins-v0.1.160) - 2025-05-29 + +### Other + +- Change `compiler-builtins` to edition 2024 +- Remove unneeded C symbols +- Reuse `libm`'s `Caat` and `CastFrom` in `compiler-builtins` +- Reuse `MinInt` and `Int` from `libm` in `compiler-builtins` +- Update `CmpResult` to use a pointer-sized return type +- Enable `__powitf2` on MSVC +- Fix `i256::MAX` +- Add a note saying why we use `frintx` rather than `frintn` +- Typo in README.md +- Clean up unused files + ## [0.1.159](https://github.com/rust-lang/compiler-builtins/compare/compiler_builtins-v0.1.158...compiler_builtins-v0.1.159) - 2025-05-12 ### Other diff --git a/library/compiler-builtins/compiler-builtins/Cargo.toml b/library/compiler-builtins/compiler-builtins/Cargo.toml index 93eb3e01b3b8a..8ceef286f57e4 100644 --- a/library/compiler-builtins/compiler-builtins/Cargo.toml +++ b/library/compiler-builtins/compiler-builtins/Cargo.toml @@ -1,7 +1,7 @@ [package] authors = ["Jorge Aparicio "] name = "compiler_builtins" -version = "0.1.159" +version = "0.1.160" license = "MIT AND Apache-2.0 WITH LLVM-exception AND (MIT OR Apache-2.0)" readme = "README.md" repository = "https://github.com/rust-lang/compiler-builtins" From c6df6a72d8f2463985b6b130673536e6d4cc7cbf Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Sun, 1 Jun 2025 19:41:03 +0000 Subject: [PATCH 21/28] Fix new `dead_code` warnings from recent nightlies --- .../libm/src/math/support/float_traits.rs | 1 + .../libm/src/math/support/hex_float.rs | 207 +++++++++--------- .../libm/src/math/support/int_traits.rs | 1 + .../libm/src/math/support/macros.rs | 6 +- .../libm/src/math/support/mod.rs | 4 +- 5 files changed, 116 insertions(+), 103 deletions(-) diff --git a/library/compiler-builtins/libm/src/math/support/float_traits.rs b/library/compiler-builtins/libm/src/math/support/float_traits.rs index 4c866ef10bdd7..dd9f46209c11d 100644 --- a/library/compiler-builtins/libm/src/math/support/float_traits.rs +++ b/library/compiler-builtins/libm/src/math/support/float_traits.rs @@ -6,6 +6,7 @@ use super::int_traits::{CastFrom, Int, MinInt}; /// Trait for some basic operations on floats // #[allow(dead_code)] +#[allow(dead_code)] // Some constants are only used with tests pub trait Float: Copy + fmt::Debug diff --git a/library/compiler-builtins/libm/src/math/support/hex_float.rs b/library/compiler-builtins/libm/src/math/support/hex_float.rs index 85569d98aef48..c8558b90053d1 100644 --- a/library/compiler-builtins/libm/src/math/support/hex_float.rs +++ b/library/compiler-builtins/libm/src/math/support/hex_float.rs @@ -1,8 +1,6 @@ //! Utilities for working with hex float formats. -use core::fmt; - -use super::{Float, Round, Status, f32_from_bits, f64_from_bits}; +use super::{Round, Status, f32_from_bits, f64_from_bits}; /// Construct a 16-bit float from hex float representation (C-style) #[cfg(f16_enabled)] @@ -352,133 +350,143 @@ const fn u128_ilog2(v: u128) -> u32 { u128::BITS - 1 - v.leading_zeros() } -/// Format a floating point number as its IEEE hex (`%a`) representation. -pub struct Hexf(pub F); +#[cfg(any(test, feature = "unstable-public-internals"))] +mod hex_fmt { + use core::fmt; -// Adapted from https://github.com/ericseppanen/hexfloat2/blob/a5c27932f0ff/src/format.rs -#[cfg(not(feature = "compiler-builtins"))] -fn fmt_any_hex(x: &F, f: &mut fmt::Formatter<'_>) -> fmt::Result { - if x.is_sign_negative() { - write!(f, "-")?; - } + use crate::support::Float; - if x.is_nan() { - return write!(f, "NaN"); - } else if x.is_infinite() { - return write!(f, "inf"); - } else if *x == F::ZERO { - return write!(f, "0x0p+0"); - } + /// Format a floating point number as its IEEE hex (`%a`) representation. + pub struct Hexf(pub F); - let mut exponent = x.exp_unbiased(); - let sig = x.to_bits() & F::SIG_MASK; - - let bias = F::EXP_BIAS as i32; - // The mantissa MSB needs to be shifted up to the nearest nibble. - let mshift = (4 - (F::SIG_BITS % 4)) % 4; - let sig = sig << mshift; - // The width is rounded up to the nearest char (4 bits) - let mwidth = (F::SIG_BITS as usize + 3) / 4; - let leading = if exponent == -bias { - // subnormal number means we shift our output by 1 bit. - exponent += 1; - "0." - } else { - "1." - }; + // Adapted from https://github.com/ericseppanen/hexfloat2/blob/a5c27932f0ff/src/format.rs + #[cfg(not(feature = "compiler-builtins"))] + pub(super) fn fmt_any_hex(x: &F, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if x.is_sign_negative() { + write!(f, "-")?; + } - write!(f, "0x{leading}{sig:0mwidth$x}p{exponent:+}") -} + if x.is_nan() { + return write!(f, "NaN"); + } else if x.is_infinite() { + return write!(f, "inf"); + } else if *x == F::ZERO { + return write!(f, "0x0p+0"); + } -#[cfg(feature = "compiler-builtins")] -fn fmt_any_hex(_x: &F, _f: &mut fmt::Formatter<'_>) -> fmt::Result { - unimplemented!() -} + let mut exponent = x.exp_unbiased(); + let sig = x.to_bits() & F::SIG_MASK; + + let bias = F::EXP_BIAS as i32; + // The mantissa MSB needs to be shifted up to the nearest nibble. + let mshift = (4 - (F::SIG_BITS % 4)) % 4; + let sig = sig << mshift; + // The width is rounded up to the nearest char (4 bits) + let mwidth = (F::SIG_BITS as usize + 3) / 4; + let leading = if exponent == -bias { + // subnormal number means we shift our output by 1 bit. + exponent += 1; + "0." + } else { + "1." + }; -impl fmt::LowerHex for Hexf { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - cfg_if! { - if #[cfg(feature = "compiler-builtins")] { - let _ = f; - unimplemented!() - } else { - fmt_any_hex(&self.0, f) + write!(f, "0x{leading}{sig:0mwidth$x}p{exponent:+}") + } + + #[cfg(feature = "compiler-builtins")] + pub(super) fn fmt_any_hex(_x: &F, _f: &mut fmt::Formatter<'_>) -> fmt::Result { + unimplemented!() + } + + impl fmt::LowerHex for Hexf { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + cfg_if! { + if #[cfg(feature = "compiler-builtins")] { + let _ = f; + unimplemented!() + } else { + fmt_any_hex(&self.0, f) + } } } } -} -impl fmt::LowerHex for Hexf<(F, F)> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - cfg_if! { - if #[cfg(feature = "compiler-builtins")] { - let _ = f; - unimplemented!() - } else { - write!(f, "({:x}, {:x})", Hexf(self.0.0), Hexf(self.0.1)) + impl fmt::LowerHex for Hexf<(F, F)> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + cfg_if! { + if #[cfg(feature = "compiler-builtins")] { + let _ = f; + unimplemented!() + } else { + write!(f, "({:x}, {:x})", Hexf(self.0.0), Hexf(self.0.1)) + } } } } -} -impl fmt::LowerHex for Hexf<(F, i32)> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - cfg_if! { - if #[cfg(feature = "compiler-builtins")] { - let _ = f; - unimplemented!() - } else { - write!(f, "({:x}, {:x})", Hexf(self.0.0), Hexf(self.0.1)) + impl fmt::LowerHex for Hexf<(F, i32)> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + cfg_if! { + if #[cfg(feature = "compiler-builtins")] { + let _ = f; + unimplemented!() + } else { + write!(f, "({:x}, {:x})", Hexf(self.0.0), Hexf(self.0.1)) + } } } } -} -impl fmt::LowerHex for Hexf { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - cfg_if! { - if #[cfg(feature = "compiler-builtins")] { - let _ = f; - unimplemented!() - } else { - fmt::LowerHex::fmt(&self.0, f) + impl fmt::LowerHex for Hexf { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + cfg_if! { + if #[cfg(feature = "compiler-builtins")] { + let _ = f; + unimplemented!() + } else { + fmt::LowerHex::fmt(&self.0, f) + } } } } -} -impl fmt::Debug for Hexf -where - Hexf: fmt::LowerHex, -{ - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - cfg_if! { - if #[cfg(feature = "compiler-builtins")] { - let _ = f; - unimplemented!() - } else { - fmt::LowerHex::fmt(self, f) + impl fmt::Debug for Hexf + where + Hexf: fmt::LowerHex, + { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + cfg_if! { + if #[cfg(feature = "compiler-builtins")] { + let _ = f; + unimplemented!() + } else { + fmt::LowerHex::fmt(self, f) + } } } } -} -impl fmt::Display for Hexf -where - Hexf: fmt::LowerHex, -{ - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - cfg_if! { - if #[cfg(feature = "compiler-builtins")] { - let _ = f; - unimplemented!() - } else { - fmt::LowerHex::fmt(self, f) + impl fmt::Display for Hexf + where + Hexf: fmt::LowerHex, + { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + cfg_if! { + if #[cfg(feature = "compiler-builtins")] { + let _ = f; + unimplemented!() + } else { + fmt::LowerHex::fmt(self, f) + } } } } } +#[cfg(any(test, feature = "unstable-public-internals"))] +pub use hex_fmt::*; + #[cfg(test)] mod parse_tests { extern crate std; @@ -1064,6 +1072,7 @@ mod print_tests { use std::string::ToString; use super::*; + use crate::support::Float; #[test] #[cfg(f16_enabled)] diff --git a/library/compiler-builtins/libm/src/math/support/int_traits.rs b/library/compiler-builtins/libm/src/math/support/int_traits.rs index 716af748a9fb8..9b29e2f4561d5 100644 --- a/library/compiler-builtins/libm/src/math/support/int_traits.rs +++ b/library/compiler-builtins/libm/src/math/support/int_traits.rs @@ -1,6 +1,7 @@ use core::{cmp, fmt, ops}; /// Minimal integer implementations needed on all integer types, including wide integers. +#[allow(dead_code)] // Some constants are only used with tests pub trait MinInt: Copy + fmt::Debug diff --git a/library/compiler-builtins/libm/src/math/support/macros.rs b/library/compiler-builtins/libm/src/math/support/macros.rs index 0b72db0e46e8b..2b8fd580a50e5 100644 --- a/library/compiler-builtins/libm/src/math/support/macros.rs +++ b/library/compiler-builtins/libm/src/math/support/macros.rs @@ -137,12 +137,12 @@ macro_rules! hf128 { #[cfg(test)] macro_rules! assert_biteq { ($left:expr, $right:expr, $($tt:tt)*) => {{ - use $crate::support::Int; let l = $left; let r = $right; - let bits = Int::leading_zeros(l.to_bits() - l.to_bits()); // hack to get the width from the value + // hack to get width from a value + let bits = $crate::support::Int::leading_zeros(l.to_bits() - l.to_bits()); assert!( - l.biteq(r), + $crate::support::Float::biteq(l, r), "{}\nl: {l:?} ({lb:#0width$x})\nr: {r:?} ({rb:#0width$x})", format_args!($($tt)*), lb = l.to_bits(), diff --git a/library/compiler-builtins/libm/src/math/support/mod.rs b/library/compiler-builtins/libm/src/math/support/mod.rs index 2771cfd32c8b5..2e7edd03c421e 100644 --- a/library/compiler-builtins/libm/src/math/support/mod.rs +++ b/library/compiler-builtins/libm/src/math/support/mod.rs @@ -17,6 +17,8 @@ pub use env::{FpResult, Round, Status}; #[allow(unused_imports)] pub use float_traits::{DFloat, Float, HFloat, IntTy}; pub(crate) use float_traits::{f32_from_bits, f64_from_bits}; +#[cfg(any(test, feature = "unstable-public-internals"))] +pub use hex_float::Hexf; #[cfg(f16_enabled)] #[allow(unused_imports)] pub use hex_float::hf16; @@ -24,7 +26,7 @@ pub use hex_float::hf16; #[allow(unused_imports)] pub use hex_float::hf128; #[allow(unused_imports)] -pub use hex_float::{Hexf, hf32, hf64}; +pub use hex_float::{hf32, hf64}; pub use int_traits::{CastFrom, CastInto, DInt, HInt, Int, MinInt}; /// Hint to the compiler that the current path is cold. From 0a7e59265a3432d6544b35d93120b66f45a67159 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Sun, 1 Jun 2025 19:22:42 +0000 Subject: [PATCH 22/28] Upgrade all dependencies to the latest available version In particular, this includes a fix to `iai-callgrind` that will allow us to simplify our benchmark runner. --- library/compiler-builtins/builtins-test/Cargo.toml | 8 ++++---- .../compiler-builtins/compiler-builtins/Cargo.toml | 4 ++-- .../crates/libm-macros/Cargo.toml | 4 ++-- .../crates/musl-math-sys/Cargo.toml | 2 +- library/compiler-builtins/libm-test/Cargo.toml | 14 +++++++------- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/library/compiler-builtins/builtins-test/Cargo.toml b/library/compiler-builtins/builtins-test/Cargo.toml index 10978c0bb7ed0..c7742aa24275f 100644 --- a/library/compiler-builtins/builtins-test/Cargo.toml +++ b/library/compiler-builtins/builtins-test/Cargo.toml @@ -10,11 +10,11 @@ license = "MIT AND Apache-2.0 WITH LLVM-exception AND (MIT OR Apache-2.0)" # 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. # `xoshiro128**` is used for its quality, size, and speed at generating `u32` shift amounts. -rand_xoshiro = "0.6" +rand_xoshiro = "0.7" # To compare float builtins against -rustc_apfloat = "0.2.1" +rustc_apfloat = "0.2.2" # Really a dev dependency, but dev dependencies can't be optional -iai-callgrind = { version = "0.14.0", optional = true } +iai-callgrind = { version = "0.14.1", optional = true } [dependencies.compiler_builtins] path = "../compiler-builtins" @@ -22,7 +22,7 @@ default-features = false features = ["unstable-public-internals"] [dev-dependencies] -criterion = { version = "0.5.1", default-features = false, features = ["cargo_bench_support"] } +criterion = { version = "0.6.0", default-features = false, features = ["cargo_bench_support"] } paste = "1.0.15" [target.'cfg(all(target_arch = "arm", not(any(target_env = "gnu", target_env = "musl")), target_os = "linux"))'.dev-dependencies] diff --git a/library/compiler-builtins/compiler-builtins/Cargo.toml b/library/compiler-builtins/compiler-builtins/Cargo.toml index 8ceef286f57e4..6bee8da68a0a2 100644 --- a/library/compiler-builtins/compiler-builtins/Cargo.toml +++ b/library/compiler-builtins/compiler-builtins/Cargo.toml @@ -19,10 +19,10 @@ test = false [dependencies] # For more information on this dependency see # https://github.com/rust-lang/rust/tree/master/library/rustc-std-workspace-core -core = { version = "1.0.0", optional = true, package = "rustc-std-workspace-core" } +core = { version = "1.0.1", optional = true, package = "rustc-std-workspace-core" } [build-dependencies] -cc = { optional = true, version = "1.0" } +cc = { optional = true, version = "1.2" } [dev-dependencies] panic-handler = { path = "../crates/panic-handler" } diff --git a/library/compiler-builtins/crates/libm-macros/Cargo.toml b/library/compiler-builtins/crates/libm-macros/Cargo.toml index 3929854f08e65..6bbf47784ff91 100644 --- a/library/compiler-builtins/crates/libm-macros/Cargo.toml +++ b/library/compiler-builtins/crates/libm-macros/Cargo.toml @@ -10,9 +10,9 @@ proc-macro = true [dependencies] heck = "0.5.0" -proc-macro2 = "1.0.94" +proc-macro2 = "1.0.95" quote = "1.0.40" -syn = { version = "2.0.100", features = ["full", "extra-traits", "visit-mut"] } +syn = { version = "2.0.101", features = ["full", "extra-traits", "visit-mut"] } [lints.rust] # Values used during testing diff --git a/library/compiler-builtins/crates/musl-math-sys/Cargo.toml b/library/compiler-builtins/crates/musl-math-sys/Cargo.toml index d3fb147e526aa..3b88117343b62 100644 --- a/library/compiler-builtins/crates/musl-math-sys/Cargo.toml +++ b/library/compiler-builtins/crates/musl-math-sys/Cargo.toml @@ -11,4 +11,4 @@ license = "MIT OR Apache-2.0" libm = { path = "../../libm" } [build-dependencies] -cc = "1.2.16" +cc = "1.2.25" diff --git a/library/compiler-builtins/libm-test/Cargo.toml b/library/compiler-builtins/libm-test/Cargo.toml index 7a306e735577e..01b45716b7a21 100644 --- a/library/compiler-builtins/libm-test/Cargo.toml +++ b/library/compiler-builtins/libm-test/Cargo.toml @@ -28,28 +28,28 @@ icount = ["dep:iai-callgrind"] short-benchmarks = [] [dependencies] -anyhow = "1.0.97" +anyhow = "1.0.98" # This is not directly used but is required so we can enable `gmp-mpfr-sys/force-cross`. -gmp-mpfr-sys = { version = "1.6.4", optional = true, default-features = false } -iai-callgrind = { version = "0.14.0", optional = true } +gmp-mpfr-sys = { version = "1.6.5", optional = true, default-features = false } +iai-callgrind = { version = "0.14.1", optional = true } indicatif = { version = "0.17.11", default-features = false } libm = { path = "../libm", features = ["unstable-public-internals"] } libm-macros = { path = "../crates/libm-macros" } musl-math-sys = { path = "../crates/musl-math-sys", optional = true } paste = "1.0.15" -rand = "0.9.0" +rand = "0.9.1" rand_chacha = "0.9.0" rayon = "1.10.0" rug = { version = "1.27.0", optional = true, default-features = false, features = ["float", "integer", "std"] } [target.'cfg(target_family = "wasm")'.dependencies] -getrandom = { version = "0.3.2", features = ["wasm_js"] } +getrandom = { version = "0.3.3", features = ["wasm_js"] } [build-dependencies] -rand = { version = "0.9.0", optional = true } +rand = { version = "0.9.1", optional = true } [dev-dependencies] -criterion = { version = "0.5.1", default-features = false, features = ["cargo_bench_support"] } +criterion = { version = "0.6.0", default-features = false, features = ["cargo_bench_support"] } libtest-mimic = "0.8.1" [[bench]] From e83ca863412a950f19fbc4e8fc632f19107dd0a2 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Mon, 2 Jun 2025 16:10:49 +0000 Subject: [PATCH 23/28] cleanup: Use `x.biteq(y)` rather than `x.to_bits() == y.to_bits()` --- library/compiler-builtins/libm-test/src/precision.rs | 2 +- .../compiler-builtins/libm-test/src/test_traits.rs | 5 +---- .../libm/src/math/generic/fmaximum.rs | 2 +- .../libm/src/math/generic/fmaximum_num.rs | 11 +++++------ .../libm/src/math/generic/fminimum.rs | 2 +- .../libm/src/math/generic/fminimum_num.rs | 11 +++++------ 6 files changed, 14 insertions(+), 19 deletions(-) diff --git a/library/compiler-builtins/libm-test/src/precision.rs b/library/compiler-builtins/libm-test/src/precision.rs index f5fb5f6707b08..f6cdd015a593c 100644 --- a/library/compiler-builtins/libm-test/src/precision.rs +++ b/library/compiler-builtins/libm-test/src/precision.rs @@ -381,7 +381,7 @@ fn unop_common( } // abs and copysign require signaling NaNs to be propagated, so verify bit equality. - if actual.to_bits() == expected.to_bits() { + if actual.biteq(expected) { return CheckAction::Custom(Ok(())); } else { return CheckAction::Custom(Err(anyhow::anyhow!("NaNs have different bitpatterns"))); diff --git a/library/compiler-builtins/libm-test/src/test_traits.rs b/library/compiler-builtins/libm-test/src/test_traits.rs index dbb97016153c7..2af6af60ba270 100644 --- a/library/compiler-builtins/libm-test/src/test_traits.rs +++ b/library/compiler-builtins/libm-test/src/test_traits.rs @@ -328,10 +328,7 @@ where // Check when both are NaNs if actual.is_nan() && expected.is_nan() { if require_biteq && ctx.basis == CheckBasis::None { - ensure!( - actual.to_bits() == expected.to_bits(), - "mismatched NaN bitpatterns" - ); + ensure!(actual.biteq(expected), "mismatched NaN bitpatterns"); } // By default, NaNs have nothing special to check. return Ok(()); diff --git a/library/compiler-builtins/libm/src/math/generic/fmaximum.rs b/library/compiler-builtins/libm/src/math/generic/fmaximum.rs index 4b6295bc0c6bc..898828b80c7ff 100644 --- a/library/compiler-builtins/libm/src/math/generic/fmaximum.rs +++ b/library/compiler-builtins/libm/src/math/generic/fmaximum.rs @@ -17,7 +17,7 @@ pub fn fmaximum(x: F, y: F) -> F { x } else if y.is_nan() { y - } else if x > y || (y.to_bits() == F::NEG_ZERO.to_bits() && x.is_sign_positive()) { + } else if x > y || (y.biteq(F::NEG_ZERO) && x.is_sign_positive()) { x } else { y diff --git a/library/compiler-builtins/libm/src/math/generic/fmaximum_num.rs b/library/compiler-builtins/libm/src/math/generic/fmaximum_num.rs index 2e97ff6d36905..05df6cbd4643e 100644 --- a/library/compiler-builtins/libm/src/math/generic/fmaximum_num.rs +++ b/library/compiler-builtins/libm/src/math/generic/fmaximum_num.rs @@ -15,12 +15,11 @@ use crate::support::Float; #[inline] pub fn fmaximum_num(x: F, y: F) -> F { - let res = - if x.is_nan() || x < y || (x.to_bits() == F::NEG_ZERO.to_bits() && y.is_sign_positive()) { - y - } else { - x - }; + let res = if x.is_nan() || x < y || (x.biteq(F::NEG_ZERO) && y.is_sign_positive()) { + y + } else { + x + }; // Canonicalize res * F::ONE diff --git a/library/compiler-builtins/libm/src/math/generic/fminimum.rs b/library/compiler-builtins/libm/src/math/generic/fminimum.rs index 9dc0b64be3f2c..8592ac5460ef0 100644 --- a/library/compiler-builtins/libm/src/math/generic/fminimum.rs +++ b/library/compiler-builtins/libm/src/math/generic/fminimum.rs @@ -17,7 +17,7 @@ pub fn fminimum(x: F, y: F) -> F { x } else if y.is_nan() { y - } else if x < y || (x.to_bits() == F::NEG_ZERO.to_bits() && y.is_sign_positive()) { + } else if x < y || (x.biteq(F::NEG_ZERO) && y.is_sign_positive()) { x } else { y diff --git a/library/compiler-builtins/libm/src/math/generic/fminimum_num.rs b/library/compiler-builtins/libm/src/math/generic/fminimum_num.rs index 40db8b18957bc..6777bbf87721b 100644 --- a/library/compiler-builtins/libm/src/math/generic/fminimum_num.rs +++ b/library/compiler-builtins/libm/src/math/generic/fminimum_num.rs @@ -15,12 +15,11 @@ use crate::support::Float; #[inline] pub fn fminimum_num(x: F, y: F) -> F { - let res = - if y.is_nan() || x < y || (x.to_bits() == F::NEG_ZERO.to_bits() && y.is_sign_positive()) { - x - } else { - y - }; + let res = if y.is_nan() || x < y || (x.biteq(F::NEG_ZERO) && y.is_sign_positive()) { + x + } else { + y + }; // Canonicalize res * F::ONE From ba7cdb681468ef7617bfa5a2bde7e3fb3d79a2be Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Sun, 1 Jun 2025 19:52:57 +0000 Subject: [PATCH 24/28] ci: Refactor benchmark regression checks iai-callgrind now correctly exits with error if regressions were found [1], so we no longer need to check for regressions manually. Remove this check and instead exit based on the exit status of the benchmark run. [1] https://github.com/iai-callgrind/iai-callgrind/issues/337 --- library/compiler-builtins/ci/bench-icount.sh | 19 ++--- library/compiler-builtins/ci/ci-util.py | 84 +++++--------------- 2 files changed, 29 insertions(+), 74 deletions(-) diff --git a/library/compiler-builtins/ci/bench-icount.sh b/library/compiler-builtins/ci/bench-icount.sh index 5b6974fe42048..5724955fe3672 100755 --- a/library/compiler-builtins/ci/bench-icount.sh +++ b/library/compiler-builtins/ci/bench-icount.sh @@ -46,17 +46,18 @@ function run_icount_benchmarks() { shift done - # Run iai-callgrind benchmarks - cargo bench "${cargo_args[@]}" -- "${iai_args[@]}" + # Run iai-callgrind benchmarks. Do this in a subshell with `&& true` to + # capture rather than exit on error. + (cargo bench "${cargo_args[@]}" -- "${iai_args[@]}") && true + exit_code="$?" - # NB: iai-callgrind should exit on error but does not, so we inspect the sumary - # for errors. See https://github.com/iai-callgrind/iai-callgrind/issues/337 - if [ -n "${PR_NUMBER:-}" ]; then - # If this is for a pull request, ignore regressions if specified. - ./ci/ci-util.py check-regressions --home "$iai_home" --allow-pr-override "$PR_NUMBER" - else + if [ "$exit_code" -eq 0 ]; then + echo "Benchmarks completed with no regressions" + elif [ -z "${PR_NUMBER:-}" ]; then # Disregard regressions after merge - ./ci/ci-util.py check-regressions --home "$iai_home" || true + echo "Benchmarks completed with regressions; ignoring (not in a PR)" + else + ./ci/ci-util.py handle-banch-regressions "$PR_NUMBER" fi } diff --git a/library/compiler-builtins/ci/ci-util.py b/library/compiler-builtins/ci/ci-util.py index 6c8b439805b44..3437d304f48c5 100755 --- a/library/compiler-builtins/ci/ci-util.py +++ b/library/compiler-builtins/ci/ci-util.py @@ -11,7 +11,7 @@ import subprocess as sp import sys from dataclasses import dataclass -from glob import glob, iglob +from glob import glob from inspect import cleandoc from os import getenv from pathlib import Path @@ -38,14 +38,10 @@ Note that `--extract` will overwrite files in `iai-home`. - check-regressions [--home iai-home] [--allow-pr-override pr_number] - Check `iai-home` (or `iai-home` if unspecified) for `summary.json` - files and see if there are any regressions. This is used as a workaround - for `iai-callgrind` not exiting with error status; see - . - - If `--allow-pr-override` is specified, the regression check will not exit - with failure if any line in the PR starts with `allow-regressions`. + handle-bench-regressions PR_NUMBER + Exit with success if the pull request contains a line starting with + `ci: allow-regressions`, indicating that regressions in benchmarks should + be accepted. Otherwise, exit 1. """ ) @@ -365,64 +361,22 @@ def locate_baseline(flags: list[str]) -> None: eprint("baseline extracted successfully") -def check_iai_regressions(args: list[str]): - """Find regressions in iai summary.json files, exit with failure if any are - found. - """ - - iai_home_str = "iai-home" - pr_number = None - - while len(args) > 0: - match args: - case ["--home", home, *rest]: - iai_home_str = home - args = rest - case ["--allow-pr-override", pr_num, *rest]: - pr_number = pr_num - args = rest - case _: - eprint(USAGE) - exit(1) - - iai_home = Path(iai_home_str) - - found_summaries = False - regressions: list[dict] = [] - for summary_path in iglob("**/summary.json", root_dir=iai_home, recursive=True): - found_summaries = True - with open(iai_home / summary_path, "r") as f: - summary = json.load(f) - - summary_regs = [] - run = summary["callgrind_summary"]["callgrind_run"] - fname = summary["function_name"] - id = summary["id"] - name_entry = {"name": f"{fname}.{id}"} - - for segment in run["segments"]: - summary_regs.extend(segment["regressions"]) +def handle_bench_regressions(args: list[str]): + """Exit with error unless the PR message contains an ignore directive.""" - summary_regs.extend(run["total"]["regressions"]) - - regressions.extend(name_entry | reg for reg in summary_regs) - - if not found_summaries: - eprint(f"did not find any summary.json files within {iai_home}") - exit(1) + match args: + case [pr_number]: + pr_number = pr_number + case _: + eprint(USAGE) + exit(1) - if len(regressions) == 0: - eprint("No regressions found") + pr = PrInfo.load(pr_number) + if pr.contains_directive(REGRESSION_DIRECTIVE): + eprint("PR allows regressions") return - eprint("Found regressions:", json.dumps(regressions, indent=4)) - - if pr_number is not None: - pr = PrInfo.load(pr_number) - if pr.contains_directive(REGRESSION_DIRECTIVE): - eprint("PR allows regressions, returning") - return - + eprint("Regressions were found; benchmark failed") exit(1) @@ -433,8 +387,8 @@ def main(): ctx.emit_workflow_output() case ["locate-baseline", *flags]: locate_baseline(flags) - case ["check-regressions", *args]: - check_iai_regressions(args) + case ["handle-bench-regressions", *args]: + handle_bench_regressions(args) case ["--help" | "-h"]: print(USAGE) exit() From 57786431746f495f4205e26ee1badd142a380fff Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Mon, 2 Jun 2025 20:20:23 +0000 Subject: [PATCH 25/28] libm-test: Fix unintentional skips in `binop_common` `binop_common` emits a `SKIP` that is intended to apply only to `copysign`, but is instead applying to all binary operators. Correct the general case but leave the currently-failing `maximum_num` tests as a FIXME, to be resolved separately in [1]. Also simplify skip logic and NaN checking, and add a few more `copysign` checks. [1]: https://github.com/rust-lang/compiler-builtins/pull/939 --- .../libm-test/src/generate/edge_cases.rs | 1 + .../libm-test/src/precision.rs | 15 +++++++++----- .../libm-test/src/test_traits.rs | 20 +++++++++++++------ .../libm/src/math/copysign.rs | 10 +++++++++- 4 files changed, 34 insertions(+), 12 deletions(-) diff --git a/library/compiler-builtins/libm-test/src/generate/edge_cases.rs b/library/compiler-builtins/libm-test/src/generate/edge_cases.rs index 2fb0746388c15..4e4a782a16988 100644 --- a/library/compiler-builtins/libm-test/src/generate/edge_cases.rs +++ b/library/compiler-builtins/libm-test/src/generate/edge_cases.rs @@ -51,6 +51,7 @@ where // Check some special values that aren't included in the above ranges values.push(Op::FTy::NAN); + values.push(Op::FTy::NEG_NAN); values.extend(Op::FTy::consts().iter()); // Check around the maximum subnormal value diff --git a/library/compiler-builtins/libm-test/src/precision.rs b/library/compiler-builtins/libm-test/src/precision.rs index f6cdd015a593c..32825b15d4761 100644 --- a/library/compiler-builtins/libm-test/src/precision.rs +++ b/library/compiler-builtins/libm-test/src/precision.rs @@ -444,13 +444,18 @@ fn binop_common( expected: F2, ctx: &CheckCtx, ) -> CheckAction { - // MPFR only has one NaN bitpattern; allow the default `.is_nan()` checks to validate. Skip if - // the first input (magnitude source) is NaN and the output is also a NaN, or if the second - // input (sign source) is NaN. - if ctx.basis == CheckBasis::Mpfr + // MPFR only has one NaN bitpattern; skip tests in cases where the first argument would take + // the sign of a NaN second argument. The default NaN checks cover other cases. + if ctx.base_name == BaseName::Copysign && ctx.basis == CheckBasis::Mpfr && input.1.is_nan() { + return SKIP; + } + + // FIXME(#939): this should not be skipped, there is a bug in our implementationi. + if ctx.base_name == BaseName::FmaximumNum + && ctx.basis == CheckBasis::Mpfr && ((input.0.is_nan() && actual.is_nan() && expected.is_nan()) || input.1.is_nan()) { - return SKIP; + return XFAIL_NOCHECK; } /* FIXME(#439): our fmin and fmax do not compare signed zeros */ diff --git a/library/compiler-builtins/libm-test/src/test_traits.rs b/library/compiler-builtins/libm-test/src/test_traits.rs index 2af6af60ba270..278274d917b35 100644 --- a/library/compiler-builtins/libm-test/src/test_traits.rs +++ b/library/compiler-builtins/libm-test/src/test_traits.rs @@ -312,12 +312,9 @@ where let mut inner = || -> TestResult { let mut allowed_ulp = ctx.ulp; - // Forbid overrides if the items came from an explicit list, as long as we are checking - // against either MPFR or the result itself. - let require_biteq = ctx.gen_kind == GeneratorKind::List && ctx.basis != CheckBasis::Musl; - match SpecialCase::check_float(input, actual, expected, ctx) { - _ if require_biteq => (), + // Forbid overrides if the items came from an explicit list + _ if ctx.gen_kind == GeneratorKind::List => (), CheckAction::AssertSuccess => (), CheckAction::AssertFailure(msg) => assert_failure_msg = Some(msg), CheckAction::Custom(res) => return res, @@ -327,9 +324,20 @@ where // Check when both are NaNs if actual.is_nan() && expected.is_nan() { - if require_biteq && ctx.basis == CheckBasis::None { + // Don't assert NaN bitwise equality if: + // + // * Testing against MPFR (there is a single NaN representation) + // * Testing against Musl except for explicit tests (Musl does some NaN quieting) + // + // In these cases, just the check that actual and expected are both NaNs is + // sufficient. + let skip_nan_biteq = ctx.basis == CheckBasis::Mpfr + || (ctx.basis == CheckBasis::Musl && ctx.gen_kind != GeneratorKind::List); + + if !skip_nan_biteq { ensure!(actual.biteq(expected), "mismatched NaN bitpatterns"); } + // By default, NaNs have nothing special to check. return Ok(()); } else if actual.is_nan() || expected.is_nan() { diff --git a/library/compiler-builtins/libm/src/math/copysign.rs b/library/compiler-builtins/libm/src/math/copysign.rs index d2a86e7fd545f..d093d61072732 100644 --- a/library/compiler-builtins/libm/src/math/copysign.rs +++ b/library/compiler-builtins/libm/src/math/copysign.rs @@ -59,9 +59,17 @@ mod tests { // Not required but we expect it assert_biteq!(f(F::NAN, F::NAN), F::NAN); - assert_biteq!(f(F::NEG_NAN, F::NAN), F::NAN); + assert_biteq!(f(F::NAN, F::ONE), F::NAN); + assert_biteq!(f(F::NAN, F::NEG_ONE), F::NEG_NAN); assert_biteq!(f(F::NAN, F::NEG_NAN), F::NEG_NAN); + assert_biteq!(f(F::NEG_NAN, F::NAN), F::NAN); + assert_biteq!(f(F::NEG_NAN, F::ONE), F::NAN); + assert_biteq!(f(F::NEG_NAN, F::NEG_ONE), F::NEG_NAN); assert_biteq!(f(F::NEG_NAN, F::NEG_NAN), F::NEG_NAN); + assert_biteq!(f(F::ONE, F::NAN), F::ONE); + assert_biteq!(f(F::ONE, F::NEG_NAN), F::NEG_ONE); + assert_biteq!(f(F::NEG_ONE, F::NAN), F::ONE); + assert_biteq!(f(F::NEG_ONE, F::NEG_NAN), F::NEG_ONE); } #[test] From 76cb31684c0702b547747bd504ad4b4406ed8b75 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Wed, 28 May 2025 14:45:14 +0000 Subject: [PATCH 26/28] Add an empty rust-version file This will be used by `josh` tooling. --- library/compiler-builtins/rust-version | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 library/compiler-builtins/rust-version diff --git a/library/compiler-builtins/rust-version b/library/compiler-builtins/rust-version new file mode 100644 index 0000000000000..e69de29bb2d1d From 5225908d84f04a581b72aac58ca6386e762b9343 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Sun, 18 May 2025 17:30:58 +0000 Subject: [PATCH 27/28] Add tooling for `josh` syncs Create a crate that handles pulling from and pushing to rust-lang/rust. This can be invoked with the following: $ cargo run -p josh-sync -- rustc-pull $ RUSTC_GIT=/Users/tmgross/Documents/projects/rust cargo run -p josh-sync -- rustc-push --- library/compiler-builtins/Cargo.toml | 1 + .../crates/josh-sync/Cargo.toml | 7 + .../crates/josh-sync/src/main.rs | 45 +++ .../crates/josh-sync/src/sync.rs | 368 ++++++++++++++++++ 4 files changed, 421 insertions(+) create mode 100644 library/compiler-builtins/crates/josh-sync/Cargo.toml create mode 100644 library/compiler-builtins/crates/josh-sync/src/main.rs create mode 100644 library/compiler-builtins/crates/josh-sync/src/sync.rs diff --git a/library/compiler-builtins/Cargo.toml b/library/compiler-builtins/Cargo.toml index bc6b4bd29795e..fb638f2fb379f 100644 --- a/library/compiler-builtins/Cargo.toml +++ b/library/compiler-builtins/Cargo.toml @@ -3,6 +3,7 @@ resolver = "2" members = [ "builtins-test", "compiler-builtins", + "crates/josh-sync", "crates/libm-macros", "crates/musl-math-sys", "crates/panic-handler", diff --git a/library/compiler-builtins/crates/josh-sync/Cargo.toml b/library/compiler-builtins/crates/josh-sync/Cargo.toml new file mode 100644 index 0000000000000..1f3bb376d6d91 --- /dev/null +++ b/library/compiler-builtins/crates/josh-sync/Cargo.toml @@ -0,0 +1,7 @@ +[package] +name = "josh-sync" +edition = "2024" +publish = false + +[dependencies] +directories = "6.0.0" diff --git a/library/compiler-builtins/crates/josh-sync/src/main.rs b/library/compiler-builtins/crates/josh-sync/src/main.rs new file mode 100644 index 0000000000000..7f0b11900337b --- /dev/null +++ b/library/compiler-builtins/crates/josh-sync/src/main.rs @@ -0,0 +1,45 @@ +use std::io::{Read, Write}; +use std::process::exit; +use std::{env, io}; + +use crate::sync::{GitSync, Josh}; + +mod sync; + +const USAGE: &str = r#"Utility for synchroniing compiler-builtins with rust-lang/rust + +Usage: + + josh-sync rustc-pull + + Pull from rust-lang/rust to compiler-builtins. Creates a commit + updating the version file, followed by a merge commit. + + josh-sync rustc-push GITHUB_USERNAME [BRANCH] + + Create a branch off of rust-lang/rust updating compiler-builtins. +"#; + +fn main() { + let sync = GitSync::from_current_dir(); + + // Collect args, then recollect as str refs so we can match on them + let args: Vec<_> = env::args().collect(); + let args: Vec<&str> = args.iter().map(String::as_str).collect(); + + match args.as_slice()[1..] { + ["rustc-pull"] => sync.rustc_pull(None), + ["rustc-push", github_user, branch] => sync.rustc_push(github_user, Some(branch)), + ["rustc-push", github_user] => sync.rustc_push(github_user, None), + ["start-josh"] => { + let _josh = Josh::start(); + println!("press enter to stop"); + io::stdout().flush().unwrap(); + let _ = io::stdin().read(&mut [0u8]).unwrap(); + } + _ => { + println!("{USAGE}"); + exit(1); + } + } +} diff --git a/library/compiler-builtins/crates/josh-sync/src/sync.rs b/library/compiler-builtins/crates/josh-sync/src/sync.rs new file mode 100644 index 0000000000000..3a1fea241e651 --- /dev/null +++ b/library/compiler-builtins/crates/josh-sync/src/sync.rs @@ -0,0 +1,368 @@ +use std::net::{SocketAddr, TcpStream}; +use std::process::{Command, Stdio, exit}; +use std::time::Duration; +use std::{env, fs, process, thread}; + +const JOSH_PORT: u16 = 42042; +const DEFAULT_PR_BRANCH: &str = "update-builtins"; + +pub struct GitSync { + upstream_repo: String, + upstream_ref: String, + upstream_url: String, + josh_filter: String, + josh_url_base: String, +} + +/// This code was adapted from the miri repository, via the rustc-dev-guide +/// () +impl GitSync { + pub fn from_current_dir() -> Self { + let upstream_repo = + env::var("UPSTREAM_ORG").unwrap_or_else(|_| "rust-lang".to_owned()) + "/rust"; + + Self { + upstream_url: format!("https://github.com/{upstream_repo}"), + upstream_repo, + upstream_ref: env::var("UPSTREAM_REF").unwrap_or_else(|_| "HEAD".to_owned()), + josh_filter: ":/library/compiler-builtins".to_owned(), + josh_url_base: format!("http://localhost:{JOSH_PORT}"), + } + } + + /// Pull from rust-lang/rust to compiler-builtins. + pub fn rustc_pull(&self, commit: Option) { + let Self { + upstream_ref, + upstream_url, + upstream_repo, + .. + } = self; + + let new_upstream_base = commit.unwrap_or_else(|| { + let out = check_output(["git", "ls-remote", upstream_url, upstream_ref]); + out.split_whitespace() + .next() + .unwrap_or_else(|| panic!("could not split output: '{out}'")) + .to_owned() + }); + + ensure_clean(); + + // Make sure josh is running. + let _josh = Josh::start(); + let josh_url_filtered = self.josh_url( + &self.upstream_repo, + Some(&new_upstream_base), + Some(&self.josh_filter), + ); + + let previous_upstream_base = fs::read_to_string("rust-version") + .expect("failed to read `rust-version`") + .trim() + .to_string(); + assert_ne!(previous_upstream_base, new_upstream_base, "nothing to pull"); + + let orig_head = check_output(["git", "rev-parse", "HEAD"]); + println!("original upstream base: {previous_upstream_base}"); + println!("new upstream base: {new_upstream_base}"); + println!("original HEAD: {orig_head}"); + + // Fetch the latest upstream HEAD so we can get a summary. Use the Josh URL for caching. + run([ + "git", + "fetch", + &self.josh_url(&self.upstream_repo, Some(&new_upstream_base), Some(":/")), + &new_upstream_base, + "--depth=1", + ]); + let new_summary = check_output(["git", "log", "-1", "--format=%h %s", &new_upstream_base]); + + // Update rust-version file. As a separate commit, since making it part of + // the merge has confused the heck out of josh in the past. + // We pass `--no-verify` to avoid running git hooks. + // We do this before the merge so that if there are merge conflicts, we have + // the right rust-version file while resolving them. + fs::write("rust-version", format!("{new_upstream_base}\n")) + .expect("failed to write rust-version"); + + let prep_message = format!( + "Update the upstream Rust version\n\n\ + To prepare for merging from {upstream_repo}, set the version file to:\n\n \ + {new_summary}\n\ + ", + ); + run([ + "git", + "commit", + "rust-version", + "--no-verify", + "-m", + &prep_message, + ]); + + // Fetch given rustc commit. + run(["git", "fetch", &josh_url_filtered]); + let incoming_ref = check_output(["git", "rev-parse", "FETCH_HEAD"]); + println!("incoming ref: {incoming_ref}"); + + let merge_message = format!( + "Merge ref {upstream_head_short}{filter} from {upstream_url}\n\n\ + Pull recent changes from {upstream_repo} via Josh.\n\n\ + Upstream ref: {new_upstream_base}\n\ + Filtered ref: {incoming_ref}\n\ + ", + upstream_head_short = &new_upstream_base[..12], + filter = self.josh_filter + ); + + // This should not add any new root commits. So count those before and after merging. + let num_roots = || -> u32 { + let out = check_output(["git", "rev-list", "HEAD", "--max-parents=0", "--count"]); + out.trim() + .parse::() + .unwrap_or_else(|e| panic!("failed to parse `{out}`: {e}")) + }; + let num_roots_before = num_roots(); + + let pre_merge_sha = check_output(["git", "rev-parse", "HEAD"]); + println!("pre-merge HEAD: {pre_merge_sha}"); + + // Merge the fetched commit. + run([ + "git", + "merge", + "FETCH_HEAD", + "--no-verify", + "--no-ff", + "-m", + &merge_message, + ]); + + let current_sha = check_output(["git", "rev-parse", "HEAD"]); + if current_sha == pre_merge_sha { + run(["git", "reset", "--hard", &orig_head]); + eprintln!( + "No merge was performed, no changes to pull were found. \ + Rolled back the preparation commit." + ); + exit(1); + } + + // Check that the number of roots did not increase. + assert_eq!( + num_roots(), + num_roots_before, + "Josh created a new root commit. This is probably not the history you want." + ); + } + + /// Construct an update to rust-lang/rust from compiler-builtins. + pub fn rustc_push(&self, github_user: &str, branch: Option<&str>) { + let Self { + josh_filter, + upstream_url, + .. + } = self; + + let branch = branch.unwrap_or(DEFAULT_PR_BRANCH); + let josh_url = self.josh_url(&format!("{github_user}/rust"), None, Some(josh_filter)); + let user_upstream_url = format!("git@github.com:{github_user}/rust.git"); + + let Ok(rustc_git) = env::var("RUSTC_GIT") else { + panic!("the RUSTC_GIT environment variable must be set to a rust-lang/rust checkout") + }; + + ensure_clean(); + let base = fs::read_to_string("rust-version") + .expect("failed to read `rust-version`") + .trim() + .to_string(); + + // Make sure josh is running. + let _josh = Josh::start(); + + // Prepare the branch. Pushing works much better if we use as base exactly + // the commit that we pulled from last time, so we use the `rust-version` + // file to find out which commit that would be. + println!("Preparing {github_user}/rust (base: {base})..."); + + if Command::new("git") + .args(["-C", &rustc_git, "fetch", &user_upstream_url, branch]) + .output() // capture output + .expect("could not run fetch") + .status + .success() + { + panic!( + "The branch '{branch}' seems to already exist in '{user_upstream_url}'. \ + Please delete it and try again." + ); + } + + run(["git", "-C", &rustc_git, "fetch", upstream_url, &base]); + + run_cfg("git", |c| { + c.args([ + "-C", + &rustc_git, + "push", + &user_upstream_url, + &format!("{base}:refs/heads/{branch}"), + ]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) // silence the "create GitHub PR" message + }); + println!("pushed PR branch"); + + // Do the actual push. + println!("Pushing changes..."); + run(["git", "push", &josh_url, &format!("HEAD:{branch}")]); + println!(); + + // Do a round-trip check to make sure the push worked as expected. + run(["git", "fetch", &josh_url, branch]); + + let head = check_output(["git", "rev-parse", "HEAD"]); + let fetch_head = check_output(["git", "rev-parse", "FETCH_HEAD"]); + assert_eq!( + head, fetch_head, + "Josh created a non-roundtrip push! Do NOT merge this into rustc!\n\ + Expected {head}, got {fetch_head}." + ); + println!( + "Confirmed that the push round-trips back to compiler-builtins properly. Please \ + create a rustc PR:" + ); + // Open PR with `subtree update` title to silence the `no-merges` triagebot check + println!( + " {upstream_url}/compare/{github_user}:{branch}?\ + quick_pull=1&title=compiler-builtins+subtree+update&body=r?+@ghost" + ); + } + + /// Construct a url to the local Josh server with (optionally) + fn josh_url(&self, repo: &str, rev: Option<&str>, filter: Option<&str>) -> String { + format!( + "{base}/{repo}.git{at}{rev}{filter}{filt_git}", + base = self.josh_url_base, + at = if rev.is_some() { "@" } else { "" }, + rev = rev.unwrap_or_default(), + filter = filter.unwrap_or_default(), + filt_git = if filter.is_some() { ".git" } else { "" } + ) + } +} + +/// Fail if there are files that need to be checked in. +fn ensure_clean() { + let read = check_output(["git", "status", "--untracked-files=no", "--porcelain"]); + assert!( + read.is_empty(), + "working directory must be clean before performing rustc pull" + ); +} + +/* Helpers for running commands with logged invocations */ + +/// Run a command from an array, passing its output through. +fn run<'a, Args: AsRef<[&'a str]>>(l: Args) { + let l = l.as_ref(); + run_cfg(l[0], |c| c.args(&l[1..])); +} + +/// Run a command from an array, collecting its output. +fn check_output<'a, Args: AsRef<[&'a str]>>(l: Args) -> String { + let l = l.as_ref(); + check_output_cfg(l[0], |c| c.args(&l[1..])) +} + +/// [`run`] with configuration. +fn run_cfg(prog: &str, f: impl FnOnce(&mut Command) -> &mut Command) { + // self.read(l.as_ref()); + check_output_cfg(prog, |c| f(c.stdout(Stdio::inherit()))); +} + +/// [`read`] with configuration. All shell helpers print the command and pass stderr. +fn check_output_cfg(prog: &str, f: impl FnOnce(&mut Command) -> &mut Command) -> String { + let mut cmd = Command::new(prog); + cmd.stderr(Stdio::inherit()); + f(&mut cmd); + eprintln!("+ {cmd:?}"); + let out = cmd.output().expect("command failed"); + assert!(out.status.success()); + String::from_utf8(out.stdout.trim_ascii().to_vec()).expect("non-UTF8 output") +} + +/// Create a wrapper that stops Josh on drop. +pub struct Josh(process::Child); + +impl Josh { + pub fn start() -> Self { + // Determine cache directory. + let user_dirs = + directories::ProjectDirs::from("org", "rust-lang", "rustc-compiler-builtins-josh") + .unwrap(); + let local_dir = user_dirs.cache_dir().to_owned(); + + // Start josh, silencing its output. + #[expect(clippy::zombie_processes, reason = "clippy can't handle the loop")] + let josh = process::Command::new("josh-proxy") + .arg("--local") + .arg(local_dir) + .args([ + "--remote=https://github.com", + &format!("--port={JOSH_PORT}"), + "--no-background", + ]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .expect("failed to start josh-proxy, make sure it is installed"); + + // Wait until the port is open. We try every 10ms until 1s passed. + for _ in 0..100 { + // This will generally fail immediately when the port is still closed. + let addr = SocketAddr::from(([127, 0, 0, 1], JOSH_PORT)); + let josh_ready = TcpStream::connect_timeout(&addr, Duration::from_millis(1)); + + if josh_ready.is_ok() { + println!("josh up and running"); + return Josh(josh); + } + + // Not ready yet. + thread::sleep(Duration::from_millis(10)); + } + panic!("Even after waiting for 1s, josh-proxy is still not available.") + } +} + +impl Drop for Josh { + fn drop(&mut self) { + if cfg!(unix) { + // Try to gracefully shut it down. + Command::new("kill") + .args(["-s", "INT", &self.0.id().to_string()]) + .output() + .expect("failed to SIGINT josh-proxy"); + // Sadly there is no "wait with timeout"... so we just give it some time to finish. + thread::sleep(Duration::from_millis(100)); + // Now hopefully it is gone. + if self + .0 + .try_wait() + .expect("failed to wait for josh-proxy") + .is_some() + { + return; + } + } + // If that didn't work (or we're not on Unix), kill it hard. + eprintln!( + "I have to kill josh-proxy the hard way, let's hope this does not \ + break anything." + ); + self.0.kill().expect("failed to SIGKILL josh-proxy"); + } +} From dd850960efec7da0132603772c806d260c98d532 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Wed, 4 Jun 2025 07:58:25 +0000 Subject: [PATCH 28/28] Update the upstream Rust version To prepare for merging from rust-lang/rust, set the version file to: 792fc2b033 Auto merge of #141984 - matthiaskrgr:rollup-wy6j9ca, r=matthiaskrgr --- library/compiler-builtins/rust-version | 1 + 1 file changed, 1 insertion(+) diff --git a/library/compiler-builtins/rust-version b/library/compiler-builtins/rust-version index e69de29bb2d1d..2bc38eebfc299 100644 --- a/library/compiler-builtins/rust-version +++ b/library/compiler-builtins/rust-version @@ -0,0 +1 @@ +792fc2b033aea7ea7b766e38bdc40f7d6bdce8c3