[COMPILER-RT] Implement __fixtfsi, __fixunstfsi

[joerg]

Summary

Implement fixtfsi, fixunstfsi, which perform quad-precision to (sign/unsigned)integer conversion

[koviankevin]

add test

[koviankevin]

Use CRT_HAS_128BIT and modify the codes according to gribozavr's comments

[koviankevin]

introduce CRT_LDBL_128BIT

[koviankevin]

modify by gribozavr's comment and use COMPILER_RT_ABI

[joerg]

lib/builtins/fixtfsi.c

31

Typos, see comments below for return value.

48

Typo. Doesn't agree with the comment above in that it provides a deterministic value?

50

Shouldn't this be sign == 1 ? INT_MAX : INT_MIN, otherwise it would return INT_MIN + 1?

lib/builtins/fixunstfsi.c

26

Code does range check?

42

Typo in comment, please spell the constant as UINT_MAX too.

[koviankevin]

use UINT64_C and drop le64

[joerg]

With the comment fix.

lib/builtins/fixunstfsi.c

40

Still typo

[koviankevin]

fix typo

[koviankevin]

This patch blocks quad math stuff on aarch64 from using compiler-rt instead of gnu libgcc
Can this patch be commited, now? Thanks

[joerg]

lib/builtins/fixtfsi.c

34

See initial comment, no need for range check.

[joerg]

Refactor existing fixXfsi functions and use common code for fixtfsi. Drop range checks as they are UB.

[koviankevin]

Since these functions are implemented for using compiler-rt instead of libgcc, and __fixtfsi returns INT_MAX/INT_MIN when the input is out of range in libgcc, I think range check is still needed to avoid inconsistent results.

[joerg]

Steve -- you wrote the original version. Can you comment on the overflow handling please?

[scanon]

I think it makes sense to clamp out-of-range values. Even though the C language cast is UB, there’s a fairly clear “right” thing to do, and we can do it without excessive performance penalty. Let’s follow libgcc here and saturate.

[joerg]

Implement saturation logic for all __fix*si variants.

[koviankevin]

__fixunstfsi should have range check, too.

[koviankevin]

Saturation logic for _fixunstfsi is also needed but it has been removed in previous revision. (i.e, range check part in fixuntfsi.cpp)

[emaste]

In D2804#95834, @koviankevin wrote:

Saturation logic for _fixunstfsi is also needed but it has been removed in previous revision. (i.e, range check part in fixuntfsi.cpp)

Is this the only outstanding issue here?

[koviankevin]

Yes, it's not resonable to implement saturation logic for fixtfsi but not for fixuntfsi.

[emaste]

In D2804#112025, @koviankevin wrote:

Yes, it's not resonable to implement saturation logic for fixtfsi but not for fixuntfsi.

That's reasonable -- I'm not asking if it can be committed in the current state, but rather if it's ready to commit once this is addressed.

[koviankevin]

OK, but the following two cases in the test will fail if there's no saturation logic in fixunstfsi. I think it's better to remove them, thanks.

if (test__fixunstfsi(0x1.23456789abcdefp+40, UINT32_C(0xffffffff)))
    return 1;
if (test__fixunstfsi(0x1.23456789abcdefp+256, UINT32_C(0xffffffff)))
    return 1;

[joerg]

Refactor fix* and fixuns* to use common logic and consistent behavior w.r.t. saturation. XF format is still excluded for now.

[joerg]

Fix glitches for XF support.

[scanon]

Thanks for keeping after this. Looks good to me.

lib/builtins/fixtfsi.c

+//===-- lib/fixtfsi.c - Quad-precision -> integer conversion ------*- C -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file implements quad-precision to integer conversion for the
+// compiler-rt library.  No range checking is performed; the behavior of this
+// conversion is undefined for out of range values in the C standard.
+//
+//===----------------------------------------------------------------------===//
+
+#define QUAD_PRECISION
+#include "fp_lib.h"
+
+#if defined(CRT_HAS_128BIT) && defined(CRT_LDBL_128BIT)
+COMPILER_RT_ABI int __fixtfsi(fp_t a) {
+
+    // Break a into sign, exponent, significand
+    const rep_t aRep = toRep(a);
+    const rep_t aAbs = aRep & absMask;
+    const int sign = aRep & signBit ? -1 : 1;
+    const int exponent = (aAbs >> significandBits) - exponentBias;
+    const rep_t significand = (aAbs & significandMask) | implicitBit;
+
+    // If 0 < exponent < significandBits, right shift to get the result.
+    if ((unsigned int)exponent < significandBits) {
+        // value exceed INT_MAX, reuturn INT_MAX
+        if (exponent >= sizeof(int) * CHAR_BIT){
+            return sign * 0x7fffffff;
+        }
+        else {
+            return sign * (significand >> (significandBits - exponent));
+        }
+    }
+    // If exponent is negative, the result is zero.
+    else if (exponent < 0) {
+        return 0;
+    }
+    // If significandBits < exponent, left shift to get the result.  This shift
+    // may end up being larger than the type width, which incurs undefined
+    // behavior, but the conversion itself is undefined in that case, so
+    // whatever the compiler decides to do is fine.
+    else {
+        // value exceed INT_MAX, reuturn INT_MAX
+        if (exponent >= sizeof(int) * CHAR_BIT){
+            return sign * 0x7fffffff;
+        }
+        else {
+            return sign * (significand << (exponent - significandBits));
+        }
+    }
+}
+
+#endif

lib/builtins/fixunstfsi.c

+/* ===-- fixunstfsi.c - Implement __fixunstfsi -----------------------------===
+ *
+ *                     The LLVM Compiler Infrastructure
+ *
+ * This file is dual licensed under the MIT and the University of Illinois Open
+ * Source Licenses. See LICENSE.TXT for details.
+ *
+ * ===----------------------------------------------------------------------===
+ *
+ * This file implements __fixunstfsi for the compiler_rt library.
+ *
+ * ===----------------------------------------------------------------------===
+ */
+
+#define QUAD_PRECISION
+#include "fp_lib.h"
+
+#if defined(CRT_HAS_128BIT) && defined(CRT_LDBL_128BIT)
+/* Returns: convert a to a unsigned int, rounding toward zero.
+ *          Negative values all become zero.
+ */
+
+/* Assumption: long double is a IEEE 128 bit floating point type
+ *             su_int is a 32 bit integral type
+ *             value in long double is representable in su_int or is negative
+ *                 (no range checking performed)
+ */
+
+/* seee eeee eeee eeee mmmm mmmm mmmm mmmm | mmmm mmmm mmmm mmmm mmmm mmmm mmmm mmmm
+   mmmm mmmm mmmm mmmm mmmm mmmm mmmm mmmm | mmmm mmmm mmmm mmmm mmmm mmmm mmmm mmmm */
+
+COMPILER_RT_ABI su_int
+__fixunstfsi(long double a)
+{
+    long_double_bits fb;
+    fb.f = a;
+    int e = ((fb.u.high.s.high & 0x7FFF0000) >> 16) - 16383;
+    if (e < 0 || (fb.u.high.s.high & 0x80000000))
+        return 0;
+    // out ou range, return UINT_MAX
+    if (e >= sizeof(su_int) * CHAR_BIT){
+        return 0xffffffff;
+    }
+    return (
+                0x80000000u                      |
+                ((fb.u.high.s.high & 0x0000FFFF) << 15) |
+                (fb.u.high.s.low >> 17)
+           ) >> (31 - e);
+}
+
+#endif

test/builtins/Unit/fixtfsi_test.c

+//===--------------- fixtfsi_test.c - Test __fixtfsi ----------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file tests __fixtfsi for the compiler_rt library.
+//
+//===----------------------------------------------------------------------===//
+
+#include <stdio.h>
+
+#if __LP64__ && __LDBL_MANT_DIG__ == 113
+
+#include "fp_test.h"
+
+int __fixtfsi(long double a);
+
+int test__fixtfsi(long double a, int expected)
+{
+    int x = __fixtfsi(a);
+    int ret = (x != expected);
+
+    if (ret){
+        printf("error in test__fixtfsi(%.20Lf) = %d, "
+               "expected %d\n", a, x, expected);
+    }
+    return ret;
+}
+
+char assumption_1[sizeof(long double) * CHAR_BIT == 128] = {0};
+
+#endif
+
+int main()
+{
+#if __LP64__ && __LDBL_MANT_DIG__ == 113
+    if (test__fixtfsi(makeInf128(), 0x7fffffff))
+        return 1;
+    if (test__fixtfsi(0, 0x0))
+        return 1;
+    if (test__fixtfsi(0x1.23456789abcdefp+5, 0x24))
+        return 1;
+    if (test__fixtfsi(0x1.23456789abcdefp-3, 0x0))
+        return 1;
+    if (test__fixtfsi(0x1.23456789abcdefp+20, 0x123456))
+        return 1;
+    if (test__fixtfsi(0x1.23456789abcdefp+40, 0x7fffffff))
+        return 1;
+    if (test__fixtfsi(0x1.23456789abcdefp+256, 0x7fffffff))
+        return 1;
+    if (test__fixtfsi(-0x1.23456789abcdefp+20, 0xffedcbaa))
+        return 1;
+    if (test__fixtfsi(-0x1.23456789abcdefp+40, 0x80000001))
+        return 1;
+
+#else
+    printf("skipped\n");
+
+#endif
+    return 0;
+}

test/builtins/Unit/fixunstfsi_test.c

+//===--------------- fixunstfsi_test.c - Test __fixunstfsi ----------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file tests __fixunstfsi for the compiler_rt library.
+//
+//===----------------------------------------------------------------------===//
+
+#include <stdio.h>
+
+#if __LP64__ && __LDBL_MANT_DIG__ == 113
+
+#include "fp_test.h"
+
+unsigned int __fixunstfsi(long double a);
+
+int test__fixunstfsi(long double a, unsigned int expected)
+{
+    unsigned int x = __fixunstfsi(a);
+    int ret = (x != expected);
+
+    if (ret)
+    {
+        printf("error in test__fixunstfsi(%.20Lf) = %u, "
+               "expected %u\n", a, x, expected);
+    }
+    return ret;
+}
+
+char assumption_1[sizeof(long double) * CHAR_BIT == 128] = {0};
+
+#endif
+
+int main()
+{
+#if __LP64__ && __LDBL_MANT_DIG__ == 113
+    if (test__fixunstfsi(makeInf128(), 0xffffffff))
+        return 1;
+    if (test__fixunstfsi(0, 0x0U))
+        return 1;
+    if (test__fixunstfsi(0x1.23456789abcdefp+5, 0x24U))
+        return 1;
+    if (test__fixunstfsi(0x1.23456789abcdefp-3, 0x0U))
+        return 1;
+    if (test__fixunstfsi(0x1.23456789abcdefp+20, 0x123456U))
+        return 1;
+    if (test__fixunstfsi(0x1.23456789abcdefp+40, 0xffffffffU))
+        return 1;
+    if (test__fixunstfsi(0x1.23456789abcdefp+256, 0xffffffffU))
+        return 1;
+    if (test__fixunstfsi(-0x1.23456789abcdefp+3, 0x0U))
+        return 1;
+
+#else
+    printf("skipped\n");
+
+#endif
+    return 0;
+}