Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 25, 2025
Merged

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

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
298c2c6
[libcxx] [test] Clarify the condition for long double hex formatting
mstorsjo Mar 12, 2025
c8bf9d0
Add a test define TEST_LONG_DOUBLE_IS_80_BIT in test_macros.h
mstorsjo Apr 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions ...egories/category.numeric/locale.nm.put/facet.num.put.members/put_long_double.hex.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

@philnik777 philnik777 Apr 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Member Author

@mstorsjo mstorsjo Apr 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@philnik777 philnik777 Apr 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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".

Copy link
Member Author

@mstorsjo mstorsjo Apr 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

// 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];
{
Expand Down
Loading