Skip to content

[libc][math] Update getpayload and fmul with NaN inputs. #99812

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 5 commits into from
Jul 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions libc/config/linux/riscv/entrypoints.txt
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@ set(TARGET_LIBM_ENTRYPOINTS
libc.src.math.cosf
libc.src.math.coshf
libc.src.math.cospif
libc.src.math.dmull
libc.src.math.dsqrtl
libc.src.math.erff
libc.src.math.exp
Expand Down Expand Up @@ -434,6 +435,7 @@ set(TARGET_LIBM_ENTRYPOINTS
libc.src.math.fmodf
libc.src.math.fmodl
libc.src.math.fmul
libc.src.math.fmull
libc.src.math.frexp
libc.src.math.frexpf
libc.src.math.frexpl
Expand Down
13 changes: 11 additions & 2 deletions libc/src/__support/FPUtil/BasicOperations.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@

#include "FEnvImpl.h"
#include "FPBits.h"
#include "dyadic_float.h"

#include "FEnvImpl.h"
#include "src/__support/CPP/type_traits.h"
#include "src/__support/common.h"
#include "src/__support/macros/config.h"
Expand Down Expand Up @@ -269,12 +269,21 @@ totalordermag(T x, T y) {
template <typename T>
LIBC_INLINE cpp::enable_if_t<cpp::is_floating_point_v<T>, T> getpayload(T x) {
using FPBits = FPBits<T>;
using StorageType = typename FPBits::StorageType;
FPBits x_bits(x);

if (!x_bits.is_nan())
return T(-1.0);

return T(x_bits.uintval() & (FPBits::FRACTION_MASK >> 1));
StorageType payload = x_bits.uintval() & (FPBits::FRACTION_MASK >> 1);

if constexpr (is_big_int_v<StorageType>) {
DyadicFloat<FPBits::STORAGE_LEN> payload_dfloat(Sign::POS, 0, payload);

return static_cast<T>(payload_dfloat);
} else {
return static_cast<T>(payload);
}
}

template <bool IsSignaling, typename T>
Expand Down
57 changes: 29 additions & 28 deletions libc/src/__support/FPUtil/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -75,19 +75,6 @@ add_header_library(
libc.src.__support.common
)

add_header_library(
basic_operations
HDRS
BasicOperations.h
DEPENDS
.fp_bits
.fenv_impl
libc.src.__support.CPP.type_traits
libc.src.__support.uint128
libc.src.__support.common
libc.src.__support.macros.optimization
)

add_header_library(
division_and_remainder_operations
HDRS
Expand All @@ -113,21 +100,6 @@ add_header_library(
)


add_header_library(
hypot
HDRS
Hypot.h
DEPENDS
.basic_operations
.fenv_impl
.fp_bits
.rounding_mode
libc.src.__support.common
libc.src.__support.CPP.bit
libc.src.__support.CPP.type_traits
libc.src.__support.uint128
)

add_header_library(
sqrt
HDRS
Expand Down Expand Up @@ -208,6 +180,35 @@ add_header_library(
libc.src.__support.macros.optimization
)

add_header_library(
basic_operations
HDRS
BasicOperations.h
DEPENDS
.dyadic_float
.fp_bits
.fenv_impl
libc.src.__support.CPP.type_traits
libc.src.__support.uint128
libc.src.__support.common
libc.src.__support.macros.optimization
)

add_header_library(
hypot
HDRS
Hypot.h
DEPENDS
.basic_operations
.fenv_impl
.fp_bits
.rounding_mode
libc.src.__support.common
libc.src.__support.CPP.bit
libc.src.__support.CPP.type_traits
libc.src.__support.uint128
)

add_header_library(
manipulation_functions
HDRS
Expand Down
45 changes: 27 additions & 18 deletions libc/src/__support/FPUtil/generic/FMA.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,27 +129,27 @@ fma(InType x, InType y, InType z) {
raise_except_if_required(FE_INVALID);

if (x_bits.is_quiet_nan()) {
InStorageType x_payload = static_cast<InStorageType>(getpayload(x));
if ((x_payload & ~(OutFPBits::FRACTION_MASK >> 1)) == 0)
return OutFPBits::quiet_nan(x_bits.sign(),
static_cast<OutStorageType>(x_payload))
.get_val();
InStorageType x_payload = x_bits.get_mantissa();
x_payload >>= InFPBits::FRACTION_LEN - OutFPBits::FRACTION_LEN;
return OutFPBits::quiet_nan(x_bits.sign(),
static_cast<OutStorageType>(x_payload))
.get_val();
}

if (y_bits.is_quiet_nan()) {
InStorageType y_payload = static_cast<InStorageType>(getpayload(y));
if ((y_payload & ~(OutFPBits::FRACTION_MASK >> 1)) == 0)
return OutFPBits::quiet_nan(y_bits.sign(),
static_cast<OutStorageType>(y_payload))
.get_val();
InStorageType y_payload = y_bits.get_mantissa();
y_payload >>= InFPBits::FRACTION_LEN - OutFPBits::FRACTION_LEN;
return OutFPBits::quiet_nan(y_bits.sign(),
static_cast<OutStorageType>(y_payload))
.get_val();
}

if (z_bits.is_quiet_nan()) {
InStorageType z_payload = static_cast<InStorageType>(getpayload(z));
if ((z_payload & ~(OutFPBits::FRACTION_MASK >> 1)) == 0)
return OutFPBits::quiet_nan(z_bits.sign(),
static_cast<OutStorageType>(z_payload))
.get_val();
InStorageType z_payload = z_bits.get_mantissa();
z_payload >>= InFPBits::FRACTION_LEN - OutFPBits::FRACTION_LEN;
return OutFPBits::quiet_nan(z_bits.sign(),
static_cast<OutStorageType>(z_payload))
.get_val();
}

return OutFPBits::quiet_nan().get_val();
Expand All @@ -163,18 +163,27 @@ fma(InType x, InType y, InType z) {
int y_exp = 0;
int z_exp = 0;

// Denormal scaling = 2^(fraction length).
constexpr InStorageType IMPLICIT_MASK =
InFPBits::SIG_MASK - InFPBits::FRACTION_MASK;

constexpr InType DENORMAL_SCALING =
InFPBits::create_value(
Sign::POS, InFPBits::FRACTION_LEN + InFPBits::EXP_BIAS, IMPLICIT_MASK)
.get_val();

// Normalize denormal inputs.
if (LIBC_UNLIKELY(InFPBits(x).is_subnormal())) {
x_exp -= InFPBits::FRACTION_LEN;
x *= InType(InStorageType(1) << InFPBits::FRACTION_LEN);
x *= DENORMAL_SCALING;
}
if (LIBC_UNLIKELY(InFPBits(y).is_subnormal())) {
y_exp -= InFPBits::FRACTION_LEN;
y *= InType(InStorageType(1) << InFPBits::FRACTION_LEN);
y *= DENORMAL_SCALING;
}
if (LIBC_UNLIKELY(InFPBits(z).is_subnormal())) {
z_exp -= InFPBits::FRACTION_LEN;
z *= InType(InStorageType(1) << InFPBits::FRACTION_LEN);
z *= DENORMAL_SCALING;
}

x_bits = InFPBits(x);
Expand Down
26 changes: 14 additions & 12 deletions libc/src/__support/FPUtil/generic/add_sub.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,19 +56,19 @@ add_or_sub(InType x, InType y) {
raise_except_if_required(FE_INVALID);

if (x_bits.is_quiet_nan()) {
InStorageType x_payload = static_cast<InStorageType>(getpayload(x));
if ((x_payload & ~(OutFPBits::FRACTION_MASK >> 1)) == 0)
return OutFPBits::quiet_nan(x_bits.sign(),
static_cast<OutStorageType>(x_payload))
.get_val();
InStorageType x_payload = x_bits.get_mantissa();
x_payload >>= InFPBits::FRACTION_LEN - OutFPBits::FRACTION_LEN;
return OutFPBits::quiet_nan(x_bits.sign(),
static_cast<OutStorageType>(x_payload))
.get_val();
}

if (y_bits.is_quiet_nan()) {
InStorageType y_payload = static_cast<InStorageType>(getpayload(y));
if ((y_payload & ~(OutFPBits::FRACTION_MASK >> 1)) == 0)
return OutFPBits::quiet_nan(y_bits.sign(),
static_cast<OutStorageType>(y_payload))
.get_val();
InStorageType y_payload = y_bits.get_mantissa();
y_payload >>= InFPBits::FRACTION_LEN - OutFPBits::FRACTION_LEN;
return OutFPBits::quiet_nan(y_bits.sign(),
static_cast<OutStorageType>(y_payload))
.get_val();
}

return OutFPBits::quiet_nan().get_val();
Expand Down Expand Up @@ -174,10 +174,12 @@ add_or_sub(InType x, InType y) {
else
aligned_min_mant_sticky = true;

InStorageType min_mant_sticky(static_cast<int>(aligned_min_mant_sticky));

if (is_effectively_add)
result_mant = max_mant + (aligned_min_mant | aligned_min_mant_sticky);
result_mant = max_mant + (aligned_min_mant | min_mant_sticky);
else
result_mant = max_mant - (aligned_min_mant | aligned_min_mant_sticky);
result_mant = max_mant - (aligned_min_mant | min_mant_sticky);
}

int result_exp = max_bits.get_exponent() - RESULT_FRACTION_LEN;
Expand Down
20 changes: 10 additions & 10 deletions libc/src/__support/FPUtil/generic/div.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,19 +49,19 @@ div(InType x, InType y) {
raise_except_if_required(FE_INVALID);

if (x_bits.is_quiet_nan()) {
InStorageType x_payload = static_cast<InStorageType>(getpayload(x));
if ((x_payload & ~(OutFPBits::FRACTION_MASK >> 1)) == 0)
return OutFPBits::quiet_nan(x_bits.sign(),
static_cast<OutStorageType>(x_payload))
.get_val();
InStorageType x_payload = x_bits.get_mantissa();
x_payload >>= InFPBits::FRACTION_LEN - OutFPBits::FRACTION_LEN;
return OutFPBits::quiet_nan(x_bits.sign(),
static_cast<OutStorageType>(x_payload))
.get_val();
}

if (y_bits.is_quiet_nan()) {
InStorageType y_payload = static_cast<InStorageType>(getpayload(y));
if ((y_payload & ~(OutFPBits::FRACTION_MASK >> 1)) == 0)
return OutFPBits::quiet_nan(y_bits.sign(),
static_cast<OutStorageType>(y_payload))
.get_val();
InStorageType y_payload = y_bits.get_mantissa();
y_payload >>= InFPBits::FRACTION_LEN - OutFPBits::FRACTION_LEN;
return OutFPBits::quiet_nan(y_bits.sign(),
static_cast<OutStorageType>(y_payload))
.get_val();
}

return OutFPBits::quiet_nan().get_val();
Expand Down
20 changes: 10 additions & 10 deletions libc/src/__support/FPUtil/generic/mul.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,19 +50,19 @@ mul(InType x, InType y) {
raise_except_if_required(FE_INVALID);

if (x_bits.is_quiet_nan()) {
InStorageType x_payload = static_cast<InStorageType>(getpayload(x));
if ((x_payload & ~(OutFPBits::FRACTION_MASK >> 1)) == 0)
return OutFPBits::quiet_nan(x_bits.sign(),
static_cast<OutStorageType>(x_payload))
.get_val();
InStorageType x_payload = x_bits.get_mantissa();
x_payload >>= InFPBits::FRACTION_LEN - OutFPBits::FRACTION_LEN;
return OutFPBits::quiet_nan(x_bits.sign(),
static_cast<OutStorageType>(x_payload))
.get_val();
}

if (y_bits.is_quiet_nan()) {
InStorageType y_payload = static_cast<InStorageType>(getpayload(y));
if ((y_payload & ~(OutFPBits::FRACTION_MASK >> 1)) == 0)
return OutFPBits::quiet_nan(y_bits.sign(),
static_cast<OutStorageType>(y_payload))
.get_val();
InStorageType y_payload = y_bits.get_mantissa();
y_payload >>= InFPBits::FRACTION_LEN - OutFPBits::FRACTION_LEN;
return OutFPBits::quiet_nan(y_bits.sign(),
static_cast<OutStorageType>(y_payload))
.get_val();
}

return OutFPBits::quiet_nan().get_val();
Expand Down
19 changes: 2 additions & 17 deletions libc/test/src/math/smoke/AddTest.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,23 +38,8 @@ class AddTest : public LIBC_NAMESPACE::testing::FEnvSafeTest {
EXPECT_FP_IS_NAN_WITH_EXCEPTION(func(sNaN, sNaN), FE_INVALID);

InType qnan_42 = InFPBits::quiet_nan(Sign::POS, 0x42).get_val();
EXPECT_FP_EQ(InType(0x42.0p+0),
LIBC_NAMESPACE::fputil::getpayload(func(qnan_42, zero)));
EXPECT_FP_EQ(InType(0x42.0p+0),
LIBC_NAMESPACE::fputil::getpayload(func(zero, qnan_42)));

if constexpr (sizeof(OutType) < sizeof(InType)) {
InStorageType max_payload = InFPBits::FRACTION_MASK >> 1;
InType qnan_max = InFPBits::quiet_nan(Sign::POS, max_payload).get_val();
EXPECT_FP_EQ(zero,
LIBC_NAMESPACE::fputil::getpayload(func(qnan_max, zero)));
EXPECT_FP_EQ(zero,
LIBC_NAMESPACE::fputil::getpayload(func(zero, qnan_max)));
EXPECT_FP_EQ(InType(0x42.0p+0),
LIBC_NAMESPACE::fputil::getpayload(func(qnan_max, qnan_42)));
EXPECT_FP_EQ(InType(0x42.0p+0),
LIBC_NAMESPACE::fputil::getpayload(func(qnan_42, qnan_max)));
}
EXPECT_FP_IS_NAN(func(qnan_42, zero));
EXPECT_FP_IS_NAN(func(zero, qnan_42));

EXPECT_FP_EQ(inf, func(inf, zero));
EXPECT_FP_EQ(neg_inf, func(neg_inf, zero));
Expand Down
19 changes: 2 additions & 17 deletions libc/test/src/math/smoke/DivTest.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,23 +36,8 @@ class DivTest : public LIBC_NAMESPACE::testing::FEnvSafeTest {
EXPECT_FP_IS_NAN_WITH_EXCEPTION(func(sNaN, sNaN), FE_INVALID);

InType qnan_42 = InFPBits::quiet_nan(Sign::POS, 0x42).get_val();
EXPECT_FP_EQ(InType(0x42.0p+0),
LIBC_NAMESPACE::fputil::getpayload(func(qnan_42, zero)));
EXPECT_FP_EQ(InType(0x42.0p+0),
LIBC_NAMESPACE::fputil::getpayload(func(zero, qnan_42)));

if constexpr (sizeof(OutType) < sizeof(InType)) {
InStorageType max_payload = InFPBits::FRACTION_MASK >> 1;
InType qnan_max = InFPBits::quiet_nan(Sign::POS, max_payload).get_val();
EXPECT_FP_EQ(zero,
LIBC_NAMESPACE::fputil::getpayload(func(qnan_max, zero)));
EXPECT_FP_EQ(zero,
LIBC_NAMESPACE::fputil::getpayload(func(zero, qnan_max)));
EXPECT_FP_EQ(InType(0x42.0p+0),
LIBC_NAMESPACE::fputil::getpayload(func(qnan_max, qnan_42)));
EXPECT_FP_EQ(InType(0x42.0p+0),
LIBC_NAMESPACE::fputil::getpayload(func(qnan_42, qnan_max)));
}
EXPECT_FP_IS_NAN(func(qnan_42, zero));
EXPECT_FP_IS_NAN(func(zero, qnan_42));

EXPECT_FP_EQ(inf, func(inf, zero));
EXPECT_FP_EQ(neg_inf, func(neg_inf, zero));
Expand Down
19 changes: 2 additions & 17 deletions libc/test/src/math/smoke/MulTest.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,23 +38,8 @@ class MulTest : public LIBC_NAMESPACE::testing::FEnvSafeTest {
EXPECT_FP_IS_NAN_WITH_EXCEPTION(func(sNaN, sNaN), FE_INVALID);

InType qnan_42 = InFPBits::quiet_nan(Sign::POS, 0x42).get_val();
EXPECT_FP_EQ(InType(0x42.0p+0),
LIBC_NAMESPACE::fputil::getpayload(func(qnan_42, zero)));
EXPECT_FP_EQ(InType(0x42.0p+0),
LIBC_NAMESPACE::fputil::getpayload(func(zero, qnan_42)));

if constexpr (sizeof(OutType) < sizeof(InType)) {
InStorageType max_payload = InFPBits::FRACTION_MASK >> 1;
InType qnan_max = InFPBits::quiet_nan(Sign::POS, max_payload).get_val();
EXPECT_FP_EQ(zero,
LIBC_NAMESPACE::fputil::getpayload(func(qnan_max, zero)));
EXPECT_FP_EQ(zero,
LIBC_NAMESPACE::fputil::getpayload(func(zero, qnan_max)));
EXPECT_FP_EQ(InType(0x42.0p+0),
LIBC_NAMESPACE::fputil::getpayload(func(qnan_max, qnan_42)));
EXPECT_FP_EQ(InType(0x42.0p+0),
LIBC_NAMESPACE::fputil::getpayload(func(qnan_42, qnan_max)));
}
EXPECT_FP_IS_NAN(func(qnan_42, zero));
EXPECT_FP_IS_NAN(func(zero, qnan_42));
Comment on lines +41 to +42
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure you don't want to check the payload?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I don't think we need to be that strict on NaN propagation when it is underspecified in the standards, as long as the NaN semantic is preserved.


EXPECT_FP_EQ(inf, func(inf, InType(1.0)));
EXPECT_FP_EQ(neg_inf, func(neg_inf, InType(1.0)));
Expand Down
Loading
Loading