Skip to content

Update CmpResult to use a pointer-sized return type #920

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 27 additions & 16 deletions builtins-test/benches/float_cmp.rs
Original file line number Diff line number Diff line change
@@ -1,27 +1,38 @@
#![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::<CmpResult>() == 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)
}

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}",
Expand All @@ -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",
Expand All @@ -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}",
Expand All @@ -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",
Expand All @@ -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}",
Expand All @@ -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",
Expand All @@ -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}",
Expand All @@ -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",
Expand All @@ -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,
Expand All @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions builtins-test/src/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
25 changes: 17 additions & 8 deletions compiler-builtins/src/float/cmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions libm/src/math/support/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
Loading