[libcxx] [test] Clarify the condition for long double hex formatting

[mstorsjo]

This test currently hardcodes which environments have got 80 bit long doubles on x86_64; add a check for the actual size of the long doubles as well.

This allows waiving this part of the test, if we have x86_64 setups in any of these environments, configured for a nonstandard size of long doubles.

Also clarify the exact reasons for why specific OSes such as FreeBSD are skipped for these tests.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Martin Storsjö (mstorsjo)

Changes

This test currently hardcodes which environments have got 80 bit long doubles on x86_64; add a check for the actual size of the long doubles as well.

This allows waiving this part of the test, if we have x86_64 setups in any of these environments, configured for a nonstandard size of long doubles.

Also clarify the exact reasons for why specific OSes such as FreeBSD are skipped for these tests.

---

Full diff: https://github.com/llvm/llvm-project/pull/135334.diff

1 Files Affected:

• (modified) libcxx/test/std/localization/locale.categories/category.numeric/locale.nm.put/facet.num.put.members/put_long_double.hex.pass.cpp (+20-5)

diff --git a/libcxx/test/std/localization/locale.categories/category.numeric/locale.nm.put/facet.num.put.members/put_long_double.hex.pass.cpp b/libcxx/test/std/localization/locale.categories/category.numeric/locale.nm.put/facet.num.put.members/put_long_double.hex.pass.cpp
index b9b02f5ea7b67..51863a30445f6 100644
--- a/libcxx/test/std/localization/locale.categories/category.numeric/locale.nm.put/facet.num.put.members/put_long_double.hex.pass.cpp
+++ b/libcxx/test/std/localization/locale.categories/category.numeric/locale.nm.put/facet.num.put.members/put_long_double.hex.pass.cpp
@@ -1846,11 +1846,26 @@ void test1() {
 void test2() {
   std::locale lc = std::locale::classic();
   std::locale lg(lc, new my_numpunct);
-#if (defined(__APPLE__) || defined(TEST_HAS_GLIBC) || defined(__MINGW32__)) && defined(__x86_64__)
-  // This test is failing on FreeBSD, possibly due to different representations
-  // of the floating point numbers.
-  // This test is failing in MSVC environments, where long double is equal to regular
-  // double, and instead of "0x9.32c05a44p+27", this prints "0x1.26580b4880000p+30".
+#if (defined(__APPLE__) || defined(TEST_HAS_GLIBC) || defined(__MINGW32__)) && defined(__x86_64__) &&                  \
+    __LDBL_MANT_DIG__ == 64
+  // This test assumes that long doubles are x87 80 bit long doubles, and
+  // assumes one specific way of formatting the long doubles. (There are
+  // multiple valid ways of hex formatting the same float.)
+  //
+  // FreeBSD does use x87 80 bit long doubles, but normalizes the hex floats
+  // differently.
+  //
+  // This test assumes the form used by Glibc, Darwin and others, where the
+  // 64 mantissa bits are grouped by nibble as they are stored in the long
+  // double representation (nibble aligned at the end of the least significant
+  // bits). This makes 1.0L to be formatted as "0x8p-3" (where the leading
+  // bit of the mantissa is the higest bit in the 0x8 nibble), and makes
+  // __LDBL_MAX__ be formatted as "0xf.fffffffffffffffp+16380".
+  //
+  // FreeBSD normalizes/aligns the leading bit of the mantissa as a separate
+  // nibble, so that 1.0L is formatted as "0x1p+0" and __LDBL_MAX__ as
+  // "0x1.fffffffffffffffep+16383" (note the lowest bit of the last nibble
+  // being zero as the nibbles don't align to the actual floats).
   const my_facet f(1);
   char str[200];
   {

[philnik777]

Can we drop that defined(__x86_64__) or is there another platform with __LDBL_MANT_DIG__ == 64? Also, would it maybe make sense to put the floating point detection stuff into test_macros.h? That way we could check TEST_LONG_DOUBLE_TYPE == TEST_X86_WHATEVER_IT_IS_CALLED_LONG_DOUBLE_TYPE here, which would probably make it a lot clearer what we care about (and avoid spreading implementation-defined macros throughout the tests).

[mstorsjo]

i386 (both on mingw and on unix platforms) has got __LDBL_MANT_DIG__ == 64. I tested on mingw, and it seems like the tests within this ifdef do pass on i386 mingw too. I don't think we have CI test coverage of i386 Linux or macOS though, but it's likely that they'd behave the same. So we probably can remove the arch check and generalize it to just check for the long double size.

I guess we can move some part of the check to test_macros.h, but I'm not sure how you envision it with TEST_LONG_DOUBLE_TYPE == TEST_X86_WHATEVER_IT_IS_CALLED_LONG_DOUBLE_TYPE. We can do e.g. TEST_LONG_DOUBLE_IS_80_BIT which expands to 0/1 depending on this condition? But that'd just still be a check for __LDBL_MANT_DIG__ == 64, just abstracted with a slightly prettier name.

[philnik777]

Yeah, an abstraction with a slightly prettier name is basically what I'm looking for. Most people don't know that __LDBL_MANT_DIG__ == 64 is the same as "is an 80 bit long double".

[mstorsjo]

Ok, sure - I added such a define there now; it seems to match a preexisting define TEST_LONG_DOUBLE_IS_DOUBLE quite well.