Skip to content

[libc] Simplify the version guard for mpfr. #146354

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.

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 1 commit into from
Jul 8, 2025
Merged

[libc] Simplify the version guard for mpfr. #146354

merged 1 commit into from
Jul 8, 2025

Conversation

c8ef
Copy link
Contributor

@c8ef c8ef commented Jun 30, 2025

Instead of manually calculating the major and minor version numbers, we can directly use MPFR_VERSION_NUM to simplify this.

@llvmbot llvmbot added the libc label Jun 30, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 30, 2025

@llvm/pr-subscribers-libc

Author: Connector Switch (c8ef)

Changes

Instead of manually calculating the major and minor version numbers, we can directly use MPFR_VERSION_NUM to simplify this.


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

1 Files Affected:

  • (modified) libc/utils/MPFRWrapper/MPCommon.cpp (+6-14)
diff --git a/libc/utils/MPFRWrapper/MPCommon.cpp b/libc/utils/MPFRWrapper/MPCommon.cpp
index ccd4d2d01a4e2..441d5420b278b 100644
--- a/libc/utils/MPFRWrapper/MPCommon.cpp
+++ b/libc/utils/MPFRWrapper/MPCommon.cpp
@@ -73,8 +73,7 @@ MPFRNumber MPFRNumber::acosh() const {
 MPFRNumber MPFRNumber::acospi() const {
   MPFRNumber result(*this);
 
-#if MPFR_VERSION_MAJOR > 4 ||                                                  \
-    (MPFR_VERSION_MAJOR == 4 && MPFR_VERSION_MINOR >= 2)
+#if MPFR_VERSION >= MPFR_VERSION_NUM(4, 2, 0)
   mpfr_acospi(result.value, value, mpfr_rounding);
   return result;
 #else
@@ -150,8 +149,7 @@ MPFRNumber MPFRNumber::cosh() const {
 MPFRNumber MPFRNumber::cospi() const {
   MPFRNumber result(*this);
 
-#if MPFR_VERSION_MAJOR > 4 ||                                                  \
-    (MPFR_VERSION_MAJOR == 4 && MPFR_VERSION_MINOR >= 2)
+#if MPFR_VERSION >= MPFR_VERSION_NUM(4, 2, 0)
   mpfr_cospi(result.value, value, mpfr_rounding);
   return result;
 #else
@@ -195,8 +193,7 @@ MPFRNumber MPFRNumber::exp2() const {
 
 MPFRNumber MPFRNumber::exp2m1() const {
   // TODO: Only use mpfr_exp2m1 once CI and buildbots get MPFR >= 4.2.0.
-#if MPFR_VERSION_MAJOR > 4 ||                                                  \
-    (MPFR_VERSION_MAJOR == 4 && MPFR_VERSION_MINOR >= 2)
+#if MPFR_VERSION >= MPFR_VERSION_NUM(4, 2, 0)
   MPFRNumber result(*this);
   mpfr_exp2m1(result.value, value, mpfr_rounding);
   return result;
@@ -231,8 +228,7 @@ MPFRNumber MPFRNumber::exp10() const {
 
 MPFRNumber MPFRNumber::exp10m1() const {
   // TODO: Only use mpfr_exp10m1 once CI and buildbots get MPFR >= 4.2.0.
-#if MPFR_VERSION_MAJOR > 4 ||                                                  \
-    (MPFR_VERSION_MAJOR == 4 && MPFR_VERSION_MINOR >= 2)
+#if MPFR_VERSION >= MPFR_VERSION_NUM(4, 2, 0)
   MPFRNumber result(*this);
   mpfr_exp10m1(result.value, value, mpfr_rounding);
   return result;
@@ -402,9 +398,7 @@ MPFRNumber MPFRNumber::sin() const {
 MPFRNumber MPFRNumber::sinpi() const {
   MPFRNumber result(*this);
 
-#if MPFR_VERSION_MAJOR > 4 ||                                                  \
-    (MPFR_VERSION_MAJOR == 4 && MPFR_VERSION_MINOR >= 2)
-
+#if MPFR_VERSION >= MPFR_VERSION_NUM(4, 2, 0)
   mpfr_sinpi(result.value, value, mpfr_rounding);
   return result;
 #else
@@ -463,9 +457,7 @@ MPFRNumber MPFRNumber::tanh() const {
 MPFRNumber MPFRNumber::tanpi() const {
   MPFRNumber result(*this);
 
-#if MPFR_VERSION_MAJOR > 4 ||                                                  \
-    (MPFR_VERSION_MAJOR == 4 && MPFR_VERSION_MINOR >= 2)
-
+#if MPFR_VERSION >= MPFR_VERSION_NUM(4, 2, 0)
   mpfr_tanpi(result.value, value, mpfr_rounding);
   return result;
 #else

Copy link
Contributor

@michaelrj-google michaelrj-google left a comment

Choose a reason for hiding this comment

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

I don't see any problems with this, approving. If this causes buildbot failures please revert.

@c8ef
Copy link
Contributor Author

c8ef commented Jul 8, 2025

I don't see any problems with this, approving. If this causes buildbot failures please revert.

Thank you! In fact, gcc is already using this: https://github.com/gcc-mirror/gcc/blob/master/gcc/fortran/simplify.cc#L1188, so I don't think it will cause any issues.

@c8ef c8ef merged commit 708c0fe into llvm:main Jul 8, 2025
15 checks passed
@c8ef c8ef deleted the mpfr branch July 8, 2025 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants