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.

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

[libc][NFC] Small abs related simplifications #79858

Merged
merged 2 commits into from
Feb 13, 2024

Conversation

gchatelet
Copy link
Contributor

No description provided.

@gchatelet gchatelet requested a review from lntue January 29, 2024 16:41
@llvmbot llvmbot added the libc label Jan 29, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 29, 2024

@llvm/pr-subscribers-libc

Author: Guillaume Chatelet (gchatelet)

Changes

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

3 Files Affected:

  • (modified) libc/src/__support/FPUtil/BasicOperations.h (+1-3)
  • (modified) libc/src/math/generic/acoshf.cpp (+1-5)
  • (modified) libc/src/math/generic/asinhf.cpp (+1-3)
diff --git a/libc/src/__support/FPUtil/BasicOperations.h b/libc/src/__support/FPUtil/BasicOperations.h
index ccc61a89c5f831f..a19d6d0bef08ff0 100644
--- a/libc/src/__support/FPUtil/BasicOperations.h
+++ b/libc/src/__support/FPUtil/BasicOperations.h
@@ -19,9 +19,7 @@ namespace fputil {
 
 template <typename T, cpp::enable_if_t<cpp::is_floating_point_v<T>, int> = 0>
 LIBC_INLINE T abs(T x) {
-  FPBits<T> bits(x);
-  bits.set_sign(Sign::POS);
-  return bits.get_val();
+  return FPBits<T>(x).abs().get_val();
 }
 
 template <typename T, cpp::enable_if_t<cpp::is_floating_point_v<T>, int> = 0>
diff --git a/libc/src/math/generic/acoshf.cpp b/libc/src/math/generic/acoshf.cpp
index a546cc2268b049d..16328c7b07c3908 100644
--- a/libc/src/math/generic/acoshf.cpp
+++ b/libc/src/math/generic/acoshf.cpp
@@ -33,12 +33,8 @@ LLVM_LIBC_FUNCTION(float, acoshf, (float x)) {
   }
 
   if (LIBC_UNLIKELY(x_u >= 0x4f8ffb03)) {
-    // Check for exceptional values.
-    uint32_t x_abs = xbits.abs().uintval();
-    if (LIBC_UNLIKELY(x_abs >= 0x7f80'0000U)) {
-      // x is +inf or NaN.
+    if (LIBC_UNLIKELY(xbits.is_inf_or_nan()))
       return x;
-    }
 
     // Helper functions to set results for exceptional cases.
     auto round_result_slightly_down = [](float r) -> float {
diff --git a/libc/src/math/generic/asinhf.cpp b/libc/src/math/generic/asinhf.cpp
index ac059910b4ef2ac..6e351786e3eca1c 100644
--- a/libc/src/math/generic/asinhf.cpp
+++ b/libc/src/math/generic/asinhf.cpp
@@ -59,10 +59,8 @@ LLVM_LIBC_FUNCTION(float, asinhf, (float x)) {
   };
 
   if (LIBC_UNLIKELY(x_abs >= 0x4bdd'65a5U)) {
-    if (LIBC_UNLIKELY(x_abs >= 0x7f80'0000U)) {
-      // x is +-inf or nan
+    if (LIBC_UNLIKELY(xbits.is_inf_or_nan()))
       return x;
-    }
 
     // Exceptional cases when x > 2^24.
     switch (x_abs) {

@gchatelet gchatelet changed the title [libc][NFC] Small abs simplifications [libc][NFC] Small abs related simplifications Jan 29, 2024
@nickdesaulniers
Copy link
Member

don't forget to land this

@jhuber6
Copy link
Contributor

jhuber6 commented Feb 12, 2024

don't forget to land this

Can't other people merge PRs on the author's behalf?

@nickdesaulniers
Copy link
Member

don't forget to land this

Can't other people merge PRs on the author's behalf?

Yes; I was hesitant in case there was a reason this hasn't been merged. There might not be, but just in case my personal preference is to defer to the patch author (when they have merge access). I also prefer to leave the dopamine hit for clicking merge to the author of the work. But if other people feel differently, I don't feel strongly about it either way.

@gchatelet
Copy link
Contributor Author

I actually missed @lntue's approval, thx for the ping. Landing this now.

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.

None yet

5 participants