Page MenuHomePhabricator

[builtins] Build float128 soft float builtins for x86_64.
AbandonedPublic

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
troyj added a subscriber: troyj.Nov 7 2019, 11:33 AM
troyj added a comment.Nov 7 2019, 11:38 AM

Wound up here while trying to use compiler-rt for static linking with our downstream compiler. It seems that compiler-rt's current approach of only providing these routines on certain platforms is problematic because libgcc always provides them, and thus compiler-rt is not a full replacement for libgcc in some cases. I also encountered the same two missing entry points.

Would very much like to see an upstream change to match the libgcc entry points, no matter the platform.

Yes, it makes sense to provide these routines, but someone has to write the code to make it work. This patch is currently incomplete:

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.

manojgupta abandoned this revision.Oct 10 2020, 7:07 PM