[compiler-rt] Implement __extendxftf2 and __trunctfxf2 for x86_64

[alexander-shaposhnikov]

This patch implements __extendxftf2 (long double -> f128) and
__trunctfxf2 (f128 -> long double) on x86_64.
This is a preparation to unblock https://reviews.llvm.org/D53608,
We intentionally do not modify compiler-rt/lib/builtins/fp_lib.h in this PR
(in particular, to limit the scope and avoid exposing other functions on X86_64 in this PR).
Instead, TODOs were added to use fp_lib.h once it is available.

Test plan:

1. ninja check-compiler-rt (verified on X86_64 and on Aarch64)
   In particular, new tests (extendxftf2_test.c and trunctfxf2_test.c) were added.
2. compared the results of conversions with what other compilers (gcc) produce.

[alexander-shaposhnikov]

@efriedma-quic

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff b225934a4b0d2944958a53269665b00e7eae4875 6811d6e51b4d8af3a6d423839d0b9438e7945c57 -- compiler-rt/lib/builtins/extendxftf2.c compiler-rt/lib/builtins/trunctfxf2.c compiler-rt/test/builtins/Unit/extendxftf2_test.c compiler-rt/test/builtins/Unit/trunctfxf2_test.c compiler-rt/lib/builtins/fp_extend.h compiler-rt/lib/builtins/fp_trunc.h compiler-rt/test/builtins/Unit/addtf3_test.c compiler-rt/test/builtins/Unit/divtf3_test.c compiler-rt/test/builtins/Unit/extenddftf2_test.c compiler-rt/test/builtins/Unit/extendhftf2_test.c compiler-rt/test/builtins/Unit/extendsftf2_test.c compiler-rt/test/builtins/Unit/floatditf_test.c compiler-rt/test/builtins/Unit/floatsitf_test.c compiler-rt/test/builtins/Unit/floatunditf_test.c compiler-rt/test/builtins/Unit/floatunsitf_test.c compiler-rt/test/builtins/Unit/fp_test.h compiler-rt/test/builtins/Unit/multf3_test.c compiler-rt/test/builtins/Unit/subtf3_test.c

View the diff from clang-format here.

diff --git a/compiler-rt/lib/builtins/fp_extend.h b/compiler-rt/lib/builtins/fp_extend.h
index 86b32be12..caed34dd8 100644
--- a/compiler-rt/lib/builtins/fp_extend.h
+++ b/compiler-rt/lib/builtins/fp_extend.h
@@ -137,13 +137,15 @@ static inline src_rep_t extract_sig_frac_from_src(src_rep_t x) {
 
 #ifdef src_rep_t_clz
 static inline int clz_in_sig_frac(src_rep_t sigFrac) {
-      const int skip = (sizeof(dst_t) * CHAR_BIT - srcBits) + 1 + srcExpBits;
-      return src_rep_t_clz(sigFrac) - skip;
+  const int skip = (sizeof(dst_t) * CHAR_BIT - srcBits) + 1 + srcExpBits;
+  return src_rep_t_clz(sigFrac) - skip;
 }
 #endif
 
-static inline dst_rep_t construct_dst_rep(dst_rep_t sign, dst_rep_t exp, dst_rep_t sigFrac) {
-  return (sign << (dstBits - 1)) | (exp << (dstBits - 1 - dstExpBits)) | sigFrac;
+static inline dst_rep_t construct_dst_rep(dst_rep_t sign, dst_rep_t exp,
+                                          dst_rep_t sigFrac) {
+  return (sign << (dstBits - 1)) | (exp << (dstBits - 1 - dstExpBits)) |
+         sigFrac;
 }
 
 // Two helper routines for conversion to and from the representation of
diff --git a/compiler-rt/lib/builtins/fp_trunc.h b/compiler-rt/lib/builtins/fp_trunc.h
index ea13dc2ef..52acbce3a 100644
--- a/compiler-rt/lib/builtins/fp_trunc.h
+++ b/compiler-rt/lib/builtins/fp_trunc.h
@@ -125,8 +125,10 @@ static inline src_rep_t extract_sig_frac_from_src(src_rep_t x) {
   return x & srcSigFracMask;
 }
 
-static inline dst_rep_t construct_dst_rep(dst_rep_t sign, dst_rep_t exp, dst_rep_t sigFrac) {
-  dst_rep_t result = (sign << (dstBits - 1)) | (exp << (dstBits - 1 - dstExpBits)) | sigFrac;
+static inline dst_rep_t construct_dst_rep(dst_rep_t sign, dst_rep_t exp,
+                                          dst_rep_t sigFrac) {
+  dst_rep_t result =
+      (sign << (dstBits - 1)) | (exp << (dstBits - 1 - dstExpBits)) | sigFrac;
   // Set the explicit integer bit in F80 if present.
   if (dstBits == 80 && exp) {
     result |= (DST_REP_C(1) << dstSigFracBits);
diff --git a/compiler-rt/test/builtins/Unit/extendxftf2_test.c b/compiler-rt/test/builtins/Unit/extendxftf2_test.c
index f52118754..8537c4329 100644
--- a/compiler-rt/test/builtins/Unit/extendxftf2_test.c
+++ b/compiler-rt/test/builtins/Unit/extendxftf2_test.c
@@ -7,7 +7,7 @@
 #if __LDBL_MANT_DIG__ == 64 && defined(__x86_64__) &&                          \
     (defined(__FLOAT128__) || defined(__SIZEOF_FLOAT128__))
 
-#include "fp_test.h"
+#  include "fp_test.h"
 
 COMPILER_RT_ABI __float128 __extendxftf2(long double a);
 
diff --git a/compiler-rt/test/builtins/Unit/fp_test.h b/compiler-rt/test/builtins/Unit/fp_test.h
index f095ae070..3f2f4e018 100644
--- a/compiler-rt/test/builtins/Unit/fp_test.h
+++ b/compiler-rt/test/builtins/Unit/fp_test.h
@@ -13,12 +13,12 @@
 #if __LDBL_MANT_DIG__ == 113 ||                                                \
     ((__LDBL_MANT_DIG__ == 64) && defined(__x86_64__) &&                       \
      (defined(__FLOAT128__) || defined(__SIZEOF_FLOAT128__)))
-#if __LDBL_MANT_DIG__ == 113
-#define TYPE_FP128 long double
-#else
-#define TYPE_FP128 __float128
-#endif
-#define TEST_COMPILER_RT_HAS_FLOAT128
+#  if __LDBL_MANT_DIG__ == 113
+#    define TYPE_FP128 long double
+#  else
+#    define TYPE_FP128 __float128
+#  endif
+#  define TEST_COMPILER_RT_HAS_FLOAT128
 #endif
 
 enum EXPECTED_RESULT {
@@ -52,10 +52,10 @@ static inline double fromRep64(uint64_t x)
 
 #ifdef TEST_COMPILER_RT_HAS_FLOAT128
 static inline TYPE_FP128 fromRep128(uint64_t hi, uint64_t lo) {
-    __uint128_t x = ((__uint128_t)hi << 64) + lo;
-    TYPE_FP128 ret;
-    memcpy(&ret, &x, 16);
-    return ret;
+  __uint128_t x = ((__uint128_t)hi << 64) + lo;
+  TYPE_FP128 ret;
+  memcpy(&ret, &x, 16);
+  return ret;
 }
 #endif
 
@@ -86,9 +86,9 @@ static inline uint64_t toRep64(double x)
 
 #ifdef TEST_COMPILER_RT_HAS_FLOAT128
 static inline __uint128_t toRep128(TYPE_FP128 x) {
-    __uint128_t ret;
-    memcpy(&ret, &x, 16);
-    return ret;
+  __uint128_t ret;
+  memcpy(&ret, &x, 16);
+  return ret;
 }
 #endif
 
@@ -152,21 +152,21 @@ static inline int compareResultD(double result,
 // because 128-bit integer constant can't be assigned directly
 static inline int compareResultF128(TYPE_FP128 result, uint64_t expectedHi,
                                     uint64_t expectedLo) {
-    __uint128_t rep = toRep128(result);
-    uint64_t hi = rep >> 64;
-    uint64_t lo = rep;
-
-    if (hi == expectedHi && lo == expectedLo) {
-        return 0;
+  __uint128_t rep = toRep128(result);
+  uint64_t hi = rep >> 64;
+  uint64_t lo = rep;
+
+  if (hi == expectedHi && lo == expectedLo) {
+    return 0;
+  }
+  // test other possible NaN representation(signal NaN)
+  else if (expectedHi == 0x7fff800000000000UL && expectedLo == 0x0UL) {
+    if ((hi & 0x7fff000000000000UL) == 0x7fff000000000000UL &&
+        ((hi & 0xffffffffffffUL) > 0 || lo > 0)) {
+      return 0;
     }
-    // test other possible NaN representation(signal NaN)
-    else if (expectedHi == 0x7fff800000000000UL && expectedLo == 0x0UL) {
-        if ((hi & 0x7fff000000000000UL) == 0x7fff000000000000UL &&
-            ((hi & 0xffffffffffffUL) > 0 || lo > 0)) {
-            return 0;
-        }
-    }
-    return 1;
+  }
+  return 1;
 }
 #endif
 
@@ -242,44 +242,44 @@ static inline double makeQNaN64(void)
 
 #if __LDBL_MANT_DIG__ == 64 && defined(__x86_64__)
 static inline long double F80FromRep128(uint64_t hi, uint64_t lo) {
-    __uint128_t x = ((__uint128_t)hi << 64) + lo;
-    long double ret;
-    memcpy(&ret, &x, 16);
-    return ret;
+  __uint128_t x = ((__uint128_t)hi << 64) + lo;
+  long double ret;
+  memcpy(&ret, &x, 16);
+  return ret;
 }
 
 static inline __uint128_t F80ToRep128(long double x) {
-    __uint128_t ret;
-    memcpy(&ret, &x, 16);
-    return ret;
+  __uint128_t ret;
+  memcpy(&ret, &x, 16);
+  return ret;
 }
 
 static inline int compareResultF80(long double result, uint64_t expectedHi,
                                    uint64_t expectedLo) {
-    __uint128_t rep = F80ToRep128(result);
-    // F80 occupies the lower 80 bits of __uint128_t.
-    uint64_t hi = (rep >> 64) & ((1UL << (80 - 64)) - 1);
-    uint64_t lo = rep;
-    return !(hi == expectedHi && lo == expectedLo);
+  __uint128_t rep = F80ToRep128(result);
+  // F80 occupies the lower 80 bits of __uint128_t.
+  uint64_t hi = (rep >> 64) & ((1UL << (80 - 64)) - 1);
+  uint64_t lo = rep;
+  return !(hi == expectedHi && lo == expectedLo);
 }
 
 static inline long double makeQNaN80(void) {
-    return F80FromRep128(0x7fffUL, 0xc000000000000000UL);
+  return F80FromRep128(0x7fffUL, 0xc000000000000000UL);
 }
 
 static inline long double makeNaN80(uint64_t rand) {
-    return F80FromRep128(0x7fffUL,
-                         0x8000000000000000 | (rand & 0x3fffffffffffffff));
+  return F80FromRep128(0x7fffUL,
+                       0x8000000000000000 | (rand & 0x3fffffffffffffff));
 }
 
 static inline long double makeInf80(void) {
-    return F80FromRep128(0x7fffUL, 0x8000000000000000UL);
+  return F80FromRep128(0x7fffUL, 0x8000000000000000UL);
 }
 #endif
 
 #ifdef TEST_COMPILER_RT_HAS_FLOAT128
 static inline TYPE_FP128 makeQNaN128(void) {
-    return fromRep128(0x7fff800000000000UL, 0x0UL);
+  return fromRep128(0x7fff800000000000UL, 0x0UL);
 }
 #endif
 
@@ -300,7 +300,7 @@ static inline double makeNaN64(uint64_t rand)
 
 #ifdef TEST_COMPILER_RT_HAS_FLOAT128
 static inline TYPE_FP128 makeNaN128(uint64_t rand) {
-    return fromRep128(0x7fff000000000000UL | (rand & 0xffffffffffffUL), 0x0UL);
+  return fromRep128(0x7fff000000000000UL | (rand & 0xffffffffffffUL), 0x0UL);
 }
 #endif
 
@@ -331,10 +331,10 @@ static inline double makeNegativeInf64(void)
 
 #ifdef TEST_COMPILER_RT_HAS_FLOAT128
 static inline TYPE_FP128 makeInf128(void) {
-    return fromRep128(0x7fff000000000000UL, 0x0UL);
+  return fromRep128(0x7fff000000000000UL, 0x0UL);
 }
 
 static inline TYPE_FP128 makeNegativeInf128(void) {
-    return fromRep128(0xffff000000000000UL, 0x0UL);
+  return fromRep128(0xffff000000000000UL, 0x0UL);
 }
 #endif
diff --git a/compiler-rt/test/builtins/Unit/trunctfxf2_test.c b/compiler-rt/test/builtins/Unit/trunctfxf2_test.c
index 53024ef13..b6b64351e 100644
--- a/compiler-rt/test/builtins/Unit/trunctfxf2_test.c
+++ b/compiler-rt/test/builtins/Unit/trunctfxf2_test.c
@@ -7,7 +7,7 @@
 #if __LDBL_MANT_DIG__ == 64 && defined(__x86_64__) &&                          \
     (defined(__FLOAT128__) || defined(__SIZEOF_FLOAT128__))
 
-#include "fp_test.h"
+#  include "fp_test.h"
 
 COMPILER_RT_ABI long double __trunctfxf2(__float128 a);
 

[echristo]

Adding @erichkeane here as well to help review.

[erichkeane]

Adding @erichkeane here as well to help review.

I have no idea about this, I actually have no idea what these do. Were you perhaps thinking of someone else?

[echristo]

This looks reasonable to me. I've added Erich in case he has any comments before merging.

[efriedma-quic]

I don't see any tests for denormal numbers?

I'm a little concerned that mixing together fp80 and non-fp80 codepaths is going to be confusing for anyone unfamiliar with fp80 because the rules for fp80 significands are different... can you try to pull out the differences into helper methods, so it's more clear what's the IEEE codepath, and what's the non-IEEE codepath?

[alexander-shaposhnikov]

@efriedma-quic

1. Added more tests (for denormal numbers).
2. Refactored/Unified the codepaths for f_80 vs usual f_N.

[MaskRay]

I will be out of town for about 2 weeks but happy to take a look after the trip...

[arichardson]

Some of this can be simplified if #68132 lands first, but I guess the cleanups could also be done afterwards?

[alexander-shaposhnikov]

@arichardson - yeah, there are opportunities for clean ups, adding these conversions will unblock a few things => so wanted to do them afterwards.

[alexander-shaposhnikov]

@efriedma-quic - both "extend" and "truncate" have been updated/refactored.

[arichardson]

No objections from my side, cleanups can be done later.

[efriedma-quic]

LGTM