Skip to content
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

Jump to bottom

Disable numeric_limits specialization for GCC-14 #993

Merged
merged 1 commit into from
Jun 8, 2023
Merged

Disable numeric_limits specialization for GCC-14 #993

merged 1 commit into from
Jun 8, 2023

Conversation

mborland
Copy link
Member

@mborland mborland commented Jun 7, 2023

Closes: #992

@mborland
Copy link
Member Author

mborland commented Jun 7, 2023

I built GCC from source, test_cstdfloat and the minimal reproducer from the example both pass fine with this PR.

@jzmaddock
Copy link
Collaborator

Just checking.... does libstdc++3 encase numeric_limits<__float128> inside a #if? They often do, in which case it might be better to check for whatever internal macro they are using rather than assuming gcc-14 unconditionally supports this.

@mborland
Copy link
Member Author

mborland commented Jun 7, 2023 •
edited
Loading

It does have an #if block, and the diff is here. The two macros they use are already wrapped into defining BOOST_MATH_USE_FLOAT128.

@jzmaddock
Copy link
Collaborator

Ah, nothing useful there then, this all looks good to me then.

@mborland mborland merged commit 26de5b9 into develop Jun 8, 2023
@mborland mborland deleted the float128 branch June 8, 2023 07:08
@mborland mborland mentioned this pull request Jun 8, 2023
Closed
jwakely
jwakely reviewed Dec 12, 2023
View reviewed changes
include/boost/math/cstdfloat/cstdfloat_limits.hpp
@@ -24,7 +24,7 @@
#pragma GCC system_header
#endif

#if defined(BOOST_CSTDFLOAT_HAS_INTERNAL_FLOAT128_T) && defined(BOOST_MATH_USE_FLOAT128) && !defined(BOOST_CSTDFLOAT_NO_LIBQUADMATH_SUPPORT)
#if defined(BOOST_CSTDFLOAT_HAS_INTERNAL_FLOAT128_T) && defined(BOOST_MATH_USE_FLOAT128) && !defined(BOOST_CSTDFLOAT_NO_LIBQUADMATH_SUPPORT) && (!defined(__GNUC__) || (defined(__GNUC__) && __GNUC__ < 14))
Copy link
Contributor

Choose a reason for hiding this comment

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

This check seems wrong. The new numeric_limits<__float128> specialization will be enabled when using new libstdc++ headers with Clang, which does not define __GNUC__ to 14 (it can be told to use any value, but by default it defines it to 4).

Copy link
Contributor

Choose a reason for hiding this comment

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

Checking _GLIBCXX_RELEASE < 14 would be more correct, as that is determined by the libstdc++ headers, not by the compiler including them.

@jwakely
Copy link
Contributor

jwakely commented Dec 12, 2023

It does have an #if block, and the diff is here. The two macros they use are already wrapped into defining BOOST_MATH_USE_FLOAT128.

FWIW the current code uses slightly different conditions:
https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/include/std/limits;h=ec0b7a1ca7bf38b7440b6e94c692cd5937f1bf23;hb=HEAD#l2084

That was changed by https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=6261d10521f9fdc2a43d54b4dc365025288aa8e9

@mborland mborland mentioned this pull request Dec 13, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jwakely jwakely jwakely left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

float128 with gcc master
3 participants
@mborland @jzmaddock @jwakely