ppc64(le): Add an option to use IEEE long double ABI on Linux [2] by kinke · Pull Request #4840 · ldc-developers/ldc

kinke · 2025-02-18T21:20:46Z

Based on #4833, with my proposed changes.

... and then add a long double rewrite to convert it to ppc_f128 when lowering

... if -mabi=ieeelongdouble is specified on ppc64

... if IEEE long double ABI is selected

... to avoid a LLVM bug on multiple platforms

This will allow the user to specify -real-precision=quad + -mabi=elfv1 together without changing how LDC parses mabi options

... the system is using the new ABI that supports IEEE 754R long double instead of the legacy IBM double double format

ABI switching

liushuyu · 2025-02-18T21:28:47Z

gen/target.cpp

+#if defined(__linux__) && defined(__powerpc64__)
+    // for a PowerPC64 Linux build:
+    // default to the C++ host compiler's `long double` ABI
+    switch (std::numeric_limits<long double>::digits) {


I would use:

Suggested change

switch (std::numeric_limits<long double>::digits) {

switch (__LDBL_MANT_DIG__) {

This one is defined by the compiler directly and I have tested to contain correct information.

Fine by me. - We should probably only forward the host precision when targeting a Linux ppc64 target, not e.g. default to IEEE quad when cross-compiling to a ppc64 FreeBSD target.

when changed to the compiler macro, could this be #if sequence such that failure happens during compilation rather than at runtime?

liushuyu · 2025-02-18T21:34:40Z

gen/target.cpp

+  if (realPrecision == RealPrecision::Double) {
    return LLType::getDoubleTy(ctx);
+  } else if (realPrecision == RealPrecision::Quad) {
+    return LLType::getFP128Ty(ctx);


Have you tested if this will blow up x86 builds?

Android x86_64 is already using it, so I guess that's fine. - This is an advanced option (hence hidden), for pros only - druntime and Phobos need to match too etc. (no problem for simple betterC stuff in wasm, just need to compile everything with the override). If LLVM can't cope with this format for a target's codegen, it'll error out. [I'm a fan of giving users full control and letting them explore the compiler/LLVM limits.]

[Plus there are some compiler assumptions wrt. real precision, in existing TargetABI implementations - as the real precision wasn't configurable so far.]

[I'm a fan of giving users full control and letting them explore the compiler/LLVM limits.]

I see. Then that's fine by me. [Although LLVM is quite fragile when operating beyond its comfort zone]

RealPrecision::DoubleDouble should be handled here too (i.e. the user has set it explicitly)

RealPrecision::DoubleDouble should be handled here too (i.e. the user has set it explicitly)

Right, and for all other architectures, it should throw an error.

RealPrecision::DoubleDouble should be handled here too (i.e. the user has set it explicitly)

That I wanted to avoid though. I originally only wanted to expose the 2 standard IEEE formats as overrides, but unfortunately we need a way to make a native-PPC64 build with default IEEE quad ABI still cross-compile to the old doubledouble ABI. (I still don't wanna expose x87 if we can help it - although admittedly it could be useful for MSVC targets, to unlock the x87 real, which the compiler itself could use as real_t - bye-bye dmd.root.longdouble...)

But yeah, we can bail out as a compromise if this option is used for non-ppc64 targets.

JohanEngelen · 2025-02-18T21:58:59Z

gen/target.cpp

+      return LLType::getDoubleTy(ctx);
+    default:
+      llvm_unreachable("Unexpected host C++ 'long double' precision for a "
+                       "PowerPC64 target!");


perhaps print the actual __LDBL_MANT_DIG__ number in the diagnostic? (will help troubleshooting when this error triggers)

JohanEngelen · 2025-02-18T22:07:29Z

considering there is a difference in C++ mangling, should the D mangling scheme also be extended to differentiate the types? (would help with troubleshooting linking with binaries that have been compiled with different compiler settings, notably with prebuilt druntime/phobos) ?

gen/target.cpp

kinke · 2025-02-18T22:35:33Z

should the D mangling scheme also be extended to differentiate the types?

I'd say D doesn't need to, assuming PPC will remain the only architecture (well, AFAIK) with such a long double ABI change. [And users now starting to use -real-precision should IMO remain an uncommon special case, for wasm and the like, not needing the nicety of undefined symbols.]

kinke · 2025-02-18T22:52:48Z

driver/main.cpp

+      const llvm::SmallVector<llvm::StringRef> features{};
+      const std::string abi = getABI(triple, features);
+      VersionCondition::addPredefinedGlobalIdent(abi == "elfv1" ? "ELFv1"
+                                                                : "ELFv2");


According to https://github.com/dlang/dmd/blob/e082ce247afa6cf41c552ec57db2e95c3f7c0dac/druntime/src/etc/valgrind/valgrind.h#L154-L159, little-endian uses v2, big-endian v1.

According to https://github.com/dlang/dmd/blob/e082ce247afa6cf41c552ec57db2e95c3f7c0dac/druntime/src/etc/valgrind/valgrind.h#L154-L159, little-endian uses v2, big-endian v1.

That's a bad detection: big-endian ppc can use ELFv2, and little-endian ppc can also use ELFv1: https://godbolt.org/z/s5eceeqxs.

kinke · 2025-02-18T22:53:43Z

gen/modules.cpp

+      m.setModuleFlag(ModuleErrFlag, "float-abi", ConstantIEEE128String);
+    } else if (target.RealProperties.mant_dig == 106) {
+      const auto ConstantIBM128String = llvm::MDString::get(gIR->context(), "doubledouble");
+      m.setModuleFlag(ModuleErrFlag, "float-abi", ConstantIBM128String);


I take it this is just informational, or is this supported by some toolchains?

I take it this is just informational, or is this supported by some toolchains?

I see Clang emit this information, I am guessing this is to avoid mixing compilation units with different float ABIs when doing LTO.

kinke · 2025-02-18T22:56:42Z

gen/target.cpp

+    RealProperties.max_exp = 1024;
+    RealProperties.min_exp = -968;
+    RealProperties.max_10_exp = 308;
+    RealProperties.min_10_exp = -291;


I was surprised by this being quite a bit greater than double.min_10_exp (-307). How did you get these numbers? Querying GDC or the C++ compiler?

I was surprised by this being quite a bit greater than double.min_10_exp (-307). How did you get these numbers? Querying GDC or the C++ compiler?

Yes, the numbers are obtained from GCC/Clang. IBM stated due to the peculiarity of the type, those values are best-effort approximations.

kinke · 2025-02-19T16:58:51Z

@liushuyu: Are you able to/have you already tested this on ppc64le, checking whether the results haven't suffered since your original PR?

liushuyu · 2025-02-19T18:33:25Z

@liushuyu: Are you able to/have you already tested this on ppc64le, checking whether the results haven't suffered since your original PR?

I have tested your changes on a POWER 9 system and it seems okay.
Some parts of phobos are failing, but these are also failing in my PR (also these are not compiler issue).

kinke · 2025-02-20T12:08:01Z

Perfect, thx for testing! So I guess we're set and ready to merge? I'm happy with the overall changes.

liushuyu · 2025-02-20T18:13:55Z

Perfect, thx for testing! So I guess we're set and ready to merge? I'm happy with the overall changes.

I think it's ready for the merge.

liushuyu and others added 16 commits February 15, 2025 19:15

gen: use IEEE128 on ppc64le internally ...

05ccbb7

... and then add a long double rewrite to convert it to ppc_f128 when lowering

gen: use ppc_f128 or fp128 on ppc64le depending on the ABI switch

7802f8d

driver: add D_PPCUseIEEE128 pre-defined version ...

d892d68

... if -mabi=ieeelongdouble is specified on ppc64

druntime: redirect dual-ABI functions on glibc to IEEE128 version ...

84c6a2b

... if IEEE long double ABI is selected

gen/modules.cpp: add PowerPC float-abi metadata marking

eb7c41c

gen/toir.cpp: use xor operator to emulate unary not operation ...

b0ab201

... to avoid a LLVM bug on multiple platforms

gen/target.cpp: special C++ mangling for ppc64

a698caf

gen/abi/ppc64le.cpp: fix zext/sext for vector types

4e7800d

runtime/fiber: enable CheckFiberMigration for ppc64

26034e5

runtime/atomic: enable 128BitCAS for ppc64 when using IEEE 128 ABI

f6d20f4

driver/main + gen/target: fix environment detection for ppc64 real type

5180b4d

CHANGELOG.md: document ppc64 and ppc64le IEEE 128 support changes

4151349

gen+driver: use a different approach to parse and pass IEEE 128 options

ae4c0d8

This will allow the user to specify -real-precision=quad + -mabi=elfv1 together without changing how LDC parses mabi options

cmake: do not turn off long double on ppc64 if ...

5195def

... the system is using the new ABI that supports IEEE 754R long double instead of the legacy IBM double double format

tests/driver/ppc_float_abi.d: add a test to test ppc64 real type

96c455c

ABI switching

[rework]

12df0e4

kinke mentioned this pull request Feb 18, 2025

ppc64(le): Add an option to use IEEE long double ABI on Linux #4833

Merged

liushuyu reviewed Feb 18, 2025

View reviewed changes

JohanEngelen reviewed Feb 18, 2025

View reviewed changes

liushuyu reviewed Feb 18, 2025

View reviewed changes

gen/target.cpp Outdated Show resolved Hide resolved

kinke commented Feb 18, 2025

View reviewed changes

[address reviews]

e61fcd7

kinke merged commit 3726a93 into ldc-developers:master Feb 20, 2025
19 of 20 checks passed

kinke deleted the ppc64-d-ieee754-fix-new branch February 20, 2025 18:17

	switch (std::numeric_limits<long double>::digits) {
	switch (__LDBL_MANT_DIG__) {

Uh oh!

Conversation

kinke commented Feb 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JohanEngelen commented Feb 18, 2025

Uh oh!

Uh oh!

kinke commented Feb 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kinke commented Feb 19, 2025

Uh oh!

liushuyu commented Feb 19, 2025

Uh oh!

kinke commented Feb 20, 2025

Uh oh!

liushuyu commented Feb 20, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants