[compiler-rt] 777ca51 - [builtins] Fix ABI-incompatibility with GCC for floating-point compare
Alex Richardson via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 28 04:23:33 PDT 2021
Author: Alex Richardson
Date: 2021-04-28T12:19:19+01:00
New Revision: 777ca513c864fa02292ac255185ea1b0589fc288
URL: https://github.com/llvm/llvm-project/commit/777ca513c864fa02292ac255185ea1b0589fc288
DIFF: https://github.com/llvm/llvm-project/commit/777ca513c864fa02292ac255185ea1b0589fc288.diff
LOG: [builtins] Fix ABI-incompatibility with GCC for floating-point compare
While implementing support for the float128 routines on x86_64, I noticed
that __builtin_isinf() was returning true for 128-bit floating point
values that are not infinite when compiling with GCC and using the
compiler-rt implementation of the soft-float comparison functions.
After stepping through the assembly, I discovered that this was caused by
GCC assuming a sign-extended 64-bit -1 result, but our implementation
returns an enum (which then has zeroes in the upper bits) and therefore
causes the comparison with -1 to fail.
Fix this by using a CMP_RESULT typedef and add a static_assert that it
matches the GCC soft-float comparison return type when compiling with GCC
(GCC has a __libgcc_cmp_return__ mode that can be used for this purpose).
Also move the 3 copies of the same code to a shared .inc file.
Reviewed By: compnerd
Differential Revision: https://reviews.llvm.org/D98205
Added:
compiler-rt/lib/builtins/fp_compare_impl.inc
Modified:
compiler-rt/lib/builtins/comparedf2.c
compiler-rt/lib/builtins/comparesf2.c
compiler-rt/lib/builtins/comparetf2.c
Removed:
################################################################################
diff --git a/compiler-rt/lib/builtins/comparedf2.c b/compiler-rt/lib/builtins/comparedf2.c
index 58290d87de655..e1fc12c54d48a 100644
--- a/compiler-rt/lib/builtins/comparedf2.c
+++ b/compiler-rt/lib/builtins/comparedf2.c
@@ -39,47 +39,9 @@
#define DOUBLE_PRECISION
#include "fp_lib.h"
-enum LE_RESULT { LE_LESS = -1, LE_EQUAL = 0, LE_GREATER = 1, LE_UNORDERED = 1 };
+#include "fp_compare_impl.inc"
-COMPILER_RT_ABI enum LE_RESULT __ledf2(fp_t a, fp_t b) {
-
- const srep_t aInt = toRep(a);
- const srep_t bInt = toRep(b);
- const rep_t aAbs = aInt & absMask;
- const rep_t bAbs = bInt & absMask;
-
- // If either a or b is NaN, they are unordered.
- if (aAbs > infRep || bAbs > infRep)
- return LE_UNORDERED;
-
- // If a and b are both zeros, they are equal.
- if ((aAbs | bAbs) == 0)
- return LE_EQUAL;
-
- // If at least one of a and b is positive, we get the same result comparing
- // a and b as signed integers as we would with a floating-point compare.
- if ((aInt & bInt) >= 0) {
- if (aInt < bInt)
- return LE_LESS;
- else if (aInt == bInt)
- return LE_EQUAL;
- else
- return LE_GREATER;
- }
-
- // Otherwise, both are negative, so we need to flip the sense of the
- // comparison to get the correct result. (This assumes a twos- or ones-
- // complement integer representation; if integers are represented in a
- // sign-magnitude representation, then this flip is incorrect).
- else {
- if (aInt > bInt)
- return LE_LESS;
- else if (aInt == bInt)
- return LE_EQUAL;
- else
- return LE_GREATER;
- }
-}
+COMPILER_RT_ABI CMP_RESULT __ledf2(fp_t a, fp_t b) { return __leXf2__(a, b); }
#if defined(__ELF__)
// Alias for libgcc compatibility
@@ -89,48 +51,12 @@ COMPILER_RT_ALIAS(__ledf2, __eqdf2)
COMPILER_RT_ALIAS(__ledf2, __ltdf2)
COMPILER_RT_ALIAS(__ledf2, __nedf2)
-enum GE_RESULT {
- GE_LESS = -1,
- GE_EQUAL = 0,
- GE_GREATER = 1,
- GE_UNORDERED = -1 // Note:
diff erent from LE_UNORDERED
-};
-
-COMPILER_RT_ABI enum GE_RESULT __gedf2(fp_t a, fp_t b) {
-
- const srep_t aInt = toRep(a);
- const srep_t bInt = toRep(b);
- const rep_t aAbs = aInt & absMask;
- const rep_t bAbs = bInt & absMask;
-
- if (aAbs > infRep || bAbs > infRep)
- return GE_UNORDERED;
- if ((aAbs | bAbs) == 0)
- return GE_EQUAL;
- if ((aInt & bInt) >= 0) {
- if (aInt < bInt)
- return GE_LESS;
- else if (aInt == bInt)
- return GE_EQUAL;
- else
- return GE_GREATER;
- } else {
- if (aInt > bInt)
- return GE_LESS;
- else if (aInt == bInt)
- return GE_EQUAL;
- else
- return GE_GREATER;
- }
-}
+COMPILER_RT_ABI CMP_RESULT __gedf2(fp_t a, fp_t b) { return __geXf2__(a, b); }
COMPILER_RT_ALIAS(__gedf2, __gtdf2)
-COMPILER_RT_ABI int
-__unorddf2(fp_t a, fp_t b) {
- const rep_t aAbs = toRep(a) & absMask;
- const rep_t bAbs = toRep(b) & absMask;
- return aAbs > infRep || bAbs > infRep;
+COMPILER_RT_ABI CMP_RESULT __unorddf2(fp_t a, fp_t b) {
+ return __unordXf2__(a, b);
}
#if defined(__ARM_EABI__)
diff --git a/compiler-rt/lib/builtins/comparesf2.c b/compiler-rt/lib/builtins/comparesf2.c
index 1cb99e468c18a..b8a955448f574 100644
--- a/compiler-rt/lib/builtins/comparesf2.c
+++ b/compiler-rt/lib/builtins/comparesf2.c
@@ -39,47 +39,9 @@
#define SINGLE_PRECISION
#include "fp_lib.h"
-enum LE_RESULT { LE_LESS = -1, LE_EQUAL = 0, LE_GREATER = 1, LE_UNORDERED = 1 };
+#include "fp_compare_impl.inc"
-COMPILER_RT_ABI enum LE_RESULT __lesf2(fp_t a, fp_t b) {
-
- const srep_t aInt = toRep(a);
- const srep_t bInt = toRep(b);
- const rep_t aAbs = aInt & absMask;
- const rep_t bAbs = bInt & absMask;
-
- // If either a or b is NaN, they are unordered.
- if (aAbs > infRep || bAbs > infRep)
- return LE_UNORDERED;
-
- // If a and b are both zeros, they are equal.
- if ((aAbs | bAbs) == 0)
- return LE_EQUAL;
-
- // If at least one of a and b is positive, we get the same result comparing
- // a and b as signed integers as we would with a fp_ting-point compare.
- if ((aInt & bInt) >= 0) {
- if (aInt < bInt)
- return LE_LESS;
- else if (aInt == bInt)
- return LE_EQUAL;
- else
- return LE_GREATER;
- }
-
- // Otherwise, both are negative, so we need to flip the sense of the
- // comparison to get the correct result. (This assumes a twos- or ones-
- // complement integer representation; if integers are represented in a
- // sign-magnitude representation, then this flip is incorrect).
- else {
- if (aInt > bInt)
- return LE_LESS;
- else if (aInt == bInt)
- return LE_EQUAL;
- else
- return LE_GREATER;
- }
-}
+COMPILER_RT_ABI CMP_RESULT __lesf2(fp_t a, fp_t b) { return __leXf2__(a, b); }
#if defined(__ELF__)
// Alias for libgcc compatibility
@@ -89,48 +51,12 @@ COMPILER_RT_ALIAS(__lesf2, __eqsf2)
COMPILER_RT_ALIAS(__lesf2, __ltsf2)
COMPILER_RT_ALIAS(__lesf2, __nesf2)
-enum GE_RESULT {
- GE_LESS = -1,
- GE_EQUAL = 0,
- GE_GREATER = 1,
- GE_UNORDERED = -1 // Note:
diff erent from LE_UNORDERED
-};
-
-COMPILER_RT_ABI enum GE_RESULT __gesf2(fp_t a, fp_t b) {
-
- const srep_t aInt = toRep(a);
- const srep_t bInt = toRep(b);
- const rep_t aAbs = aInt & absMask;
- const rep_t bAbs = bInt & absMask;
-
- if (aAbs > infRep || bAbs > infRep)
- return GE_UNORDERED;
- if ((aAbs | bAbs) == 0)
- return GE_EQUAL;
- if ((aInt & bInt) >= 0) {
- if (aInt < bInt)
- return GE_LESS;
- else if (aInt == bInt)
- return GE_EQUAL;
- else
- return GE_GREATER;
- } else {
- if (aInt > bInt)
- return GE_LESS;
- else if (aInt == bInt)
- return GE_EQUAL;
- else
- return GE_GREATER;
- }
-}
+COMPILER_RT_ABI CMP_RESULT __gesf2(fp_t a, fp_t b) { return __geXf2__(a, b); }
COMPILER_RT_ALIAS(__gesf2, __gtsf2)
-COMPILER_RT_ABI int
-__unordsf2(fp_t a, fp_t b) {
- const rep_t aAbs = toRep(a) & absMask;
- const rep_t bAbs = toRep(b) & absMask;
- return aAbs > infRep || bAbs > infRep;
+COMPILER_RT_ABI CMP_RESULT __unordsf2(fp_t a, fp_t b) {
+ return __unordXf2__(a, b);
}
#if defined(__ARM_EABI__)
diff --git a/compiler-rt/lib/builtins/comparetf2.c b/compiler-rt/lib/builtins/comparetf2.c
index 2eb34cf37fbcc..f1592454138cd 100644
--- a/compiler-rt/lib/builtins/comparetf2.c
+++ b/compiler-rt/lib/builtins/comparetf2.c
@@ -40,45 +40,9 @@
#include "fp_lib.h"
#if defined(CRT_HAS_128BIT) && defined(CRT_LDBL_128BIT)
-enum LE_RESULT { LE_LESS = -1, LE_EQUAL = 0, LE_GREATER = 1, LE_UNORDERED = 1 };
+#include "fp_compare_impl.inc"
-COMPILER_RT_ABI enum LE_RESULT __letf2(fp_t a, fp_t b) {
-
- const srep_t aInt = toRep(a);
- const srep_t bInt = toRep(b);
- const rep_t aAbs = aInt & absMask;
- const rep_t bAbs = bInt & absMask;
-
- // If either a or b is NaN, they are unordered.
- if (aAbs > infRep || bAbs > infRep)
- return LE_UNORDERED;
-
- // If a and b are both zeros, they are equal.
- if ((aAbs | bAbs) == 0)
- return LE_EQUAL;
-
- // If at least one of a and b is positive, we get the same result comparing
- // a and b as signed integers as we would with a floating-point compare.
- if ((aInt & bInt) >= 0) {
- if (aInt < bInt)
- return LE_LESS;
- else if (aInt == bInt)
- return LE_EQUAL;
- else
- return LE_GREATER;
- } else {
- // Otherwise, both are negative, so we need to flip the sense of the
- // comparison to get the correct result. (This assumes a twos- or ones-
- // complement integer representation; if integers are represented in a
- // sign-magnitude representation, then this flip is incorrect).
- if (aInt > bInt)
- return LE_LESS;
- else if (aInt == bInt)
- return LE_EQUAL;
- else
- return LE_GREATER;
- }
-}
+COMPILER_RT_ABI CMP_RESULT __letf2(fp_t a, fp_t b) { return __leXf2__(a, b); }
#if defined(__ELF__)
// Alias for libgcc compatibility
@@ -88,47 +52,12 @@ COMPILER_RT_ALIAS(__letf2, __eqtf2)
COMPILER_RT_ALIAS(__letf2, __lttf2)
COMPILER_RT_ALIAS(__letf2, __netf2)
-enum GE_RESULT {
- GE_LESS = -1,
- GE_EQUAL = 0,
- GE_GREATER = 1,
- GE_UNORDERED = -1 // Note:
diff erent from LE_UNORDERED
-};
-
-COMPILER_RT_ABI enum GE_RESULT __getf2(fp_t a, fp_t b) {
-
- const srep_t aInt = toRep(a);
- const srep_t bInt = toRep(b);
- const rep_t aAbs = aInt & absMask;
- const rep_t bAbs = bInt & absMask;
-
- if (aAbs > infRep || bAbs > infRep)
- return GE_UNORDERED;
- if ((aAbs | bAbs) == 0)
- return GE_EQUAL;
- if ((aInt & bInt) >= 0) {
- if (aInt < bInt)
- return GE_LESS;
- else if (aInt == bInt)
- return GE_EQUAL;
- else
- return GE_GREATER;
- } else {
- if (aInt > bInt)
- return GE_LESS;
- else if (aInt == bInt)
- return GE_EQUAL;
- else
- return GE_GREATER;
- }
-}
+COMPILER_RT_ABI CMP_RESULT __getf2(fp_t a, fp_t b) { return __geXf2__(a, b); }
COMPILER_RT_ALIAS(__getf2, __gttf2)
-COMPILER_RT_ABI int __unordtf2(fp_t a, fp_t b) {
- const rep_t aAbs = toRep(a) & absMask;
- const rep_t bAbs = toRep(b) & absMask;
- return aAbs > infRep || bAbs > infRep;
+COMPILER_RT_ABI CMP_RESULT __unordtf2(fp_t a, fp_t b) {
+ return __unordXf2__(a, b);
}
#endif
diff --git a/compiler-rt/lib/builtins/fp_compare_impl.inc b/compiler-rt/lib/builtins/fp_compare_impl.inc
new file mode 100644
index 0000000000000..40fc7df4c679c
--- /dev/null
+++ b/compiler-rt/lib/builtins/fp_compare_impl.inc
@@ -0,0 +1,116 @@
+//===-- lib/fp_compare_impl.inc - Floating-point comparison -------*- C -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "fp_lib.h"
+
+// GCC uses long (at least for x86_64) as the return type of the comparison
+// functions. We need to ensure that the return value is sign-extended in the
+// same way as GCC expects (since otherwise GCC-generated __builtin_isinf
+// returns true for finite 128-bit floating-point numbers).
+#ifdef __aarch64__
+// AArch64 GCC overrides libgcc_cmp_return to use int instead of long.
+typedef int CMP_RESULT;
+#elif __SIZEOF_POINTER__ == 8 && __SIZEOF_LONG__ == 4
+// LLP64 ABIs use long long instead of long.
+typedef long long CMP_RESULT;
+#else
+// Otherwise the comparison functions return long.
+typedef long CMP_RESULT;
+#endif
+
+#if !defined(__clang__) && defined(__GNUC__)
+// GCC uses a special __libgcc_cmp_return__ mode to define the return type, so
+// check that we are ABI-compatible when compiling the builtins with GCC.
+typedef int GCC_CMP_RESULT __attribute__((__mode__(__libgcc_cmp_return__)));
+_Static_assert(sizeof(GCC_CMP_RESULT) == sizeof(CMP_RESULT),
+ "SOFTFP ABI not compatible with GCC");
+#endif
+
+enum {
+ LE_LESS = -1,
+ LE_EQUAL = 0,
+ LE_GREATER = 1,
+ LE_UNORDERED = 1,
+};
+
+static inline CMP_RESULT __leXf2__(fp_t a, fp_t b) {
+ const srep_t aInt = toRep(a);
+ const srep_t bInt = toRep(b);
+ const rep_t aAbs = aInt & absMask;
+ const rep_t bAbs = bInt & absMask;
+
+ // If either a or b is NaN, they are unordered.
+ if (aAbs > infRep || bAbs > infRep)
+ return LE_UNORDERED;
+
+ // If a and b are both zeros, they are equal.
+ if ((aAbs | bAbs) == 0)
+ return LE_EQUAL;
+
+ // If at least one of a and b is positive, we get the same result comparing
+ // a and b as signed integers as we would with a floating-point compare.
+ if ((aInt & bInt) >= 0) {
+ if (aInt < bInt)
+ return LE_LESS;
+ else if (aInt == bInt)
+ return LE_EQUAL;
+ else
+ return LE_GREATER;
+ } else {
+ // Otherwise, both are negative, so we need to flip the sense of the
+ // comparison to get the correct result. (This assumes a twos- or ones-
+ // complement integer representation; if integers are represented in a
+ // sign-magnitude representation, then this flip is incorrect).
+ if (aInt > bInt)
+ return LE_LESS;
+ else if (aInt == bInt)
+ return LE_EQUAL;
+ else
+ return LE_GREATER;
+ }
+}
+
+enum {
+ GE_LESS = -1,
+ GE_EQUAL = 0,
+ GE_GREATER = 1,
+ GE_UNORDERED = -1 // Note:
diff erent from LE_UNORDERED
+};
+
+static inline CMP_RESULT __geXf2__(fp_t a, fp_t b) {
+ const srep_t aInt = toRep(a);
+ const srep_t bInt = toRep(b);
+ const rep_t aAbs = aInt & absMask;
+ const rep_t bAbs = bInt & absMask;
+
+ if (aAbs > infRep || bAbs > infRep)
+ return GE_UNORDERED;
+ if ((aAbs | bAbs) == 0)
+ return GE_EQUAL;
+ if ((aInt & bInt) >= 0) {
+ if (aInt < bInt)
+ return GE_LESS;
+ else if (aInt == bInt)
+ return GE_EQUAL;
+ else
+ return GE_GREATER;
+ } else {
+ if (aInt > bInt)
+ return GE_LESS;
+ else if (aInt == bInt)
+ return GE_EQUAL;
+ else
+ return GE_GREATER;
+ }
+}
+
+static inline CMP_RESULT __unordXf2__(fp_t a, fp_t b) {
+ const rep_t aAbs = toRep(a) & absMask;
+ const rep_t bAbs = toRep(b) & absMask;
+ return aAbs > infRep || bAbs > infRep;
+}
More information about the llvm-commits
mailing list