Page MenuHomePhabricator

[builtins] Build float128 soft float builtins for x86_64.
Needs ReviewPublic

Authored by manojgupta on Oct 23 2018, 1:38 PM.

Details

Reviewers
efriedma
joerg
Summary

float128 builtins are currently not built for x86_64.
This causes linker to complain baout missing symbols when linking
glibc 2.27 with float128 support.

Event Timeline

manojgupta created this revision.Oct 23 2018, 1:38 PM
manojgupta added inline comments.Oct 23 2018, 1:42 PM
lib/builtins/fp_lib.h
107

I really don't know the accurate set of checks that should be used here, Please advise,

114–115

Changed long double to __float128 on Eli's advice in PR39376.

efriedma added inline comments.Oct 23 2018, 2:13 PM
lib/builtins/fp_lib.h
114–115

You need to guard this with an ifdef; clang doesn't supports __float128 on every target, even if long double is an 128-bit IEEE float.

For reasons I don't really understand, there are apparently two different macros for this; #if defined(__FLOAT128__) || defined(__SIZEOF_FLOAT128__) should do the right thing.

Added checked for defined(FLOAT128) || defined(SIZEOF_FLOAT128)

manojgupta added inline comments.Oct 23 2018, 2:53 PM
lib/builtins/fp_lib.h
114–115

Thanks Eli. I also found out that GCC 4.9 does not seem to have these defined even though it supports __float128.
https://godbolt.org/z/ReVm8j

I am also getting these tests failures because of missing trunctfxf2 and extendxftf2. These are provided by libgcc but compiler-rt does not seem to have an implementation for them.

Builtins-x86_64-linux :: compiler_rt_logbl_test.c
Builtins-x86_64-linux :: divtc3_test.c
Builtins-x86_64-linux :: floattitf_test.c
Builtins-x86_64-linux :: floatuntitf_test.c

compiler_rt_logbl_test.c:26: error: undefined reference to 'trunctfxf2'
compiler_rt_logbl_test.c:26: error: undefined reference to '
extendxftf2'

__extendxftf2 and __trunctfxf2 are for conversions between x86 long double and __float128; you'll need to write implementations yourself. (That should be a separate patch.)

Thanks Eli. I also found out that GCC 4.9 does not seem to have these defined even though it supports __float128.

gcc 5 has the define; that's good enough.

Missing changes to run the unittests (test/builtins/Unit/) for the new functions. Unfortunately, it looks like that will be a large patch because they use "long double" and "LDBL_MANT_DIG" explicitly all over the place.

lib/builtins/fp_lib.h
107

Instead of checking for __x86_64__, this should probably also check for the FLOAT128 defines.

Took another look and seems like long double is hardcoded in many of the builtins. So I think the current patch needs to rename a lot of places using long double to __float128 type.

Some examples where I think __float128 type (propagating the type in fp_lib.h) should be used instead of long double:

./extenddftf2.c: COMPILER_RT_ABI long double extenddftf2(double a) {
./trunctfsf2.c: COMPILER_RT_ABI float
trunctfsf2(long double a) {
./extendsftf2.c: COMPILER_RT_ABI long double __extendsftf2(float a) {

@efriedma what do you think?

Yes, that looks right.

Hi
What's the status for __float128 support? Has it already been finished?

Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMar 9 2019, 2:04 AM
Herald added a subscriber: jdoerfert. · View Herald Transcript

Hi
What's the status for __float128 support? Has it already been finished?

Sorry, haven't looked at time for a while. Consider this change abandoned for now :(

E5ten added a subscriber: E5ten.Mar 15 2019, 12:59 PM