[libc][NFC] Make EXP_MANT_MASK an implementation detail
#75810
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-libc Author: Guillaume Chatelet (gchatelet) ChangesFull diff: https://github.com/llvm/llvm-project/pull/75810.diff 14 Files Affected:
diff --git a/libc/src/__support/FPUtil/FPBits.h b/libc/src/__support/FPUtil/FPBits.h
index 7f5dd0fca58d4f..86d80a9af91221 100644
--- a/libc/src/__support/FPUtil/FPBits.h
+++ b/libc/src/__support/FPUtil/FPBits.h
@@ -33,7 +33,11 @@ template <typename T> struct FPBits : private FloatProperties<T> {
"FPBits instantiated with invalid type.");
using typename FloatProperties<T>::StorageType;
using FloatProperties<T>::TOTAL_LEN;
- using FloatProperties<T>::EXP_MANT_MASK;
+
+private:
+ using FloatProperties<T>::EXP_SIG_MASK;
+
+public:
using FloatProperties<T>::EXP_MASK;
using FloatProperties<T>::EXP_BIAS;
using FloatProperties<T>::EXP_LEN;
@@ -146,15 +150,15 @@ template <typename T> struct FPBits : private FloatProperties<T> {
}
LIBC_INLINE constexpr bool is_inf() const {
- return (bits & EXP_MANT_MASK) == EXP_MASK;
+ return (bits & EXP_SIG_MASK) == EXP_MASK;
}
LIBC_INLINE constexpr bool is_nan() const {
- return (bits & EXP_MANT_MASK) > EXP_MASK;
+ return (bits & EXP_SIG_MASK) > EXP_MASK;
}
LIBC_INLINE constexpr bool is_quiet_nan() const {
- return (bits & EXP_MANT_MASK) == (EXP_MASK | QUIET_NAN_MASK);
+ return (bits & EXP_SIG_MASK) == (EXP_MASK | QUIET_NAN_MASK);
}
LIBC_INLINE constexpr bool is_inf_or_nan() const {
diff --git a/libc/src/__support/FPUtil/FloatProperties.h b/libc/src/__support/FPUtil/FloatProperties.h
index 896c29919e2f77..61d9868c37d50f 100644
--- a/libc/src/__support/FPUtil/FloatProperties.h
+++ b/libc/src/__support/FPUtil/FloatProperties.h
@@ -131,6 +131,9 @@ struct FPProperties : public internal::FPBaseProperties<fp_type> {
// The bit pattern that keeps only the *sign* part.
LIBC_INLINE_VAR static constexpr StorageType SIGN_MASK =
mask_trailing_ones<StorageType, SIGN_LEN>() << SIGN_MASK_SHIFT;
+ // The bit pattern that keeps only the *exponent + significand* part.
+ LIBC_INLINE_VAR static constexpr StorageType EXP_SIG_MASK =
+ mask_trailing_ones<StorageType, EXP_LEN + SIG_LEN>();
// The bit pattern that keeps only the *sign + exponent + significand* part.
LIBC_INLINE_VAR static constexpr StorageType FP_MASK =
mask_trailing_ones<StorageType, TOTAL_LEN>();
@@ -162,13 +165,20 @@ struct FPProperties : public internal::FPBaseProperties<fp_type> {
FRACTION_LEN + 1;
LIBC_INLINE_VAR static constexpr StorageType FRACTION_MASK =
mask_trailing_ones<StorageType, FRACTION_LEN>();
- LIBC_INLINE_VAR static constexpr StorageType EXP_MANT_MASK =
- EXP_MASK | SIG_MASK;
// If a number x is a NAN, then it is a quiet NAN if:
// QuietNaNMask & bits(x) != 0
// Else, it is a signalling NAN.
static constexpr StorageType QUIET_NAN_MASK = QNAN_MASK;
+
+ //---------------------------------------------------------------------------
+ // Modifiers
+ //---------------------------------------------------------------------------
+
+ // Returns the absolute value of 'value'.
+ LIBC_INLINE static constexpr StorageType abs(StorageType value) {
+ return value & EXP_SIG_MASK;
+ }
};
//-----------------------------------------------------------------------------
diff --git a/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h b/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
index 89c47063ebac4d..b6054b4d61e937 100644
--- a/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
+++ b/libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
@@ -29,7 +29,6 @@ namespace fputil {
template <> struct FPBits<long double> : private FloatProperties<long double> {
using typename FloatProperties<long double>::StorageType;
using FloatProperties<long double>::TOTAL_LEN;
- using FloatProperties<long double>::EXP_MANT_MASK;
using FloatProperties<long double>::EXP_MASK;
using FloatProperties<long double>::EXP_BIAS;
using FloatProperties<long double>::EXP_LEN;
diff --git a/libc/src/math/generic/acoshf.cpp b/libc/src/math/generic/acoshf.cpp
index 142c17795d083a..708653a5c5f7ce 100644
--- a/libc/src/math/generic/acoshf.cpp
+++ b/libc/src/math/generic/acoshf.cpp
@@ -20,6 +20,7 @@ namespace LIBC_NAMESPACE {
LLVM_LIBC_FUNCTION(float, acoshf, (float x)) {
using FPBits_t = typename fputil::FPBits<float>;
+ using FloatProp = typename fputil::FloatProperties<float>;
FPBits_t xbits(x);
uint32_t x_u = xbits.uintval();
@@ -34,7 +35,7 @@ LLVM_LIBC_FUNCTION(float, acoshf, (float x)) {
if (LIBC_UNLIKELY(x_u >= 0x4f8ffb03)) {
// Check for exceptional values.
- uint32_t x_abs = x_u & FPBits_t::EXP_MANT_MASK;
+ uint32_t x_abs = FloatProp::abs(x_u);
if (LIBC_UNLIKELY(x_abs >= 0x7f80'0000U)) {
// x is +inf or NaN.
return x;
diff --git a/libc/src/math/generic/asinhf.cpp b/libc/src/math/generic/asinhf.cpp
index 5b2f63d3fe144e..07adfadc280791 100644
--- a/libc/src/math/generic/asinhf.cpp
+++ b/libc/src/math/generic/asinhf.cpp
@@ -19,9 +19,10 @@ namespace LIBC_NAMESPACE {
LLVM_LIBC_FUNCTION(float, asinhf, (float x)) {
using FPBits_t = typename fputil::FPBits<float>;
+ using FloatProp = typename fputil::FloatProperties<float>;
FPBits_t xbits(x);
uint32_t x_u = xbits.uintval();
- uint32_t x_abs = x_u & FPBits_t::EXP_MANT_MASK;
+ uint32_t x_abs = FloatProp::abs(x_u);
// |x| <= 2^-3
if (LIBC_UNLIKELY(x_abs <= 0x3e80'0000U)) {
diff --git a/libc/src/math/generic/atanhf.cpp b/libc/src/math/generic/atanhf.cpp
index dfec28e9a44a72..3b3fdcb34f4b5e 100644
--- a/libc/src/math/generic/atanhf.cpp
+++ b/libc/src/math/generic/atanhf.cpp
@@ -15,9 +15,10 @@ namespace LIBC_NAMESPACE {
LLVM_LIBC_FUNCTION(float, atanhf, (float x)) {
using FPBits = typename fputil::FPBits<float>;
+ using FloatProp = typename fputil::FloatProperties<float>;
FPBits xbits(x);
bool sign = xbits.get_sign();
- uint32_t x_abs = xbits.uintval() & FPBits::EXP_MANT_MASK;
+ uint32_t x_abs = FloatProp::abs(xbits.uintval());
// |x| >= 1.0
if (LIBC_UNLIKELY(x_abs >= 0x3F80'0000U)) {
diff --git a/libc/src/math/generic/exp.cpp b/libc/src/math/generic/exp.cpp
index ebfd14dd6cc166..badef28f6c0278 100644
--- a/libc/src/math/generic/exp.cpp
+++ b/libc/src/math/generic/exp.cpp
@@ -178,7 +178,7 @@ double set_exceptional(double x) {
FPBits xbits(x);
uint64_t x_u = xbits.uintval();
- uint64_t x_abs = x_u & FloatProp::EXP_MANT_MASK;
+ uint64_t x_abs = FloatProp::abs(x_u);
// |x| <= 2^-53
if (x_abs <= 0x3ca0'0000'0000'0000ULL) {
diff --git a/libc/src/math/generic/exp10.cpp b/libc/src/math/generic/exp10.cpp
index 4e1babcee541bd..3c6e29b37448a9 100644
--- a/libc/src/math/generic/exp10.cpp
+++ b/libc/src/math/generic/exp10.cpp
@@ -225,7 +225,7 @@ double set_exceptional(double x) {
FPBits xbits(x);
uint64_t x_u = xbits.uintval();
- uint64_t x_abs = x_u & FloatProp::EXP_MANT_MASK;
+ uint64_t x_abs = FloatProp::abs(x_u);
// |x| < log10(1 + 2^-53)
if (x_abs <= 0x3c8bcb7b1526e50e) {
diff --git a/libc/src/math/generic/exp2.cpp b/libc/src/math/generic/exp2.cpp
index 07691ca0e7b62a..7574f1777d27c6 100644
--- a/libc/src/math/generic/exp2.cpp
+++ b/libc/src/math/generic/exp2.cpp
@@ -200,7 +200,7 @@ double set_exceptional(double x) {
FPBits xbits(x);
uint64_t x_u = xbits.uintval();
- uint64_t x_abs = x_u & FloatProp::EXP_MANT_MASK;
+ uint64_t x_abs = FloatProp::abs(x_u);
// |x| < log2(1 + 2^-53)
if (x_abs <= 0x3ca71547652b82fd) {
diff --git a/libc/src/math/generic/expm1.cpp b/libc/src/math/generic/expm1.cpp
index a0d47f00828ce4..00ae6743e6b4ab 100644
--- a/libc/src/math/generic/expm1.cpp
+++ b/libc/src/math/generic/expm1.cpp
@@ -223,7 +223,7 @@ double set_exceptional(double x) {
FPBits xbits(x);
uint64_t x_u = xbits.uintval();
- uint64_t x_abs = x_u & FloatProp::EXP_MANT_MASK;
+ uint64_t x_abs = FloatProp::abs(x_u);
// |x| <= 2^-53.
if (x_abs <= 0x3ca0'0000'0000'0000ULL) {
diff --git a/libc/src/math/generic/inv_trigf_utils.h b/libc/src/math/generic/inv_trigf_utils.h
index 588ebbfa71aeb0..1e44ded84b9764 100644
--- a/libc/src/math/generic/inv_trigf_utils.h
+++ b/libc/src/math/generic/inv_trigf_utils.h
@@ -38,6 +38,7 @@ extern const double ATAN_K[5];
// x should be positive, normal finite value
LIBC_INLINE double atan_eval(double x) {
using FPB = fputil::FPBits<double>;
+ using FloatProp = typename fputil::FloatProperties<double>;
// Added some small value to umin and umax mantissa to avoid possible rounding
// errors.
FPB::StorageType umin =
@@ -50,7 +51,7 @@ LIBC_INLINE double atan_eval(double x) {
FPB bs(x);
bool sign = bs.get_sign();
- auto x_abs = bs.uintval() & FPB::EXP_MANT_MASK;
+ auto x_abs = FloatProp::abs(bs.uintval());
if (x_abs <= umin) {
double pe = LIBC_NAMESPACE::fputil::polyeval(
diff --git a/libc/src/math/generic/powf.cpp b/libc/src/math/generic/powf.cpp
index dd7fa7f6115d4e..a4d29a2c31dcd4 100644
--- a/libc/src/math/generic/powf.cpp
+++ b/libc/src/math/generic/powf.cpp
@@ -517,9 +517,9 @@ LLVM_LIBC_FUNCTION(float, powf, (float x, float y)) {
FloatBits xbits(x), ybits(y);
uint32_t x_u = xbits.uintval();
- uint32_t x_abs = x_u & FloatProp::EXP_MANT_MASK;
+ uint32_t x_abs = FloatProp::abs(x_u);
uint32_t y_u = ybits.uintval();
- uint32_t y_abs = y_u & FloatProp::EXP_MANT_MASK;
+ uint32_t y_abs = FloatProp::abs(y_u);
///////// BEGIN - Check exceptional cases ////////////////////////////////////
diff --git a/libc/src/math/generic/sinhf.cpp b/libc/src/math/generic/sinhf.cpp
index db6794620b068c..fc5e760f269faa 100644
--- a/libc/src/math/generic/sinhf.cpp
+++ b/libc/src/math/generic/sinhf.cpp
@@ -16,8 +16,9 @@ namespace LIBC_NAMESPACE {
LLVM_LIBC_FUNCTION(float, sinhf, (float x)) {
using FPBits = typename fputil::FPBits<float>;
+ using FloatProp = typename fputil::FloatProperties<float>;
FPBits xbits(x);
- uint32_t x_abs = xbits.uintval() & FPBits::EXP_MANT_MASK;
+ uint32_t x_abs = FloatProp::abs(xbits.uintval());
// When |x| >= 90, or x is inf or nan
if (LIBC_UNLIKELY(x_abs >= 0x42b4'0000U || x_abs <= 0x3da0'0000U)) {
diff --git a/libc/src/math/generic/tanhf.cpp b/libc/src/math/generic/tanhf.cpp
index 9042a41c5ed3fe..71ed6097b039db 100644
--- a/libc/src/math/generic/tanhf.cpp
+++ b/libc/src/math/generic/tanhf.cpp
@@ -22,9 +22,10 @@ constexpr double LOG2_E_EXP2_6 = ExpBase::LOG2_B * 2.0;
LLVM_LIBC_FUNCTION(float, tanhf, (float x)) {
using FPBits = typename fputil::FPBits<float>;
+ using FloatProp = typename fputil::FloatProperties<float>;
FPBits xbits(x);
uint32_t x_u = xbits.uintval();
- uint32_t x_abs = x_u & FPBits::EXP_MANT_MASK;
+ uint32_t x_abs = FloatProp::abs(x_u);
// When |x| >= 15, or x is inf or nan, or |x| <= 0.078125
if (LIBC_UNLIKELY((x_abs >= 0x4170'0000U) || (x_abs <= 0x3da0'0000U))) {
|
| //--------------------------------------------------------------------------- | ||
|
|
||
| // Returns the absolute value of 'value'. | ||
| LIBC_INLINE static constexpr StorageType abs(StorageType value) { |
There was a problem hiding this comment.
It's weird to have a member function in a class named FloatProperties. This should probably be a free function with template properties.
Actually, I'm noting that this is aways called on the result of calling FPBits::uintval(), maybe it's a sign that this should be a member function of FPBits ?
There was a problem hiding this comment.
Fair enough, I wanted to unify FloatProperties and FPBits at some point but it's probably better to do this afterwards. It is clear that FPBits is a better place for abs right now. Thx for the suggestion.
This mask is an implementation detail of `FPBits` and shouldn't really leak outside of it.
gchatelet
force-pushed
the
make__exp_mant_mask__an_implementation_detail
branch
from
1c7647d to
698bb26
Compare
legrosbuffle
approved these changes
Dec 19, 2023
gchatelet
changed the title
EXP_MANT_MASK an implementation detail
|
Submitting this now to move forward. @lntue let me know if you have any concerns with this patch. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This mask is an implementation detail of
FPBitsand shouldn't really leak outside of it.