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]: 0cf3c437c1/compiler-rt/lib/builtins/fp_compare_impl.inc (L11-L27)
This commit is contained in:
Trevor Gross
2025-05-23 17:26:39 +00:00
committed by Trevor Gross
parent db21837095
commit 4c264c96ae
4 changed files with 47 additions and 25 deletions

View File

@@ -1,12 +1,23 @@
#![cfg_attr(f128_enabled, feature(f128))] #![cfg_attr(f128_enabled, feature(f128))]
use builtins_test::float_bench; use builtins_test::float_bench;
use compiler_builtins::float::cmp; use compiler_builtins::float::cmp::{self, CmpResult};
use criterion::{Criterion, criterion_main}; use criterion::{Criterion, criterion_main};
/// `gt` symbols are allowed to return differing results, they just get compared /// `gt` symbols are allowed to return differing results, they just get compared
/// to 0. /// 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 a_lt_0 = a <= 0;
let b_lt_0 = b <= 0; let b_lt_0 = b <= 0;
(a_lt_0 && b_lt_0) || (!a_lt_0 && !b_lt_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! { float_bench! {
name: cmp_f32_gt, name: cmp_f32_gt,
sig: (a: f32, b: f32) -> i32, sig: (a: f32, b: f32) -> CmpResult,
crate_fn: cmp::__gtsf2, crate_fn: cmp::__gtsf2,
sys_fn: __gtsf2, sys_fn: __gtsf2,
sys_available: all(), sys_available: all(),
output_eq: gt_res_eq, output_eq: gt_res_eq,
asm: [ asm: [
#[cfg(target_arch = "x86_64")] { #[cfg(target_arch = "x86_64")] {
let ret: i32; let ret: CmpResult;
asm!( asm!(
"xor {ret:e}, {ret:e}", "xor {ret:e}, {ret:e}",
"ucomiss {a}, {b}", "ucomiss {a}, {b}",
@@ -36,7 +47,7 @@ float_bench! {
}; };
#[cfg(target_arch = "aarch64")] { #[cfg(target_arch = "aarch64")] {
let ret: i32; let ret: CmpResult;
asm!( asm!(
"fcmp {a:s}, {b:s}", "fcmp {a:s}, {b:s}",
"cset {ret:w}, gt", "cset {ret:w}, gt",
@@ -53,13 +64,13 @@ float_bench! {
float_bench! { float_bench! {
name: cmp_f32_unord, name: cmp_f32_unord,
sig: (a: f32, b: f32) -> i32, sig: (a: f32, b: f32) -> CmpResult,
crate_fn: cmp::__unordsf2, crate_fn: cmp::__unordsf2,
sys_fn: __unordsf2, sys_fn: __unordsf2,
sys_available: all(), sys_available: all(),
asm: [ asm: [
#[cfg(target_arch = "x86_64")] { #[cfg(target_arch = "x86_64")] {
let ret: i32; let ret: CmpResult;
asm!( asm!(
"xor {ret:e}, {ret:e}", "xor {ret:e}, {ret:e}",
"ucomiss {a}, {b}", "ucomiss {a}, {b}",
@@ -74,7 +85,7 @@ float_bench! {
}; };
#[cfg(target_arch = "aarch64")] { #[cfg(target_arch = "aarch64")] {
let ret: i32; let ret: CmpResult;
asm!( asm!(
"fcmp {a:s}, {b:s}", "fcmp {a:s}, {b:s}",
"cset {ret:w}, vs", "cset {ret:w}, vs",
@@ -91,14 +102,14 @@ float_bench! {
float_bench! { float_bench! {
name: cmp_f64_gt, name: cmp_f64_gt,
sig: (a: f64, b: f64) -> i32, sig: (a: f64, b: f64) -> CmpResult,
crate_fn: cmp::__gtdf2, crate_fn: cmp::__gtdf2,
sys_fn: __gtdf2, sys_fn: __gtdf2,
sys_available: all(), sys_available: all(),
output_eq: gt_res_eq, output_eq: gt_res_eq,
asm: [ asm: [
#[cfg(target_arch = "x86_64")] { #[cfg(target_arch = "x86_64")] {
let ret: i32; let ret: CmpResult;
asm!( asm!(
"xor {ret:e}, {ret:e}", "xor {ret:e}, {ret:e}",
"ucomisd {a}, {b}", "ucomisd {a}, {b}",
@@ -113,7 +124,7 @@ float_bench! {
}; };
#[cfg(target_arch = "aarch64")] { #[cfg(target_arch = "aarch64")] {
let ret: i32; let ret: CmpResult;
asm!( asm!(
"fcmp {a:d}, {b:d}", "fcmp {a:d}, {b:d}",
"cset {ret:w}, gt", "cset {ret:w}, gt",
@@ -130,13 +141,13 @@ float_bench! {
float_bench! { float_bench! {
name: cmp_f64_unord, name: cmp_f64_unord,
sig: (a: f64, b: f64) -> i32, sig: (a: f64, b: f64) -> CmpResult,
crate_fn: cmp::__unorddf2, crate_fn: cmp::__unorddf2,
sys_fn: __unorddf2, sys_fn: __unorddf2,
sys_available: all(), sys_available: all(),
asm: [ asm: [
#[cfg(target_arch = "x86_64")] { #[cfg(target_arch = "x86_64")] {
let ret: i32; let ret: CmpResult;
asm!( asm!(
"xor {ret:e}, {ret:e}", "xor {ret:e}, {ret:e}",
"ucomisd {a}, {b}", "ucomisd {a}, {b}",
@@ -151,7 +162,7 @@ float_bench! {
}; };
#[cfg(target_arch = "aarch64")] { #[cfg(target_arch = "aarch64")] {
let ret: i32; let ret: CmpResult;
asm!( asm!(
"fcmp {a:d}, {b:d}", "fcmp {a:d}, {b:d}",
"cset {ret:w}, vs", "cset {ret:w}, vs",
@@ -168,7 +179,7 @@ float_bench! {
float_bench! { float_bench! {
name: cmp_f128_gt, name: cmp_f128_gt,
sig: (a: f128, b: f128) -> i32, sig: (a: f128, b: f128) -> CmpResult,
crate_fn: cmp::__gttf2, crate_fn: cmp::__gttf2,
crate_fn_ppc: cmp::__gtkf2, crate_fn_ppc: cmp::__gtkf2,
sys_fn: __gttf2, sys_fn: __gttf2,
@@ -180,7 +191,7 @@ float_bench! {
float_bench! { float_bench! {
name: cmp_f128_unord, name: cmp_f128_unord,
sig: (a: f128, b: f128) -> i32, sig: (a: f128, b: f128) -> CmpResult,
crate_fn: cmp::__unordtf2, crate_fn: cmp::__unordtf2,
crate_fn_ppc: cmp::__unordkf2, crate_fn_ppc: cmp::__unordkf2,
sys_fn: __unordtf2, sys_fn: __unordtf2,

View File

@@ -358,8 +358,8 @@ impl_testio!(float f16);
impl_testio!(float f32, f64); impl_testio!(float f32, f64);
#[cfg(f128_enabled)] #[cfg(f128_enabled)]
impl_testio!(float f128); impl_testio!(float f128);
impl_testio!(int i16, i32, i64, i128); impl_testio!(int i8, i16, i32, i64, i128, isize);
impl_testio!(int u16, u32, u64, u128); impl_testio!(int u8, u16, u32, u64, u128, usize);
impl_testio!((float, int)(f32, i32)); impl_testio!((float, int)(f32, i32));
impl_testio!((float, int)(f64, i32)); impl_testio!((float, int)(f64, i32));
#[cfg(f128_enabled)] #[cfg(f128_enabled)]

View File

@@ -2,14 +2,23 @@
use crate::float::Float; use crate::float::Float;
use crate::int::MinInt; use crate::int::MinInt;
use crate::support::cfg_if;
// https://github.com/llvm/llvm-project/blob/1e6ba3cd2fe96be00b6ed6ba28b3d9f9271d784d/compiler-rt/lib/builtins/fp_compare_impl.inc#L22 // Taken from LLVM config:
#[cfg(target_arch = "avr")] // https://github.com/llvm/llvm-project/blob/0cf3c437c18ed27d9663d87804a9a15ff6874af2/compiler-rt/lib/builtins/fp_compare_impl.inc#L11-L27
pub type CmpResult = i8; cfg_if! {
if #[cfg(any(target_arch = "aarch64", target_arch = "arm64ec"))] {
// https://github.com/llvm/llvm-project/blob/1e6ba3cd2fe96be00b6ed6ba28b3d9f9271d784d/compiler-rt/lib/builtins/fp_compare_impl.inc#L25 // Aarch64 uses `int` rather than a pointer-sized value.
#[cfg(not(target_arch = "avr"))] pub type CmpResult = i32;
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)] #[derive(Clone, Copy)]
enum Result { enum Result {

View File

@@ -11,6 +11,8 @@ mod int_traits;
#[allow(unused_imports)] #[allow(unused_imports)]
pub use big::{i256, u256}; pub use big::{i256, u256};
#[allow(unused_imports)]
pub(crate) use cfg_if;
pub use env::{FpResult, Round, Status}; pub use env::{FpResult, Round, Status};
#[allow(unused_imports)] #[allow(unused_imports)]
pub use float_traits::{DFloat, Float, HFloat, IntTy}; pub use float_traits::{DFloat, Float, HFloat, IntTy};