Page MenuHomePhabricator

[builtins] Support building the 128-bit float functions on x86
Needs ReviewPublic

Authored by arichardson on Mar 9 2021, 7:13 AM.

Details

Summary

GCC provides these functions (e.g. __addtf3, etc.) in libgcc on x86_64.
Since Clang supports float128, we can enable the existing code by using
float128 for fp_t if either __FLOAT128__ or __SIZEOF_FLOAT128__
is defined instead of only supporting these builtins for platforms with
128-bit IEEE long doubles.

This change also replaces the CRT_LDBL_128BIT macro with CRT_HAS_F128 to
indicate that it doesn't depend on long double being a 128-bit IEEE float.

The commit is rather large since it also updates all the tests. If this makes
it difficult to review, I'm happy to split the test changes into a separate
review.

Diff Detail

Unit TestsFailed

TimeTest
200 msx64 debian > Builtins-x86_64-linux.Builtins-x86_64-linux::floattitf_test.c
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang -gline-tables-only -m64 -fno-builtin -I /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/lib/builtins -nodefaultlibs /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/builtins/Unit/floattitf_test.c /var/lib/buildkite-agent/builds/llvm-project/build/./lib/clang/16.0.0/lib/x86_64-unknown-linux-gnu/libclang_rt.builtins.a -lc -lm -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/builtins/Unit/X86_64LinuxConfig/Output/floattitf_test.c.tmp && /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/builtins/Unit/X86_64LinuxConfig/Output/floattitf_test.c.tmp
180 msx64 debian > Builtins-x86_64-linux.Builtins-x86_64-linux::floatuntitf_test.c
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang -gline-tables-only -m64 -fno-builtin -I /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/lib/builtins -nodefaultlibs /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/builtins/Unit/floatuntitf_test.c /var/lib/buildkite-agent/builds/llvm-project/build/./lib/clang/16.0.0/lib/x86_64-unknown-linux-gnu/libclang_rt.builtins.a -lc -lm -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/builtins/Unit/X86_64LinuxConfig/Output/floatuntitf_test.c.tmp && /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/builtins/Unit/X86_64LinuxConfig/Output/floatuntitf_test.c.tmp

Event Timeline

arichardson created this revision.Mar 9 2021, 7:13 AM
arichardson requested review of this revision.Mar 9 2021, 7:13 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 9 2021, 7:13 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript
  • Fix tests with clang-10 (supports float128 but not _Complex float128)

fix unused variable in test helpers

I applied this patch to an internal workspace and did smoke testing on Linux and Windows--didn't see any problems. But I defer to others to +1.

I applied this patch to an internal workspace and did smoke testing on Linux and Windows--didn't see any problems. But I defer to others to +1.

Thanks for testing!

If this is too awkward to review, should I split the test changes to a separate revision?

  • fix tests on platforms without 128-floating point types (e.g. x86 macOS)

If I am not misreading this, this patch changes the implementation of at least __powitf2 to no longer use long double but __float128. Such changes will not work on PowerPC because there are two different 128-bit floating point types on PPC - the "PPC double double" and IEEE-754 quad precision. And long double could mean either one of those (depending on -mabi=ieeelongdouble). The tf libcalls on PPC are used for long double whereas kf ones are used for explicit __float128.

If I am not misreading this, this patch changes the implementation of at least __powitf2 to no longer use long double but __float128. Such changes will not work on PowerPC because there are two different 128-bit floating point types on PPC - the "PPC double double" and IEEE-754 quad precision. And long double could mean either one of those (depending on -mabi=ieeelongdouble). The tf libcalls on PPC are used for long double whereas kf ones are used for explicit __float128.

Thanks, I wasn't sure about PPC64. If I understand correctly, the solution for this would be to change the #if __LDBL_MANT_DIG__ == 113 check to sizeof(long double) > 8. And instead of using a f128 typedef there needs to be a f128_tf and a f128_kf type (or some better name)?

Thanks, I wasn't sure about PPC64. If I understand correctly, the solution for this would be to change the #if __LDBL_MANT_DIG__ == 113 check to sizeof(long double) > 8. And instead of using a f128 typedef there needs to be a f128_tf and a f128_kf type (or some better name)?

I think that is correct.

Rebase and try to address PPC64 concerns. I don't have PPC hardware for testing this, but the tests pass for x86_64 and i386 (with a fix for __floatuntitf that I've uploaded in a separate diff)

Herald added a project: Restricted Project. · View Herald TranscriptAug 12 2022, 9:26 AM
arichardson added inline comments.Aug 12 2022, 9:27 AM
compiler-rt/lib/builtins/int_types.h
169

@nemanjai I hope this is correct, would be great if you could double-check it.

arichardson edited the summary of this revision. (Show Details)Aug 12 2022, 9:28 AM