[libcxxabi] Use __LDBL_MANT_DIG__ for configuring demangling of long doubles #134976
Conversation
…doubles
This avoids needing to hardcode the mapping between architectures
and their sizes of long doubles.
This fixes a case in test_demangle.pass.cpp, that previously failed
like this (XFAILed):
.---command stdout------------
| Testing 29859 symbols.
| _ZN5test01hIfEEvRAcvjplstT_Le4001a000000000000000E_c should be invalid but is not
| Got: 0, void test0::h<float>(char (&) [(unsigned int)(sizeof (float) + 0x0.07ff98f7ep-1022L)])
`-----------------------------
.---command stderr------------
| Assertion failed: !passed && "demangle did not fail", file libcxxabi/test/test_demangle.pass.cpp, line 30338
`-----------------------------
This testcase is defined within
// Is long double fp80? (Only x87 extended double has 64-bit mantissa)
#define LDBL_FP80 (__LDBL_MANT_DIG__ == 64)
...
#if !LDBL_FP80
...
#endif
The case failed on x86 architectures with an unusual size for
long doubles, as the test expected the demangler to not be able to
demangle an 80 bit long double (based on the "_LDBL_MANT_DIG__ == 64"
condition in the test). However as the libcxxabi implementation was
hardcoded to demangle 80 bit long doubles on x86_64 regardless of
the actual size, this test failed (by unexpectedly being able to
demangle it).
By configuring libcxxabi's demangling of long doubles to match
what the compiler specifies, we no longer hit the expected failures
in the test_demangle.pass.cpp test on Android on x86.
This makes libcxxabi require a GCC-compatible compiler that defines
nonstandard defines like __LDBL_MANT_DIG__, but I presume that's
already essentially required anyway.
|
@llvm/pr-subscribers-libcxxabi Author: Martin Storsjö (mstorsjo) ChangesThis avoids needing to hardcode the mapping between architectures and their sizes of long doubles. This fixes a case in test_demangle.pass.cpp, that previously failed like this (XFAILed): This testcase is defined within The case failed on x86 architectures with an unusual size for long doubles, as the test expected the demangler to not be able to demangle an 80 bit long double (based on the By configuring libcxxabi's demangling of long doubles to match what the compiler specifies, we no longer hit the expected failures in the test_demangle.pass.cpp test on Android on x86. This makes libcxxabi require a GCC-compatible compiler that defines nonstandard defines like Full diff: https://github.com/llvm/llvm-project/pull/134976.diff 2 Files Affected:
diff --git a/libcxxabi/src/demangle/ItaniumDemangle.h b/libcxxabi/src/demangle/ItaniumDemangle.h
index 4122781beb599..328632d193751 100644
--- a/libcxxabi/src/demangle/ItaniumDemangle.h
+++ b/libcxxabi/src/demangle/ItaniumDemangle.h
@@ -5741,14 +5741,12 @@ struct FloatData<double>
template <>
struct FloatData<long double>
{
-#if defined(__mips__) && defined(__mips_n64) || defined(__aarch64__) || \
- defined(__wasm__) || defined(__riscv) || defined(__loongarch__) || \
- defined(__ve__)
- static const size_t mangled_size = 32;
-#elif defined(__arm__) || defined(__mips__) || defined(__hexagon__)
- static const size_t mangled_size = 16;
-#else
- static const size_t mangled_size = 20; // May need to be adjusted to 16 or 24 on other platforms
+#if __LDBL_MANT_DIG__ == 113
+ static const size_t mangled_size = 32;
+#elif __LDBL_MANT_DIG__ == 53
+ static const size_t mangled_size = 16;
+#else // __LDBL_MANT_DIG__ == 64
+ static const size_t mangled_size = 20;
#endif
// `-0x1.ffffffffffffffffffffffffffffp+16383` + 'L' + '\0' == 42 bytes.
// 28 'f's * 4 bits == 112 bits, which is the number of mantissa bits.
diff --git a/libcxxabi/test/test_demangle.pass.cpp b/libcxxabi/test/test_demangle.pass.cpp
index e9c74f70a094b..abaa787f5432b 100644
--- a/libcxxabi/test/test_demangle.pass.cpp
+++ b/libcxxabi/test/test_demangle.pass.cpp
@@ -13,10 +13,6 @@
// dd8b266ef.
// UNSUPPORTED: using-built-library-before-llvm-20
-// Android's long double on x86[-64] is (64/128)-bits instead of Linux's usual
-// 80-bit format, and this demangling test is failing on it.
-// XFAIL: LIBCXX-ANDROID-FIXME && target={{i686|x86_64}}-{{.+}}-android{{.*}}
-
// XFAIL: win32-broken-printf-a-precision
#include "support/timer.h"
|
| static const size_t mangled_size = 32; | ||
| #elif __LDBL_MANT_DIG__ == 53 | ||
| static const size_t mangled_size = 16; | ||
| #else // __LDBL_MANT_DIG__ == 64 |
There was a problem hiding this comment.
I think it'd be better to check the mantissa here as well and #error if there is a new size. Otherwise we might be silently wrong on some platforms.
There was a problem hiding this comment.
That sounds like a good idea, will fix.
However - I realized that while we don't support building libcxxabi with compilers that aren't GCC compatible, we also do copy this file into llvm/include/llvm/Demangle/ItaniumDemangle.h, which is built as part of the host build of LLVM - and that can be built with MSVC as well. (And MSVC doesn't define any of the arch defines that we defined before either.)
MSVC only has long double == double for all currently known architectures (and clang-cl matches this), so I can add an || defined(_MSC_VER) for that.
Also check for _MSC_VER for MSVC, as this file is copied and included in the main LLVM build as well.
There was a problem hiding this comment.
Looks good to me.
(FWIW, on x86-64 Android, Clang doesn't emit the e mangling -- instead, it emits g for both long double and __float128. https://godbolt.org/z/Gvqof41xz)
|
This is failing on the ppc64le-linux buildbot (https://lab.llvm.org/buildbot/#/builders/72/builds/10061): |
Sorry about that. I see that powerpc has got Anyway, we need to fix this. Do you suggest a revert, or fix forward like this? diff --git a/libcxxabi/src/demangle/ItaniumDemangle.h b/libcxxabi/src/demangle/ItaniumDemangle.h
index b9d17060c04f..eca9ddad66f9 100644
--- a/libcxxabi/src/demangle/ItaniumDemangle.h
+++ b/libcxxabi/src/demangle/ItaniumDemangle.h
@@ -5741,7 +5741,7 @@ struct FloatData<double>
template <>
struct FloatData<long double>
{
-#if __LDBL_MANT_DIG__ == 113
+#if __LDBL_MANT_DIG__ == 113 || __LDBL_MANT_DIG__ == 106
static const size_t mangled_size = 32;
#elif __LDBL_MANT_DIG__ == 53 || defined(_MSC_VER)
// MSVC doesn't define __LDBL_MANT_DIG__, but it has long double equal to |
|
I managed to test on a ppc64le machine, that the above diff works and the cxxabi tests still pass with that in place. |
|
See #135332 for the forward-fix here. |
|
Thanks for landing the fix-forward so quickly! Sorry for not being able to reply earlier - I had gone to bed (timezone difference). |
|
FYI, can you run |
…ing of long doubles Syncing in the changes from llvm#134976 using the `cp-to-llvm.sh` script.
|
…ing demangling of long doubles (#135968) Syncing in the changes from llvm/llvm-project#134976 using the `cp-to-llvm.sh` script.
…doubles (llvm#134976) This avoids needing to hardcode the mapping between architectures and their sizes of long doubles. This fixes a case in test_demangle.pass.cpp, that previously failed like this (XFAILed): .---command stdout------------ | Testing 29859 symbols. | _ZN5test01hIfEEvRAcvjplstT_Le4001a000000000000000E_c should be invalid but is not | Got: 0, void test0::h<float>(char (&) [(unsigned int)(sizeof (float) + 0x0.07ff98f7ep-1022L)]) `----------------------------- .---command stderr------------ | Assertion failed: !passed && "demangle did not fail", file libcxxabi/test/test_demangle.pass.cpp, line 30338 `----------------------------- This testcase is defined within // Is long double fp80? (Only x87 extended double has 64-bit mantissa) #define LDBL_FP80 (__LDBL_MANT_DIG__ == 64) ... #if !LDBL_FP80 ... #endif The case failed on x86 architectures with an unusual size for long doubles, as the test expected the demangler to not be able to demangle an 80 bit long double (based on the `__LDBL_MANT_DIG__ == 64` condition in the test). However as the libcxxabi implementation was hardcoded to demangle 80 bit long doubles on x86_64 regardless of the actual size, this test failed (by unexpectedly being able to demangle it). By configuring libcxxabi's demangling of long doubles to match what the compiler specifies, we no longer hit the expected failures in the test_demangle.pass.cpp test on Android on x86. This makes libcxxabi require a GCC-compatible compiler that defines nonstandard defines like `__LDBL_MANT_DIG__`, but I presume that's already essentially required anyway.
…ing of long doubles (llvm#135968) Syncing in the changes from llvm#134976 using the `cp-to-llvm.sh` script.
This avoids needing to hardcode the mapping between architectures and their sizes of long doubles.
This fixes a case in test_demangle.pass.cpp, that previously failed like this (XFAILed):
This testcase is defined within
The case failed on x86 architectures with an unusual size for long doubles, as the test expected the demangler to not be able to demangle an 80 bit long double (based on the
__LDBL_MANT_DIG__ == 64condition in the test). However as the libcxxabi implementation was hardcoded to demangle 80 bit long doubles on x86_64 regardless of the actual size, this test failed (by unexpectedly being able to demangle it).By configuring libcxxabi's demangling of long doubles to match what the compiler specifies, we no longer hit the expected failures in the test_demangle.pass.cpp test on Android on x86.
This makes libcxxabi require a GCC-compatible compiler that defines nonstandard defines like
__LDBL_MANT_DIG__, but I presume that's already essentially required anyway.