[reland][libc] Remove unnecessary FPBits functions and properties
#79128
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
This patch reduces the surface of `FPBits`.
|
@llvm/pr-subscribers-libc Author: Guillaume Chatelet (gchatelet) Changes
Patch is 99.97 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/79128.diff 69 Files Affected:
diff --git a/libc/fuzzing/stdlib/strtofloat_fuzz.cpp b/libc/fuzzing/stdlib/strtofloat_fuzz.cpp
index affef6fcf549e0..b773043bda67d8 100644
--- a/libc/fuzzing/stdlib/strtofloat_fuzz.cpp
+++ b/libc/fuzzing/stdlib/strtofloat_fuzz.cpp
@@ -28,7 +28,7 @@ using LIBC_NAMESPACE::fputil::FPBits;
// exponent. Subnormals have a lower effective precision since they don't
// necessarily use all of the bits of the mantissa.
template <typename F> inline constexpr int effective_precision(int exponent) {
- const int full_precision = FPBits<F>::MANTISSA_PRECISION;
+ const int full_precision = FPBits<F>::FRACTION_LEN + 1;
// This is intended to be 0 when the exponent is the lowest normal and
// increase as the exponent's magnitude increases.
diff --git a/libc/src/__support/FPUtil/DivisionAndRemainderOperations.h b/libc/src/__support/FPUtil/DivisionAndRemainderOperations.h
index 1798310c3e31e3..ef9593a42b0055 100644
--- a/libc/src/__support/FPUtil/DivisionAndRemainderOperations.h
+++ b/libc/src/__support/FPUtil/DivisionAndRemainderOperations.h
@@ -31,7 +31,7 @@ LIBC_INLINE T remquo(T x, T y, int &q) {
if (ybits.is_nan())
return y;
if (xbits.is_inf() || ybits.is_zero())
- return FPBits<T>::build_quiet_nan(1);
+ return FPBits<T>::build_quiet_nan(fputil::Sign::POS, 1).get_val();
if (xbits.is_zero()) {
q = 0;
diff --git a/libc/src/__support/FPUtil/FPBits.h b/libc/src/__support/FPUtil/FPBits.h
index 2465158bb2cdfc..0a79b505ecbe1c 100644
--- a/libc/src/__support/FPUtil/FPBits.h
+++ b/libc/src/__support/FPUtil/FPBits.h
@@ -390,7 +390,7 @@ struct FPRepSem : public FPStorage<fp_type> {
return exp_bits() == encode(BiasedExp::BITS_ALL_ZEROES());
}
LIBC_INLINE constexpr bool is_normal() const {
- return is_finite() && !UP::is_subnormal();
+ return is_finite() && !is_subnormal();
}
// Returns the mantissa with the implicit bit set iff the current
// value is a valid normal number.
@@ -556,6 +556,14 @@ struct FPRep : public FPRepSem<fp_type, RetT> {
using UP::FRACTION_MASK;
using UP::SIGN_MASK;
+ // Comparison
+ LIBC_INLINE constexpr friend bool operator==(FPRep a, FPRep b) {
+ return a.uintval() == b.uintval();
+ }
+ LIBC_INLINE constexpr friend bool operator!=(FPRep a, FPRep b) {
+ return a.uintval() != b.uintval();
+ }
+
// Representation
LIBC_INLINE constexpr StorageType uintval() const { return bits & FP_MASK; }
LIBC_INLINE constexpr void set_uintval(StorageType value) {
@@ -698,16 +706,6 @@ struct FPBits final : public internal::FPRep<get_fp_type<T>(), FPBits<T>> {
using UP::bits;
// Constants.
- LIBC_INLINE_VAR static constexpr uint32_t MANTISSA_PRECISION =
- UP::FRACTION_LEN + 1;
- LIBC_INLINE_VAR static constexpr StorageType MIN_NORMAL =
- UP::min_normal(Sign::POS).uintval();
- LIBC_INLINE_VAR static constexpr StorageType MAX_NORMAL =
- UP::max_normal(Sign::POS).uintval();
- LIBC_INLINE_VAR static constexpr StorageType MIN_SUBNORMAL =
- UP::min_subnormal(Sign::POS).uintval();
- LIBC_INLINE_VAR static constexpr StorageType MAX_SUBNORMAL =
- UP::max_subnormal(Sign::POS).uintval();
LIBC_INLINE_VAR static constexpr int MAX_BIASED_EXPONENT =
(1 << UP::EXP_LEN) - 1;
@@ -731,37 +729,6 @@ struct FPBits final : public internal::FPRep<get_fp_type<T>(), FPBits<T>> {
LIBC_INLINE constexpr explicit operator T() const { return get_val(); }
- // Methods below this are used by tests.
- // TODO: inline and remove.
- LIBC_INLINE static constexpr T one(Sign sign = Sign::POS) {
- return T(UP::one(sign));
- }
- LIBC_INLINE static constexpr T zero(Sign sign = Sign::POS) {
- return T(UP::zero(sign));
- }
- LIBC_INLINE static constexpr T inf(Sign sign = Sign::POS) {
- return T(UP::inf(sign));
- }
- LIBC_INLINE static constexpr T min_normal() {
- return T(UP::min_normal(Sign::POS));
- }
- LIBC_INLINE static constexpr T max_normal() {
- return T(UP::max_normal(Sign::POS));
- }
- LIBC_INLINE static constexpr T min_denormal() {
- return T(UP::min_subnormal(Sign::POS));
- }
- LIBC_INLINE static constexpr T max_denormal() {
- return T(UP::max_subnormal(Sign::POS));
- }
- LIBC_INLINE static constexpr T build_nan(StorageType v) {
- return T(UP::build_nan(Sign::POS, v));
- }
- LIBC_INLINE static constexpr T build_quiet_nan(StorageType v,
- Sign sign = Sign::POS) {
- return T(UP::build_quiet_nan(sign, v));
- }
-
// TODO: Use an uint32_t for 'biased_exp'.
LIBC_INLINE static constexpr FPBits<T>
create_value(Sign sign, StorageType biased_exp, StorageType mantissa) {
diff --git a/libc/src/__support/FPUtil/Hypot.h b/libc/src/__support/FPUtil/Hypot.h
index c38a40dfb08984..82237dec09e42e 100644
--- a/libc/src/__support/FPUtil/Hypot.h
+++ b/libc/src/__support/FPUtil/Hypot.h
@@ -197,7 +197,7 @@ LIBC_INLINE T hypot(T x, T y) {
if (int round_mode = quick_get_round();
round_mode == FE_TONEAREST || round_mode == FE_UPWARD)
return T(FPBits_t::inf());
- return T(FPBits_t(FPBits_t::MAX_NORMAL));
+ return T(FPBits_t::max_normal());
}
} else {
// For denormal result, we simply move the leading bit of the result to
@@ -254,7 +254,7 @@ LIBC_INLINE T hypot(T x, T y) {
if (out_exp >= FPBits_t::MAX_BIASED_EXPONENT) {
if (round_mode == FE_TONEAREST || round_mode == FE_UPWARD)
return T(FPBits_t::inf());
- return T(FPBits_t(FPBits_t::MAX_NORMAL));
+ return T(FPBits_t::max_normal());
}
}
diff --git a/libc/src/__support/FPUtil/ManipulationFunctions.h b/libc/src/__support/FPUtil/ManipulationFunctions.h
index 81c8281f3c7bbe..d7114625a9b314 100644
--- a/libc/src/__support/FPUtil/ManipulationFunctions.h
+++ b/libc/src/__support/FPUtil/ManipulationFunctions.h
@@ -108,7 +108,7 @@ LIBC_INLINE T logb(T x) {
return x;
} else if (bits.is_inf()) {
// Return positive infinity.
- return T(FPBits<T>::inf(Sign::POS));
+ return T(FPBits<T>::inf());
}
NormalFloat<T> normal(bits);
@@ -127,7 +127,7 @@ LIBC_INLINE T ldexp(T x, int exp) {
// that adding |exp| to it does not lead to integer rollover. But, if |exp|
// value is larger the exponent range for type T, then we can return infinity
// early. Because the result of the ldexp operation can be a subnormal number,
- // we need to accommodate the (mantissaWidht + 1) worth of shift in
+ // we need to accommodate the (mantissaWidth + 1) worth of shift in
// calculating the limit.
int exp_limit = FPBits<T>::MAX_BIASED_EXPONENT + FPBits<T>::FRACTION_LEN + 1;
if (exp > exp_limit)
@@ -164,26 +164,22 @@ LIBC_INLINE T nextafter(T from, U to) {
return static_cast<T>(to);
using StorageType = typename FPBits<T>::StorageType;
- StorageType int_val = from_bits.uintval();
- if (from != FPBits<T>::zero()) {
- if ((static_cast<U>(from) < to) == (from > FPBits<T>::zero())) {
- ++int_val;
+ if (from != T(0)) {
+ if ((static_cast<U>(from) < to) == (from > T(0))) {
+ from_bits = FPBits<T>(StorageType(from_bits.uintval() + 1));
} else {
- --int_val;
+ from_bits = FPBits<T>(StorageType(from_bits.uintval() - 1));
}
} else {
- int_val = FPBits<T>::MIN_SUBNORMAL;
- if (to_bits.is_neg())
- int_val |= FPBits<T>::SIGN_MASK;
+ from_bits = FPBits<T>::min_subnormal(to_bits.sign());
}
- StorageType exponent_bits = int_val & FPBits<T>::EXP_MASK;
- if (exponent_bits == StorageType(0))
+ if (from_bits.is_subnormal())
raise_except_if_required(FE_UNDERFLOW | FE_INEXACT);
- else if (exponent_bits == FPBits<T>::EXP_MASK)
+ else if (from_bits.is_inf())
raise_except_if_required(FE_OVERFLOW | FE_INEXACT);
- return cpp::bit_cast<T>(int_val);
+ return from_bits.get_val();
}
} // namespace fputil
diff --git a/libc/src/__support/FPUtil/NormalFloat.h b/libc/src/__support/FPUtil/NormalFloat.h
index cfa9e141751055..fa4da33b5b17fa 100644
--- a/libc/src/__support/FPUtil/NormalFloat.h
+++ b/libc/src/__support/FPUtil/NormalFloat.h
@@ -215,7 +215,7 @@ template <> LIBC_INLINE NormalFloat<long double>::operator long double() const {
// Max exponent is of the form 0xFF...E. That is why -2 and not -1.
constexpr int MAX_EXPONENT_VALUE = (1 << LDBits::EXP_LEN) - 2;
if (biased_exponent > MAX_EXPONENT_VALUE) {
- return LDBits::inf(sign);
+ return LDBits::inf(sign).get_val();
}
FPBits<long double> result(0.0l);
diff --git a/libc/src/__support/FPUtil/aarch64/FEnvImpl.h b/libc/src/__support/FPUtil/aarch64/FEnvImpl.h
index 23cde88c9c7c5b..e0eec17e038c63 100644
--- a/libc/src/__support/FPUtil/aarch64/FEnvImpl.h
+++ b/libc/src/__support/FPUtil/aarch64/FEnvImpl.h
@@ -155,8 +155,8 @@ LIBC_INLINE int set_except(int excepts) {
LIBC_INLINE int raise_except(int excepts) {
float zero = 0.0f;
float one = 1.0f;
- float largeValue = FPBits<float>::max_normal();
- float smallValue = FPBits<float>::min_normal();
+ float largeValue = FPBits<float>::max_normal().get_val();
+ float smallValue = FPBits<float>::min_normal().get_val();
auto divfunc = [](float a, float b) {
__asm__ __volatile__("ldr s0, %0\n\t"
"ldr s1, %1\n\t"
diff --git a/libc/src/__support/FPUtil/dyadic_float.h b/libc/src/__support/FPUtil/dyadic_float.h
index 5449f5561d5696..888d7ffec241ea 100644
--- a/libc/src/__support/FPUtil/dyadic_float.h
+++ b/libc/src/__support/FPUtil/dyadic_float.h
@@ -93,7 +93,7 @@ template <size_t Bits> struct DyadicFloat {
return 0.0;
// Assume that it is normalized, and output is also normal.
- constexpr uint32_t PRECISION = FPBits<T>::MANTISSA_PRECISION;
+ constexpr uint32_t PRECISION = FPBits<T>::FRACTION_LEN + 1;
using output_bits_t = typename FPBits<T>::StorageType;
int exp_hi = exponent + static_cast<int>((Bits - 1) + FPBits<T>::EXP_BIAS);
diff --git a/libc/src/__support/FPUtil/except_value_utils.h b/libc/src/__support/FPUtil/except_value_utils.h
index 89849540315f64..1e0381194009d5 100644
--- a/libc/src/__support/FPUtil/except_value_utils.h
+++ b/libc/src/__support/FPUtil/except_value_utils.h
@@ -102,15 +102,13 @@ template <typename T, size_t N> struct ExceptValues {
// Helper functions to set results for exceptional cases.
template <typename T> LIBC_INLINE T round_result_slightly_down(T value_rn) {
volatile T tmp = value_rn;
- const T MIN_NORMAL = FPBits<T>::min_normal();
- tmp = tmp - MIN_NORMAL;
+ tmp -= FPBits<T>::min_normal().get_val();
return tmp;
}
template <typename T> LIBC_INLINE T round_result_slightly_up(T value_rn) {
volatile T tmp = value_rn;
- const T MIN_NORMAL = FPBits<T>::min_normal();
- tmp = tmp + MIN_NORMAL;
+ tmp += FPBits<T>::min_normal().get_val();
return tmp;
}
diff --git a/libc/src/__support/FPUtil/generic/FMA.h b/libc/src/__support/FPUtil/generic/FMA.h
index 6285cac1983d1e..5c36463ea50213 100644
--- a/libc/src/__support/FPUtil/generic/FMA.h
+++ b/libc/src/__support/FPUtil/generic/FMA.h
@@ -130,9 +130,9 @@ template <> LIBC_INLINE double fma<double>(double x, double y, double z) {
return x * y + z;
// Extract mantissa and append hidden leading bits.
- UInt128 x_mant = x_bits.get_mantissa() | FPBits::MIN_NORMAL;
- UInt128 y_mant = y_bits.get_mantissa() | FPBits::MIN_NORMAL;
- UInt128 z_mant = z_bits.get_mantissa() | FPBits::MIN_NORMAL;
+ UInt128 x_mant = x_bits.get_explicit_mantissa();
+ UInt128 y_mant = y_bits.get_explicit_mantissa();
+ UInt128 z_mant = z_bits.get_explicit_mantissa();
// If the exponent of the product x*y > the exponent of z, then no extra
// precision beside the entire product x*y is needed. On the other hand, when
@@ -255,9 +255,7 @@ template <> LIBC_INLINE double fma<double>(double x, double y, double z) {
if ((round_mode == FE_TOWARDZERO) ||
(round_mode == FE_UPWARD && prod_sign.is_neg()) ||
(round_mode == FE_DOWNWARD && prod_sign.is_pos())) {
- result = FPBits::MAX_NORMAL;
- return prod_sign.is_neg() ? -cpp::bit_cast<double>(result)
- : cpp::bit_cast<double>(result);
+ return FPBits::max_normal(prod_sign).get_val();
}
return static_cast<double>(FPBits::inf(prod_sign));
}
diff --git a/libc/src/__support/FPUtil/generic/FMod.h b/libc/src/__support/FPUtil/generic/FMod.h
index f4000b97751efc..18355b801dbc7c 100644
--- a/libc/src/__support/FPUtil/generic/FMod.h
+++ b/libc/src/__support/FPUtil/generic/FMod.h
@@ -124,7 +124,7 @@ template <typename T> struct FModExceptionalInputHandler {
LIBC_INLINE static bool pre_check(T x, T y, T &out) {
using FPB = fputil::FPBits<T>;
- const T quiet_nan = FPB::build_quiet_nan(0);
+ const T quiet_nan = FPB::build_quiet_nan().get_val();
FPB sx(x), sy(y);
if (LIBC_LIKELY(!sy.is_zero() && !sy.is_inf_or_nan() &&
!sx.is_inf_or_nan())) {
diff --git a/libc/src/__support/FPUtil/generic/sqrt.h b/libc/src/__support/FPUtil/generic/sqrt.h
index 21ae9d081d3f12..0a0690ec1463b9 100644
--- a/libc/src/__support/FPUtil/generic/sqrt.h
+++ b/libc/src/__support/FPUtil/generic/sqrt.h
@@ -71,15 +71,19 @@ LIBC_INLINE cpp::enable_if_t<cpp::is_floating_point_v<T>, T> sqrt(T x) {
return x86::sqrt(x);
} else {
// IEEE floating points formats.
- using StorageType = typename FPBits<T>::StorageType;
- constexpr StorageType ONE = StorageType(1) << FPBits<T>::FRACTION_LEN;
+ using Sign = fputil::Sign;
+ using FPBits_t = typename fputil::FPBits<T>;
+ using StorageType = typename FPBits_t::StorageType;
+ constexpr StorageType ONE = StorageType(1) << FPBits_t::FRACTION_LEN;
+ constexpr auto FLT_NAN =
+ FPBits_t::build_quiet_nan(Sign::POS, ONE >> 1).get_val();
- FPBits<T> bits(x);
+ FPBits_t bits(x);
if (bits.is_inf_or_nan()) {
if (bits.is_neg() && (bits.get_mantissa() == 0)) {
// sqrt(-Inf) = NaN
- return FPBits<T>::build_quiet_nan(ONE >> 1);
+ return FLT_NAN;
} else {
// sqrt(NaN) = NaN
// sqrt(+Inf) = +Inf
@@ -91,7 +95,7 @@ LIBC_INLINE cpp::enable_if_t<cpp::is_floating_point_v<T>, T> sqrt(T x) {
return x;
} else if (bits.is_neg()) {
// sqrt( negative numbers ) = NaN
- return FPBits<T>::build_quiet_nan(ONE >> 1);
+ return FLT_NAN;
} else {
int x_exp = bits.get_exponent();
StorageType x_mant = bits.get_mantissa();
@@ -145,10 +149,10 @@ LIBC_INLINE cpp::enable_if_t<cpp::is_floating_point_v<T>, T> sqrt(T x) {
}
// Remove hidden bit and append the exponent field.
- x_exp = ((x_exp >> 1) + FPBits<T>::EXP_BIAS);
+ x_exp = ((x_exp >> 1) + FPBits_t::EXP_BIAS);
y = (y - ONE) |
- (static_cast<StorageType>(x_exp) << FPBits<T>::FRACTION_LEN);
+ (static_cast<StorageType>(x_exp) << FPBits_t::FRACTION_LEN);
switch (quick_get_round()) {
case FE_TONEAREST:
diff --git a/libc/src/__support/FPUtil/generic/sqrt_80_bit_long_double.h b/libc/src/__support/FPUtil/generic/sqrt_80_bit_long_double.h
index 4f8d136938f56e..b0a3776029ca78 100644
--- a/libc/src/__support/FPUtil/generic/sqrt_80_bit_long_double.h
+++ b/libc/src/__support/FPUtil/generic/sqrt_80_bit_long_double.h
@@ -38,14 +38,16 @@ LIBC_INLINE long double sqrt(long double x);
LIBC_INLINE long double sqrt(long double x) {
using LDBits = FPBits<long double>;
using StorageType = typename LDBits::StorageType;
+ using Sign = fputil::Sign;
constexpr StorageType ONE = StorageType(1) << int(LDBits::FRACTION_LEN);
+ constexpr auto LDNAN = LDBits::build_quiet_nan(Sign::POS, ONE >> 1).get_val();
- FPBits<long double> bits(x);
+ LDBits bits(x);
if (bits.is_inf_or_nan()) {
if (bits.is_neg() && (bits.get_mantissa() == 0)) {
// sqrt(-Inf) = NaN
- return LDBits::build_quiet_nan(ONE >> 1);
+ return LDNAN;
} else {
// sqrt(NaN) = NaN
// sqrt(+Inf) = +Inf
@@ -57,7 +59,7 @@ LIBC_INLINE long double sqrt(long double x) {
return x;
} else if (bits.is_neg()) {
// sqrt( negative numbers ) = NaN
- return LDBits::build_quiet_nan(ONE >> 1);
+ return LDNAN;
} else {
int x_exp = bits.get_explicit_exponent();
StorageType x_mant = bits.get_mantissa();
diff --git a/libc/src/__support/FPUtil/x86_64/NextAfterLongDouble.h b/libc/src/__support/FPUtil/x86_64/NextAfterLongDouble.h
index 512f5de4e7931a..d1c76ba954b930 100644
--- a/libc/src/__support/FPUtil/x86_64/NextAfterLongDouble.h
+++ b/libc/src/__support/FPUtil/x86_64/NextAfterLongDouble.h
@@ -43,16 +43,18 @@ LIBC_INLINE long double nextafter(long double from, long double to) {
}
using StorageType = FPBits::StorageType;
- constexpr StorageType SIGN_VAL = (StorageType(1) << 79);
+
constexpr StorageType FRACTION_MASK = FPBits::FRACTION_MASK;
- StorageType int_val = from_bits.uintval();
- if (from < 0.0l) {
- if (from > to) {
- if (int_val == (SIGN_VAL + FPBits::MAX_SUBNORMAL)) {
+ // StorageType int_val = from_bits.uintval();
+ if (from == 0.0l) { // +0.0 / -0.0
+ from_bits = FPBits::min_subnormal(from > to ? Sign::NEG : Sign::POS);
+ } else if (from < 0.0l) {
+ if (to < from) { // toward -inf
+ if (from_bits == FPBits::max_subnormal(Sign::NEG)) {
// We deal with normal/subnormal boundary separately to avoid
// dealing with the implicit bit.
- int_val = SIGN_VAL + FPBits::MIN_NORMAL;
- } else if ((int_val & FRACTION_MASK) == FRACTION_MASK) {
+ from_bits = FPBits::min_normal(Sign::NEG);
+ } else if (from_bits.get_mantissa() == FRACTION_MASK) {
from_bits.set_mantissa(0);
// Incrementing exponent might overflow the value to infinity,
// which is what is expected. Since NaNs are handling separately,
@@ -62,45 +64,40 @@ LIBC_INLINE long double nextafter(long double from, long double to) {
raise_except_if_required(FE_OVERFLOW | FE_INEXACT);
return from_bits.get_val();
} else {
- ++int_val;
+ from_bits = FPBits(StorageType(from_bits.uintval() + 1));
}
- } else {
- if (int_val == (SIGN_VAL + FPBits::MIN_NORMAL)) {
+ } else { // toward +inf
+ if (from_bits == FPBits::min_normal(Sign::NEG)) {
// We deal with normal/subnormal boundary separately to avoid
// dealing with the implicit bit.
- int_val = SIGN_VAL + FPBits::MAX_SUBNORMAL;
- } else if ((int_val & FRACTION_MASK) == 0) {
+ from_bits = FPBits::max_subnormal(Sign::NEG);
+ } else if (from_bits.get_mantissa() == 0) {
from_bits.set_mantissa(FRACTION_MASK);
// from == 0 is handled separately so decrementing the exponent will not
// lead to underflow.
from_bits.set_biased_exponent(from_bits.get_biased_exponent() - 1);
return from_bits.get_val();
} else {
- --int_val;
+ from_bits = FPBits(StorageType(from_bits.uintval() - 1));
}
}
- } else if (from == 0.0l) {
- if (from > to)
- int_val = SIGN_VAL + 1;
- else
- int_val = 1;
} else {
- if (from > to) {
- if (int_val == FPBits::MIN_NORMAL) {
- int_val = FPBits::MAX_SUBNORMAL;
- } else if ((int_val & FRACTION_MASK) == 0) {
+ if (to < from) { // toward -inf
+ if (from_bits == FPBits::min_normal(Sign::POS)) {
+ from_bits = FPBits::max_subnormal(Sign::POS);
+ } else if (from_bits.get_mantissa() == 0) {
from_bits.set_mantissa(FRACTION_MASK);
// from == 0 is handled separately so decrementing the exponent will not
// lead to underflow.
from_bits.set_biased_exponent(from_bits.get_biased_exponent() - 1);
return from_bits.get_val();
} else {
- --int_val;
+ from_bits = FPBits(StorageType(from_bits.uintval() - 1));
}
- } else {
- if (int_val == FPBits::MAX_SUBNORMAL) {
- int_val = FPBits::MIN_NORMAL;
- } else if ((int_val & FRACTION_MASK) == FRACTION_MASK) {
+ } else { // toward +inf
+ if (from_bits == FPBits::max_subnormal(Sign::POS)) {
+ from_bits = FPBits::min_normal(Sign::POS);
+ } else if (from_bits.get_mantissa() == FRACTION_MASK) {
from_bits.set_mantissa(0);
// Incrementing exponent might overflow the value to infinity,
// which is what is expected. Since NaNs are handling separately,
@@ -110,16 +107,15 @@ LIBC_INLINE long double nextafter(long double from, long double to) {
ra...
[truncated]
|
gchatelet
changed the title
FPBits functions and properties
frobtech
added a commit
that referenced
this pull request
Jan 23, 2024
lntue
added a commit
to lntue/llvm-project
that referenced
this pull request
Jan 23, 2024
jhuber6
pushed a commit
that referenced
this pull request
Jan 23, 2024
|
hmm...quite a few follow ups to this commit. The 32b arm build bots do take some time to provide feedback post submit, but it's important to wait to ensure they go back to green or don't go red in the first place. If it's near the end of your workday, I suggest holding off on merging commits until the next workday morning (especially Friday afternoons). That gives plenty of time to follow up on build breakages that are found only through post submits. My hope is that we can make the enable runtimes build more ubiquitous so that testing via cross compiling is so fast/easy, it's the default development workflow. |
So the issue does not come from the fix forward nor from this patch. It comes from the original patch that was reviewed. Unfortunately a change to In my opinion there are several things we can do to improve the situation:
I had no signal that it was broken but my apologies for the breakage nonetheless. |
|
All great ideas.
So, some Was C vs C++ one of the related issues in this case?
Yeah this is a good idea. The tradeoff between "should this be in presubmit vs post submit" is usually latency. Even if we just did builds in presubmit and then ran the tests in post submit, that might be the right balance. I think the presubmit bots configurations are somewhere in https://github.com/llvm/llvm-zorg; we can play with moving more checks into presubmit.
So we do have the Fuchsia build bot spot issues, though IDK how to find it on lab.llvm.org...is it just https://lab.llvm.org/buildbot/#/builders/98 (@petrhosek )? Maybe we should look into adding a dedicated build of just llvm-libc just for Fuchsia? What about GPU builds. Do we have GPU builds of llvm-libc on lab.llvm.org @jhuber6 ? (And forgive my ignorance)
Yeah, with enable runtimes, I bet I can BUILD arm32 faster on an x86_64 host than on the arm32 build bot itself. For running the unit tests, we need to explore qemu. I'd also bet that running the unit tests in an arm32 vm in qemu on an x86_64 host is also faster than running the tests natively on the arm32 build bot.
Ah, no worries! Thanks for brainstorming with me ways in which we can improve our development processes so that we can all go faster. |
Our GTEST uses C++, I don't think we can use it with C. In that case the issue is that our integration tests do refer to the C++ implementation but they should really refer to the C functions. I've started changing them but I'm facing a build problem that I'm trying to address with @lntue in #79319. They should be covered soon.
I agree it's hard to strike a good balance. For small patches it's probably going to be annoying but for large refactoring it definitely helps catch bugs earlier and prevents going through the painful revert / reland cycle. Also it reduces downstream users impact.
To make sure I don't miss a bot I usually use a filtered view with the
No doubt about it 😁, that would be fantastic. Last but not least, the |
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.
FPBitsfunctions and properties #79113