Compiler-rt missing __float128 builtins for x86_64

Quuxplusone commented 6 years ago


Bugzilla Link	PR39376
Status	NEW
Importance	P enhancement
Reported by	Manoj Gupta (manojgupta@google.com)
Reported on	2018-10-21 19:41:05 -0700
Last modified on	2021-10-08 06:35:38 -0700
Version	unspecified
Hardware	PC Windows NT
CC	dodoentertainment@gmail.com, efriedma@quicinc.com, joerg@NetBSD.org, llozano@chromium.org, llvm-bugs@lists.llvm.org, manojgupta@google.com, paul@pbristow.uk, saugustine@google.com, srhines@google.com, yunlian@google.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

Chrome OS bug: https://bugs.chromium.org/p/chromium/issues/detail?id=843822

With glibc 2.27, clang cannot compile a static binary with compiler-rt since
compiler-rt does not have support for long double builtins for x86_64.

With glibc 2.27
cat a.c
#include "stdio.h"
int main() {
  printf("hello.\n");
}

 clang -static a.c -rtlib=compiler-rt
/usr/lib/gcc/x86_64-pc-linux-gnu/4.9.x/../../../../lib64/libc.a(printf_fp.o):
In function `__printf_fp_l':
(.text+0x5ea): undefined reference to `__unordtf2'
/usr/lib/gcc/x86_64-pc-linux-
gnu/4.9.x/../../../../lib64/libc.a(printf_fphex.o): In function
`__printf_fphex':
(.text+0x9a): undefined reference to `__unordtf2'

The following patch is enough to add the builtins to compiler-rt but not sure
of the correctness or any other implications. Interesting thing to note is that
__LDBL_MANT_DIG__ is 64 so the check __LDBL_MANT_DIG__ == 113 failed on
x86_64/Linux.

diff --git a/lib/builtins/CMakeLists.txt b/lib/builtins/CMakeLists.txt
index 82332967b..c49d9af00 100644
--- a/lib/builtins/CMakeLists.txt
+++ b/lib/builtins/CMakeLists.txt
@@ -232,6 +232,7 @@ set(x86_ARCH_SOURCES

 if (NOT MSVC)
   set(x86_64_SOURCES
+      ${GENERIC_TF_SOURCES}
       x86_64/floatdidf.c
       x86_64/floatdisf.c
       x86_64/floatdixf.c
diff --git a/lib/builtins/fp_lib.h b/lib/builtins/fp_lib.h
index a0e19ab6a..c4f9d1d90 100644
--- a/lib/builtins/fp_lib.h
+++ b/lib/builtins/fp_lib.h
@@ -103,7 +103,7 @@ static __inline void wideMultiply(rep_t a, rep_t b, rep_t
*hi, rep_t *lo) {
 COMPILER_RT_ABI fp_t __adddf3(fp_t a, fp_t b);

 #elif defined QUAD_PRECISION
-#if __LDBL_MANT_DIG__ == 113
+#ifdef __LP64__
 #define CRT_LDBL_128BIT
 typedef __uint128_t rep_t;
 typedef __int128_t srep_t;

Quuxplusone commented 6 years ago

Aren't you confusing long double (aka 80bit Intel extended precision) with IEEE754 quad precision?

Quuxplusone commented 6 years ago

Compiler-rt is using the long double type for the builtins under quad precision
(for all architectures). Maybe __float128 should have been used instead?

For x86, My understanding is generally compiler converts uses of long double to
fp80 instructions so the uses are often not emitted as call to builtins.

lib/builtins/fp_lib.h

elif defined QUAD_PRECISION
#if __LDBL_MANT_DIG__ == 113
#define CRT_LDBL_128BIT
typedef __uint128_t rep_t;
typedef __int128_t srep_t;
typedef long double fp_t;
#define REP_C (__uint128_t)

Quuxplusone commented 6 years ago

(In reply to Manoj Gupta from comment #2)
> Compiler-rt is using the long double type for the builtins under quad
> precision (for all architectures). Maybe __float128 should have been used
> instead?

That's the right approach.  (compiler-rt was written before __float128 existed,
so "should have been used" isn't really right, but you get the idea.)