Skip to content

Commit 2459c35

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

File tree

4 files changed

+37
-26
lines changed

4 files changed

+37
-26
lines changed

builtins-test/benches/float_cmp.rs

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,27 @@
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
88
/// to 0.
9-
fn gt_res_eq(a: i32, b: i32) -> bool {
9+
fn gt_res_eq(a: CmpResult, b: CmpResult) -> bool {
1010
let a_lt_0 = a <= 0;
1111
let b_lt_0 = b <= 0;
1212
(a_lt_0 && b_lt_0) || (!a_lt_0 && !b_lt_0)
1313
}
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(),
2121
output_eq: gt_res_eq,
2222
asm: [
2323
#[cfg(target_arch = "x86_64")] {
24-
let ret: i32;
24+
let ret: CmpResult;
2525
asm!(
2626
"xor {ret:e}, {ret:e}",
2727
"ucomiss {a}, {b}",
@@ -36,7 +36,7 @@ float_bench! {
3636
};
3737

3838
#[cfg(target_arch = "aarch64")] {
39-
let ret: i32;
39+
let ret: CmpResult;
4040
asm!(
4141
"fcmp {a:s}, {b:s}",
4242
"cset {ret:w}, gt",
@@ -53,13 +53,13 @@ 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(),
6060
asm: [
6161
#[cfg(target_arch = "x86_64")] {
62-
let ret: i32;
62+
let ret: CmpResult;
6363
asm!(
6464
"xor {ret:e}, {ret:e}",
6565
"ucomiss {a}, {b}",
@@ -74,7 +74,7 @@ float_bench! {
7474
};
7575

7676
#[cfg(target_arch = "aarch64")] {
77-
let ret: i32;
77+
let ret: CmpResult;
7878
asm!(
7979
"fcmp {a:s}, {b:s}",
8080
"cset {ret:w}, vs",
@@ -91,14 +91,14 @@ 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(),
9898
output_eq: gt_res_eq,
9999
asm: [
100100
#[cfg(target_arch = "x86_64")] {
101-
let ret: i32;
101+
let ret: CmpResult;
102102
asm!(
103103
"xor {ret:e}, {ret:e}",
104104
"ucomisd {a}, {b}",
@@ -113,7 +113,7 @@ float_bench! {
113113
};
114114

115115
#[cfg(target_arch = "aarch64")] {
116-
let ret: i32;
116+
let ret: CmpResult;
117117
asm!(
118118
"fcmp {a:d}, {b:d}",
119119
"cset {ret:w}, gt",
@@ -130,13 +130,13 @@ 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(),
137137
asm: [
138138
#[cfg(target_arch = "x86_64")] {
139-
let ret: i32;
139+
let ret: CmpResult;
140140
asm!(
141141
"xor {ret:e}, {ret:e}",
142142
"ucomisd {a}, {b}",
@@ -151,7 +151,7 @@ float_bench! {
151151
};
152152

153153
#[cfg(target_arch = "aarch64")] {
154-
let ret: i32;
154+
let ret: CmpResult;
155155
asm!(
156156
"fcmp {a:d}, {b:d}",
157157
"cset {ret:w}, vs",
@@ -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)