Skip to content

Commit f4ec382

Browse files
committed
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: #919 [1]: https://github.com/llvm/llvm-project/blob/0cf3c437c18ed27d9663d87804a9a15ff6874af2/compiler-rt/lib/builtins/fp_compare_impl.inc#L11-L27
1 parent 157a0b7 commit f4ec382

File tree

4 files changed

+28
-17
lines changed

4 files changed

+28
-17
lines changed

builtins-test/benches/float_cmp.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#![cfg_attr(f128_enabled, feature(f128))]
22

33
use builtins_test::float_bench;
4-
use compiler_builtins::float::cmp;
4+
use compiler_builtins::float::cmp::{self, CmpResult};
55
use criterion::{Criterion, criterion_main};
66

77
/// `gt` symbols are allowed to return differing results, they just get compared
@@ -14,7 +14,7 @@ fn gt_res_eq(a: i32, b: i32) -> bool {
1414

1515
float_bench! {
1616
name: cmp_f32_gt,
17-
sig: (a: f32, b: f32) -> i32,
17+
sig: (a: f32, b: f32) -> CmpResult,
1818
crate_fn: cmp::__gtsf2,
1919
sys_fn: __gtsf2,
2020
sys_available: all(),
@@ -53,7 +53,7 @@ float_bench! {
5353

5454
float_bench! {
5555
name: cmp_f32_unord,
56-
sig: (a: f32, b: f32) -> i32,
56+
sig: (a: f32, b: f32) -> CmpResult,
5757
crate_fn: cmp::__unordsf2,
5858
sys_fn: __unordsf2,
5959
sys_available: all(),
@@ -91,7 +91,7 @@ float_bench! {
9191

9292
float_bench! {
9393
name: cmp_f64_gt,
94-
sig: (a: f64, b: f64) -> i32,
94+
sig: (a: f64, b: f64) -> CmpResult,
9595
crate_fn: cmp::__gtdf2,
9696
sys_fn: __gtdf2,
9797
sys_available: all(),
@@ -130,7 +130,7 @@ float_bench! {
130130

131131
float_bench! {
132132
name: cmp_f64_unord,
133-
sig: (a: f64, b: f64) -> i32,
133+
sig: (a: f64, b: f64) -> CmpResult,
134134
crate_fn: cmp::__unorddf2,
135135
sys_fn: __unorddf2,
136136
sys_available: all(),
@@ -168,7 +168,7 @@ float_bench! {
168168

169169
float_bench! {
170170
name: cmp_f128_gt,
171-
sig: (a: f128, b: f128) -> i32,
171+
sig: (a: f128, b: f128) -> CmpResult,
172172
crate_fn: cmp::__gttf2,
173173
crate_fn_ppc: cmp::__gtkf2,
174174
sys_fn: __gttf2,
@@ -180,7 +180,7 @@ float_bench! {
180180

181181
float_bench! {
182182
name: cmp_f128_unord,
183-
sig: (a: f128, b: f128) -> i32,
183+
sig: (a: f128, b: f128) -> CmpResult,
184184
crate_fn: cmp::__unordtf2,
185185
crate_fn_ppc: cmp::__unordkf2,
186186
sys_fn: __unordtf2,

builtins-test/src/bench.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -358,8 +358,8 @@ impl_testio!(float f16);
358358
impl_testio!(float f32, f64);
359359
#[cfg(f128_enabled)]
360360
impl_testio!(float f128);
361-
impl_testio!(int i16, i32, i64, i128);
362-
impl_testio!(int u16, u32, u64, u128);
361+
impl_testio!(int i16, i32, i64, i128, isize);
362+
impl_testio!(int u16, u32, u64, u128, usize);
363363
impl_testio!((float, int)(f32, i32));
364364
impl_testio!((float, int)(f64, i32));
365365
#[cfg(f128_enabled)]

compiler-builtins/src/float/cmp.rs

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,23 @@
22

33
use crate::float::Float;
44
use crate::int::MinInt;
5-
6-
// https://github.com/llvm/llvm-project/blob/1e6ba3cd2fe96be00b6ed6ba28b3d9f9271d784d/compiler-rt/lib/builtins/fp_compare_impl.inc#L22
7-
#[cfg(target_arch = "avr")]
8-
pub type CmpResult = i8;
9-
10-
// https://github.com/llvm/llvm-project/blob/1e6ba3cd2fe96be00b6ed6ba28b3d9f9271d784d/compiler-rt/lib/builtins/fp_compare_impl.inc#L25
11-
#[cfg(not(target_arch = "avr"))]
12-
pub type CmpResult = i32;
5+
use crate::support::cfg_if;
6+
7+
// Taken from LLVM config:
8+
// https://github.com/llvm/llvm-project/blob/0cf3c437c18ed27d9663d87804a9a15ff6874af2/compiler-rt/lib/builtins/fp_compare_impl.inc#L11-L27
9+
cfg_if! {
10+
if #[cfg(any(target_arch = "aarch64", target_arch = "arm64ec"))] {
11+
// Aarch64 uses `int` rather than a pointer-sized value.
12+
pub type CmpResult = i32;
13+
} else if #[cfg(target_arch = "avr")] {
14+
// AVR uses a single byte.
15+
pub type CmpResult = i8;
16+
} else {
17+
// In compiler-rt, LLP64 ABIs use `long long` and everything else uses `long`. In effect,
18+
// this means the return value is always pointer-sized.
19+
pub type CmpResult = isize;
20+
}
21+
}
1322

1423
#[derive(Clone, Copy)]
1524
enum Result {

libm/src/math/support/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ mod int_traits;
1111

1212
#[allow(unused_imports)]
1313
pub use big::{i256, u256};
14+
#[allow(unused_imports)]
15+
pub(crate) use cfg_if;
1416
pub use env::{FpResult, Round, Status};
1517
#[allow(unused_imports)]
1618
pub use float_traits::{DFloat, Float, HFloat, IntTy};

0 commit comments

Comments
 (0)