This is an archive of the discontinued LLVM Phabricator instance.

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

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 also 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 commit defines a new tf_float typedef that matches a float with
attribute((mode(TF)) on each given architecture.

There are more tests that could be enabled for x86, but to keep the diff
smaller, I restricted test changes to ones that started failing as part of
this refactoring.

Diff Detail

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

Rebase, drop most test changes and hopefully simplify the logic a bit

I've dropped the diff for the test from this commit which should hopefully make it easier to review.

compiler-rt/lib/builtins/int_types.h
168

Another option could be to use typedef float f128 __attribute__((mode("TF"));, but I'm not sure that's better.

  • Renamed macros and types to use TF instead of F128 to make it cleared that we are referring to mode(TF) values
  • Ran tests using GCC and qemu-user for PPC64 and AArch64 and checked that they pass
arichardson edited the summary of this revision. (Show Details)Jun 27 2023, 1:50 PM
arichardson added inline comments.Jun 27 2023, 1:53 PM
compiler-rt/test/builtins/Unit/floattitf_test.c
15

Comment is probably not quite accurate anymore as I ifdef'd out test cases that fail for PPC64.

pranavk added a subscriber: pranavk.Oct 3 2023, 9:14 AM