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

Author: tgross35

builtins-test/benches/float_cmp.rs

@@ -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}",
@@ -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,

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)]

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 {

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};