[libc-commits] [libc] Revert "[libc] `FPRep` builders return `FPRep` instead of raw `StorageType`" (PR #78974)
via libc-commits
libc-commits at lists.llvm.org
Mon Jan 22 06:05:22 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libc
Author: Guillaume Chatelet (gchatelet)
<details>
<summary>Changes</summary>
Reverts llvm/llvm-project#<!-- -->78588
---
Patch is 46.41 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/78974.diff
2 Files Affected:
- (modified) libc/src/__support/FPUtil/FPBits.h (+281-312)
- (modified) libc/test/src/__support/FPUtil/fpbits_test.cpp (+84-87)
``````````diff
diff --git a/libc/src/__support/FPUtil/FPBits.h b/libc/src/__support/FPUtil/FPBits.h
index bc6b19b6ebf1089..be700285de82854 100644
--- a/libc/src/__support/FPUtil/FPBits.h
+++ b/libc/src/__support/FPUtil/FPBits.h
@@ -64,46 +64,38 @@ LIBC_INLINE_VAR constexpr Sign Sign::POS = Sign(false);
// └─────────▲─────────┘
// │
// ┌─────────┴─────────┐
-// │ FPStorage<FPType> │
+// │ FPRepBase<FPType> │
// └─────────▲─────────┘
// │
// ┌────────────┴─────────────┐
// │ │
-// ┌────────┴─────────┐ ┌──────────────┴──────────────────┐
-// │ FPRepSem<FPType> │ │ FPRepSem<FPType::X86_Binary80 │
-// └────────▲─────────┘ └──────────────▲──────────────────┘
+// ┌────────┴──────┐ ┌─────────────┴──────────────┐
+// │ FPRep<FPType> │ │ FPRep<FPType::X86_Binary80 │
+// └────────▲──────┘ └─────────────▲──────────────┘
// │ │
// └────────────┬─────────────┘
// │
// ┌─────┴─────┐
-// │ FPRep<T> │
-// └───────────┘
-// │
-// ┌─────┴─────┐
// │ FPBits<T> │
// └───────────┘
//
-// - 'FPLayout' defines only a few constants, namely the 'StorageType' and
-// length of the sign, the exponent, fraction and significand parts.
-// - 'FPStorage' builds more constants on top of those from 'FPLayout' like
-// exponent bias and masks. It also holds the bit representation of the
-// floating point as a 'StorageType' type and defines tools to assemble or test
+// - 'FPLayout' defines only a few constants, namely the 'StorageType' and the
+// length of the sign, the exponent and significand parts.
+// - 'FPRepBase' builds more constants on top of those from 'FPLayout' like
+// exponent bias, shifts and masks. It also defines tools to assemble or test
// these parts.
-// - 'FPRepSem' defines functions to interact semantically with the floating
-// point representation. The default implementation is the one for 'IEEE754', a
-// specialization is provided for X86 Extended Precision.
-// - 'FPRep' derives from 'FPRepSem' and adds functions that are common to all
-// implementations.
-// - 'FPBits' exposes all functions from 'FPRep' but operates on the native C++
-// floating point type instead of 'FPType'.
+// - 'FPRep' defines functions to interact with the floating point
+// representation. The default implementation is the one for 'IEEE754', a
+// specialization is provided for X86 Extended Precision that has a different
+// encoding.
+// - 'FPBits' is templated on the platform floating point types. Contrary to
+// 'FPRep' that is platform agnostic 'FPBits' is architecture dependent.
namespace internal {
// Defines the layout (sign, exponent, significand) of a floating point type in
// memory. It also defines its associated StorageType, i.e., the unsigned
// integer type used to manipulate its representation.
-// Additionally we provide the fractional part length, i.e., the number of bits
-// after the decimal dot when the number is in normal form.
template <FPType> struct FPLayout {};
template <> struct FPLayout<FPType::IEEE754_Binary16> {
@@ -111,7 +103,6 @@ template <> struct FPLayout<FPType::IEEE754_Binary16> {
LIBC_INLINE_VAR static constexpr int SIGN_LEN = 1;
LIBC_INLINE_VAR static constexpr int EXP_LEN = 5;
LIBC_INLINE_VAR static constexpr int SIG_LEN = 10;
- LIBC_INLINE_VAR static constexpr int FRACTION_LEN = SIG_LEN;
};
template <> struct FPLayout<FPType::IEEE754_Binary32> {
@@ -119,7 +110,6 @@ template <> struct FPLayout<FPType::IEEE754_Binary32> {
LIBC_INLINE_VAR static constexpr int SIGN_LEN = 1;
LIBC_INLINE_VAR static constexpr int EXP_LEN = 8;
LIBC_INLINE_VAR static constexpr int SIG_LEN = 23;
- LIBC_INLINE_VAR static constexpr int FRACTION_LEN = SIG_LEN;
};
template <> struct FPLayout<FPType::IEEE754_Binary64> {
@@ -127,7 +117,6 @@ template <> struct FPLayout<FPType::IEEE754_Binary64> {
LIBC_INLINE_VAR static constexpr int SIGN_LEN = 1;
LIBC_INLINE_VAR static constexpr int EXP_LEN = 11;
LIBC_INLINE_VAR static constexpr int SIG_LEN = 52;
- LIBC_INLINE_VAR static constexpr int FRACTION_LEN = SIG_LEN;
};
template <> struct FPLayout<FPType::IEEE754_Binary128> {
@@ -135,7 +124,6 @@ template <> struct FPLayout<FPType::IEEE754_Binary128> {
LIBC_INLINE_VAR static constexpr int SIGN_LEN = 1;
LIBC_INLINE_VAR static constexpr int EXP_LEN = 15;
LIBC_INLINE_VAR static constexpr int SIG_LEN = 112;
- LIBC_INLINE_VAR static constexpr int FRACTION_LEN = SIG_LEN;
};
template <> struct FPLayout<FPType::X86_Binary80> {
@@ -143,22 +131,23 @@ template <> struct FPLayout<FPType::X86_Binary80> {
LIBC_INLINE_VAR static constexpr int SIGN_LEN = 1;
LIBC_INLINE_VAR static constexpr int EXP_LEN = 15;
LIBC_INLINE_VAR static constexpr int SIG_LEN = 64;
- LIBC_INLINE_VAR static constexpr int FRACTION_LEN = SIG_LEN - 1;
};
-// FPStorage derives useful constants from the FPLayout above.
-template <FPType fp_type> struct FPStorage : public FPLayout<fp_type> {
- using UP = FPLayout<fp_type>;
+} // namespace internal
+
+// FPRepBase derives useful constants from the FPLayout.
+template <FPType fp_type>
+struct FPRepBase : public internal::FPLayout<fp_type> {
+private:
+ using UP = internal::FPLayout<fp_type>;
+public:
using UP::EXP_LEN; // The number of bits for the *exponent* part
using UP::SIG_LEN; // The number of bits for the *significand* part
using UP::SIGN_LEN; // The number of bits for the *sign* part
// For convenience, the sum of `SIG_LEN`, `EXP_LEN`, and `SIGN_LEN`.
LIBC_INLINE_VAR static constexpr int TOTAL_LEN = SIGN_LEN + EXP_LEN + SIG_LEN;
- // The number of bits after the decimal dot when the number is in normal form.
- using UP::FRACTION_LEN;
-
// An unsigned integer that is wide enough to contain all of the floating
// point bits.
using StorageType = typename UP::StorageType;
@@ -173,30 +162,41 @@ template <FPType fp_type> struct FPStorage : public FPLayout<fp_type> {
(1U << (EXP_LEN - 1U)) - 1U;
static_assert(EXP_BIAS > 0);
+protected:
+ // The shift amount to get the *significand* part to the least significant
+ // bit. Always `0` but kept for consistency.
+ LIBC_INLINE_VAR static constexpr int SIG_MASK_SHIFT = 0;
+ // The shift amount to get the *exponent* part to the least significant bit.
+ LIBC_INLINE_VAR static constexpr int EXP_MASK_SHIFT = SIG_LEN;
+ // The shift amount to get the *sign* part to the least significant bit.
+ LIBC_INLINE_VAR static constexpr int SIGN_MASK_SHIFT = SIG_LEN + EXP_LEN;
+
// The bit pattern that keeps only the *significand* part.
LIBC_INLINE_VAR static constexpr StorageType SIG_MASK =
- mask_trailing_ones<StorageType, SIG_LEN>();
+ mask_trailing_ones<StorageType, SIG_LEN>() << SIG_MASK_SHIFT;
+
+public:
// The bit pattern that keeps only the *exponent* part.
LIBC_INLINE_VAR static constexpr StorageType EXP_MASK =
- mask_trailing_ones<StorageType, EXP_LEN>() << SIG_LEN;
+ mask_trailing_ones<StorageType, EXP_LEN>() << EXP_MASK_SHIFT;
// The bit pattern that keeps only the *sign* part.
LIBC_INLINE_VAR static constexpr StorageType SIGN_MASK =
- mask_trailing_ones<StorageType, SIGN_LEN>() << (EXP_LEN + SIG_LEN);
+ 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>();
- // The bit pattern that keeps only the *fraction* part.
- // i.e., the *significand* without the leading one.
- LIBC_INLINE_VAR static constexpr StorageType FRACTION_MASK =
- mask_trailing_ones<StorageType, FRACTION_LEN>();
static_assert((SIG_MASK & EXP_MASK & SIGN_MASK) == 0, "masks disjoint");
static_assert((SIG_MASK | EXP_MASK | SIGN_MASK) == FP_MASK, "masks cover");
protected:
+ LIBC_INLINE static constexpr StorageType bit_at(int position) {
+ return StorageType(1) << position;
+ }
+
// A stongly typed integer that prevents mixing and matching integers with
// different semantics.
template <typename T> struct TypedInt {
@@ -248,7 +248,7 @@ template <FPType fp_type> struct FPStorage : public FPLayout<fp_type> {
// An opaque type to store a floating point significand.
// We define special values but it is valid to create arbitrary values as long
- // as they are in the range [ZERO, BITS_ALL_ONES].
+ // as they are in the range [BITS_ALL_ZEROES, BITS_ALL_ONES].
// Note that the semantics of the Significand are implementation dependent.
// Values greater than BITS_ALL_ONES are truncated.
struct Significand : public TypedInt<StorageType> {
@@ -277,8 +277,10 @@ template <FPType fp_type> struct FPStorage : public FPLayout<fp_type> {
return Significand(StorageType(1));
}
LIBC_INLINE static constexpr auto MSB() {
- return Significand(StorageType(1) << (SIG_LEN - 1));
+ return Significand(StorageType(bit_at(SIG_LEN - 1)));
}
+ // Aliases
+ LIBC_INLINE static constexpr auto BITS_ALL_ZEROES() { return ZERO(); }
LIBC_INLINE static constexpr auto BITS_ALL_ONES() {
return Significand(SIG_MASK);
}
@@ -304,95 +306,181 @@ template <FPType fp_type> struct FPStorage : public FPLayout<fp_type> {
return encode(exp, sig);
}
- // The floating point number representation as an unsigned integer.
- StorageType bits{};
-
- LIBC_INLINE constexpr FPStorage() : bits(0) {}
- LIBC_INLINE constexpr FPStorage(StorageType value) : bits(value) {}
-
- // Observers
LIBC_INLINE constexpr StorageType exp_bits() const { return bits & EXP_MASK; }
LIBC_INLINE constexpr StorageType sig_bits() const { return bits & SIG_MASK; }
LIBC_INLINE constexpr StorageType exp_sig_bits() const {
return bits & EXP_SIG_MASK;
}
-};
-// This layer defines all functions that are specific to how the the floating
-// point type is encoded. It enables constructions, modification and observation
-// of values manipulated as 'StorageType'.
-template <FPType fp_type, typename RetT>
-struct FPRepSem : public FPStorage<fp_type> {
- using UP = FPStorage<fp_type>;
- using typename UP::StorageType;
- using UP::FRACTION_LEN;
- using UP::FRACTION_MASK;
+private:
+ // Merge bits from 'a' and 'b' values according to 'mask'.
+ // Use 'a' bits when corresponding 'mask' bits are zeroes and 'b' bits when
+ // corresponding bits are ones.
+ LIBC_INLINE static constexpr StorageType merge(StorageType a, StorageType b,
+ StorageType mask) {
+ // https://graphics.stanford.edu/~seander/bithacks.html#MaskedMerge
+ return a ^ ((a ^ b) & mask);
+ }
protected:
- using BiasedExp = typename UP::BiasedExponent;
- using Exp = typename UP::Exponent;
- using Sig = typename UP::Significand;
- using UP::encode;
- using UP::exp_bits;
- using UP::exp_sig_bits;
- using UP::sig_bits;
- using UP::UP;
+ // The number of bits after the decimal dot when the number is in normal form.
+ LIBC_INLINE_VAR static constexpr int FRACTION_LEN =
+ fp_type == FPType::X86_Binary80 ? SIG_LEN - 1 : SIG_LEN;
+ LIBC_INLINE_VAR static constexpr uint32_t MANTISSA_PRECISION =
+ FRACTION_LEN + 1;
+ LIBC_INLINE_VAR static constexpr StorageType FRACTION_MASK =
+ mask_trailing_ones<StorageType, FRACTION_LEN>();
+
+ // The floating point number representation as an unsigned integer.
+ StorageType bits = 0;
public:
- // Builders
- LIBC_INLINE static constexpr RetT one(Sign sign = Sign::POS) {
- return RetT(encode(sign, Exp::ZERO(), Sig::ZERO()));
+ LIBC_INLINE constexpr Sign sign() const {
+ return (bits & SIGN_MASK) ? Sign::NEG : Sign::POS;
}
- LIBC_INLINE static constexpr RetT min_subnormal(Sign sign = Sign::POS) {
- return RetT(encode(sign, BiasedExp::BITS_ALL_ZEROES(), Sig::LSB()));
+
+ LIBC_INLINE constexpr void set_sign(Sign signVal) {
+ if (sign() != signVal)
+ bits ^= SIGN_MASK;
}
- LIBC_INLINE static constexpr RetT max_subnormal(Sign sign = Sign::POS) {
- return RetT(
- encode(sign, BiasedExp::BITS_ALL_ZEROES(), Sig::BITS_ALL_ONES()));
+
+ LIBC_INLINE constexpr StorageType get_mantissa() const {
+ return bits & FRACTION_MASK;
+ }
+
+ LIBC_INLINE constexpr void set_mantissa(StorageType mantVal) {
+ bits = merge(bits, mantVal, FRACTION_MASK);
+ }
+
+ LIBC_INLINE constexpr uint16_t get_biased_exponent() const {
+ return uint16_t((bits & EXP_MASK) >> EXP_MASK_SHIFT);
}
- LIBC_INLINE static constexpr RetT min_normal(Sign sign = Sign::POS) {
- return RetT(encode(sign, Exp::MIN(), Sig::ZERO()));
+
+ LIBC_INLINE constexpr void set_biased_exponent(StorageType biased) {
+ bits = merge(bits, biased << EXP_MASK_SHIFT, EXP_MASK);
}
- LIBC_INLINE static constexpr RetT max_normal(Sign sign = Sign::POS) {
- return RetT(encode(sign, Exp::MAX(), Sig::BITS_ALL_ONES()));
+
+ LIBC_INLINE constexpr int get_exponent() const {
+ return int(get_biased_exponent()) - EXP_BIAS;
}
- LIBC_INLINE static constexpr RetT inf(Sign sign = Sign::POS) {
- return RetT(encode(sign, BiasedExp::BITS_ALL_ONES(), Sig::ZERO()));
+
+ // If the number is subnormal, the exponent is treated as if it were the
+ // minimum exponent for a normal number. This is to keep continuity between
+ // the normal and subnormal ranges, but it causes problems for functions where
+ // values are calculated from the exponent, since just subtracting the bias
+ // will give a slightly incorrect result. Additionally, zero has an exponent
+ // of zero, and that should actually be treated as zero.
+ LIBC_INLINE constexpr int get_explicit_exponent() const {
+ const int biased_exp = int(get_biased_exponent());
+ if (is_zero()) {
+ return 0;
+ } else if (biased_exp == 0) {
+ return 1 - EXP_BIAS;
+ } else {
+ return biased_exp - EXP_BIAS;
+ }
}
- LIBC_INLINE static constexpr RetT build_nan(Sign sign = Sign::POS,
- StorageType v = 0) {
- return RetT(encode(sign, BiasedExp::BITS_ALL_ONES(),
- (v ? Sig(v) : (Sig::MSB() >> 1))));
+
+ LIBC_INLINE constexpr StorageType uintval() const { return bits & FP_MASK; }
+ LIBC_INLINE constexpr void set_uintval(StorageType value) {
+ bits = (value & FP_MASK);
}
- LIBC_INLINE static constexpr RetT build_quiet_nan(Sign sign = Sign::POS,
- StorageType v = 0) {
- return RetT(encode(sign, BiasedExp::BITS_ALL_ONES(), Sig::MSB() | Sig(v)));
+
+ LIBC_INLINE constexpr bool is_zero() const { return exp_sig_bits() == 0; }
+
+ LIBC_INLINE
+ constexpr bool is_subnormal() const {
+ return exp_bits() == encode(BiasedExponent::BITS_ALL_ZEROES());
}
- // Observers
+ LIBC_INLINE constexpr bool is_neg() const { return sign().is_neg(); }
+ LIBC_INLINE constexpr bool is_pos() const { return sign().is_pos(); }
+};
+
+namespace internal {
+
+// Manipulates the representation of a floating point number defined by its
+// FPType. This layer is architecture agnostic and does not handle C++ floating
+// point types directly ('float', 'double' and 'long double'). Use the FPBits
+// below if needed.
+//
+// TODO: Specialize this class for FPType::X86_Binary80 and remove ad-hoc logic
+// from FPRepBase.
+template <FPType fp_type> struct FPRep : public FPRepBase<fp_type> {
+ using UP = FPRepBase<fp_type>;
+ using typename UP::StorageType;
+ using UP::FRACTION_LEN;
+ using UP::FRACTION_MASK;
+ using UP::MANTISSA_PRECISION;
+
+protected:
+ using typename UP::BiasedExponent;
+ using typename UP::Exponent;
+ using typename UP::Significand;
+ using UP::encode;
+ using UP::exp_bits;
+ using UP::exp_sig_bits;
+ using UP::sig_bits;
+
+public:
LIBC_INLINE constexpr bool is_nan() const {
- return exp_sig_bits() > encode(BiasedExp::BITS_ALL_ONES(), Sig::ZERO());
+ return exp_sig_bits() >
+ encode(BiasedExponent::BITS_ALL_ONES(), Significand::ZERO());
}
LIBC_INLINE constexpr bool is_quiet_nan() const {
- return exp_sig_bits() >= encode(BiasedExp::BITS_ALL_ONES(), Sig::MSB());
+ return exp_sig_bits() >=
+ encode(BiasedExponent::BITS_ALL_ONES(), Significand::MSB());
}
LIBC_INLINE constexpr bool is_signaling_nan() const {
return is_nan() && !is_quiet_nan();
}
LIBC_INLINE constexpr bool is_inf() const {
- return exp_sig_bits() == encode(BiasedExp::BITS_ALL_ONES(), Sig::ZERO());
+ return exp_sig_bits() ==
+ encode(BiasedExponent::BITS_ALL_ONES(), Significand::ZERO());
}
LIBC_INLINE constexpr bool is_finite() const {
- return exp_bits() != encode(BiasedExp::BITS_ALL_ONES());
- }
- LIBC_INLINE
- constexpr bool is_subnormal() const {
- return exp_bits() == encode(BiasedExp::BITS_ALL_ZEROES());
+ return exp_bits() != encode(BiasedExponent::BITS_ALL_ONES());
}
LIBC_INLINE constexpr bool is_normal() const {
return is_finite() && !UP::is_subnormal();
}
- // Returns the mantissa with the implicit bit set iff the current
+
+ LIBC_INLINE static constexpr StorageType zero(Sign sign = Sign::POS) {
+ return encode(sign, BiasedExponent::BITS_ALL_ZEROES(), Significand::ZERO());
+ }
+ LIBC_INLINE static constexpr StorageType one(Sign sign = Sign::POS) {
+ return encode(sign, Exponent::ZERO(), Significand::ZERO());
+ }
+ LIBC_INLINE static constexpr StorageType
+ min_subnormal(Sign sign = Sign::POS) {
+ return encode(sign, BiasedExponent::BITS_ALL_ZEROES(), Significand::LSB());
+ }
+ LIBC_INLINE static constexpr StorageType
+ max_subnormal(Sign sign = Sign::POS) {
+ return encode(sign, BiasedExponent::BITS_ALL_ZEROES(),
+ Significand::BITS_ALL_ONES());
+ }
+ LIBC_INLINE static constexpr StorageType min_normal(Sign sign = Sign::POS) {
+ return encode(sign, Exponent::MIN(), Significand::ZERO());
+ }
+ LIBC_INLINE static constexpr StorageType max_normal(Sign sign = Sign::POS) {
+ return encode(sign, Exponent::MAX(), Significand::BITS_ALL_ONES());
+ }
+ LIBC_INLINE static constexpr StorageType inf(Sign sign = Sign::POS) {
+ return encode(sign, BiasedExponent::BITS_ALL_ONES(), Significand::ZERO());
+ }
+ LIBC_INLINE static constexpr StorageType build_nan(Sign sign = Sign::POS,
+ StorageType v = 0) {
+ return encode(sign, BiasedExponent::BITS_ALL_ONES(),
+ (v ? Significand(v) : (Significand::MSB() >> 1)));
+ }
+ LIBC_INLINE static constexpr StorageType
+ build_quiet_nan(Sign sign = Sign::POS, StorageType v = 0) {
+ return encode(sign, BiasedExponent::BITS_ALL_ONES(),
+ Significand::MSB() | Significand(v));
+ }
+
+ // The function return mantissa with the implicit bit set iff the current
// value is a valid normal number.
LIBC_INLINE constexpr StorageType get_explicit_mantissa() {
if (UP::is_subnormal())
@@ -402,14 +490,20 @@ struct FPRepSem : public FPStorage<fp_type> {
};
// Specialization for the X86 Extended Precision type.
-template <typename RetT>
-struct FPRepSem<FPType::X86_Binary80, RetT>
- : public FPStorage<FPType::X86_Binary80> {
- using UP = FPStorage<FPType::X86_Binary80>;
+template <>
+struct FPRep<FPType::X86_Binary80> : public FPRepBase<FPType::X86_Binary80> {
+ using UP = FPRepBase<FPType::X86_Binary80>;
using typename UP::StorageType;
using UP::FRACTION_LEN;
using UP::FRACTION_MASK;
+ using UP::MANTISSA_PRECISION;
+
+protected:
+ using typename UP::BiasedExponent;
+ using typename UP::Significand;
+ using UP::encode;
+public:
// The x86 80 bit float represents the leading digit of the mantissa
// explicitly. This is the mask for that bit.
static constexpr StorageType EXPLICIT_BIT_MASK = StorageType(1)
@@ -421,45 +515,6 @@ struct FPRepSem<FPType::X86_Binary80, RetT>
"the explicit bit and the fractional part should cover the "
"whole significand");
-protected:
- using BiasedExp = ty...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/78974
More information about the libc-commits
mailing list