Skip to content

Commit

Permalink
Fix compile of float rounding function tests on MSVC and test for NaNs.
Browse files Browse the repository at this point in the history
I mistakenly thought fesetround() would return the previous mode, but it
returns 0 for success. So we need to call fegetround() first to be able
to restore the mode.

Changing the fp environment does not produce a reliable output in tests
so don't do it (no one changes the env anyway).
  • Loading branch information
danakj committed Sep 26, 2023
1 parent 7e61b56 commit db4bb87
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 52 deletions.
6 changes: 6 additions & 0 deletions sus/num/__private/float_methods.inc
Original file line number Diff line number Diff line change
Expand Up @@ -734,6 +734,9 @@ sus_pure inline _self recip() const& noexcept {
/// $sus::num::@doc.self::round_ties) which often makes the latter a better
/// choice.
///
/// If `self` is a `NaN`, infinity, or zero, the same will be returned, though
/// a different `NaN` may be returned.
///
/// As in Rust's [`round`](
/// https://doc.rust-lang.org/stable/std/[email protected]#method.round),
/// this method preserves the sign bit when the result is `-0.0`, but this is
Expand All @@ -751,6 +754,9 @@ sus_pure inline _self round() const& noexcept {
/// However it breaks with the legacy behaviour of [`std::round`](
/// https://en.cppreference.com/w/cpp/numeric/math/round).
///
/// If `self` is a `NaN`, infinity, or zero, the same will be returned, though
/// a different `NaN` may be returned.
///
/// Rust has the unstable [`round_ties_even`](
/// https://doc.rust-lang.org/stable/std/[email protected]#method.round_ties_even)
/// method that always uses the [`FE_TONEAREST`](
Expand Down
34 changes: 15 additions & 19 deletions sus/num/__private/intrinsics.h
Original file line number Diff line number Diff line change
Expand Up @@ -1511,7 +1511,7 @@ sus_pure_const sus_always_inline constexpr int32_t float_normal_exponent_value(
return exponent_bits(x) - int32_t{1023};
}

constexpr sus_always_inline uint32_t mantissa(float x) noexcept {
sus_pure_const sus_always_inline constexpr uint32_t mantissa(float x) noexcept {
constexpr uint32_t mask = 0b00000000011111111111111111111111;
return into_unsigned_integer(x) & mask;
}
Expand Down Expand Up @@ -1675,46 +1675,42 @@ sus_pure_const inline T float_signum(T x) noexcept {
template <class T>
requires(std::is_floating_point_v<T> && ::sus::mem::size_of<T>() <= 8)
sus_pure_const sus_always_inline T float_round(T x) noexcept {
// MSVC round(float) is returning a double for some reason.
const auto out = into_unsigned_integer(static_cast<T>(std::round(x)));
// `round()` doesn't preserve the sign bit for -0, so we need to restore it,
// for (-0.5, -0.0].
return into_float((out & ~high_bit<T>()) |
(into_unsigned_integer(x) & high_bit<T>()));
return std::round(x);
}

// Note: not sus_pure_const because it depends on global state: the rounding
// mode.
template <class T>
requires(std::is_floating_point_v<T> && ::sus::mem::size_of<T>() <= 8)
sus_pure_const sus_always_inline T float_round_ties_by_mode(T x) noexcept {
sus_pure sus_always_inline T float_round_ties_by_mode(T x) noexcept {
return std::nearbyint(x);
}

// Note: not sus_pure_const because it depends on global state: the rounding
// mode.
template <class I, class T>
requires(std::is_floating_point_v<T> && ::sus::mem::size_of<T>() <= 8)
sus_pure_const sus_always_inline I float_round_to(T x) noexcept {
sus_pure sus_always_inline I float_round_to(T x) noexcept {
static_assert(::sus::mem::size_of<I>() <= ::sus::mem::size_of<long long>());

if (float_is_nan(x)) [[unlikely]]
return I{0};
if (x > max_int_float<I, T>()) [[unlikely]]
return max_value<I>();
if (x < min_int_float<I, T>()) [[unlikely]]
return min_value<I>();
if constexpr (::sus::mem::size_of<I>() == ::sus::mem::size_of<long long>()) {
if (x > max_int_float<I, T>()) [[unlikely]]
return max_value<I>();
if (x < min_int_float<I, T>()) [[unlikely]]
return min_value<I>();
return std::llrint(x);
} else {
if (x > max_int_float<I, T>()) [[unlikely]]
return max_value<I>();
if (x < min_int_float<I, T>()) [[unlikely]]
return min_value<I>();
return std::lrint(x);
return static_cast<I>(std::lrint(x));
}
}

#if __has_builtin(__builtin_fpclassify)
template <class T>
requires(std::is_floating_point_v<T> && ::sus::mem::size_of<T>() <= 8)
constexpr inline ::sus::num::FpCategory float_category(T x) noexcept {
sus_pure_const constexpr inline ::sus::num::FpCategory float_category(
T x) noexcept {
constexpr auto nan = 1;
constexpr auto inf = 2;
constexpr auto norm = 3;
Expand Down
24 changes: 8 additions & 16 deletions sus/num/f32_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#pragma STDC FENV_ACCESS ON

#include <bit>
#include <cfenv>
#include <cmath>
#include <limits>

#include "googletest/include/gtest/gtest.h"
Expand Down Expand Up @@ -785,6 +784,8 @@ TEST(f32, Round) {
auto g = (-0.02_f32).round();
EXPECT_EQ(g.is_sign_negative(), true);
EXPECT_EQ(g, -0_f32);
auto h = f32::NAN.round();
EXPECT_EQ(h.is_nan(), true);
}

TEST(f32, RoundTies) {
Expand All @@ -796,27 +797,18 @@ TEST(f32, RoundTies) {
EXPECT_EQ(c, 2_f32);
auto d = (-1.546_f32).round_ties();
EXPECT_EQ(d, -2_f32);
// On a tie, honour the rounding mode.
{
auto e = (-100.5_f32).round_ties();
EXPECT_EQ(e, -100_f32);
}
{
int mode = std::fesetround(FE_DOWNWARD);
auto e = (-100.5_f32).round_ties();
// Normally this would be -101, and it is with Clang, and sometimes with
// GCC. But on our CI bots GCC is ignoring fesetround() and thus gives back
// -100 from nearbyint() with FE_DOWNWARD.
EXPECT_EQ(e, std::nearbyint(-100.5f));
std::fesetround(mode);
}
// On a tie, honour the rounding mode, which defaults to nearest even.
auto e = (-100.5_f32).round_ties();
EXPECT_EQ(e, -100_f32);
// Preserve sign bit.
auto f = (-0_f32).round_ties();
EXPECT_EQ(f.is_sign_negative(), true);
EXPECT_EQ(f, -0_f32);
auto g = (-0.02_f32).round_ties();
EXPECT_EQ(g.is_sign_negative(), true);
EXPECT_EQ(g, -0_f32);
auto h = f32::NAN.round_ties();
EXPECT_EQ(h.is_nan(), true);
}

TEST(f32, RoundTo) {
Expand Down
28 changes: 11 additions & 17 deletions sus/num/f64_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#pragma STDC FENV_ACCESS ON

#include <math.h>
#include <bit>
#include <cfenv>
#include <cmath>
#include <limits>

#include "googletest/include/gtest/gtest.h"
#include "sus/cmp/eq.h"
Expand Down Expand Up @@ -785,6 +786,8 @@ TEST(f64, Round) {
auto g = (-0.02_f64).round();
EXPECT_EQ(g.is_sign_negative(), true);
EXPECT_EQ(g, -0_f64);
auto h = f64::NAN.round();
EXPECT_EQ(h.is_nan(), true);
}

TEST(f64, RoundTies) {
Expand All @@ -796,27 +799,18 @@ TEST(f64, RoundTies) {
EXPECT_EQ(c, 2_f64);
auto d = (-1.546_f64).round_ties();
EXPECT_EQ(d, -2_f64);
// On a tie, honour the rounding mode.
{
auto e = (-100.5_f64).round_ties();
EXPECT_EQ(e, -100_f64);
}
{
int mode = std::fesetround(FE_DOWNWARD);
auto e = (-100.5_f64).round_ties();
// Normally this would be -101, and it is with Clang, and sometimes with
// GCC. But on our CI bots GCC is ignoring fesetround() and thus gives back
// -100 from nearbyint() with FE_DOWNWARD.
EXPECT_EQ(e, std::nearbyint(-100.5));
std::fesetround(mode);
}
// On a tie, honour the rounding mode, which defaults to nearest even.
auto e = (-100.5_f64).round_ties();
EXPECT_EQ(e, -100_f64);
// Preserve sign bit.
auto f = (-0_f64).round_ties();
EXPECT_EQ(f.is_sign_negative(), true);
EXPECT_EQ(f, -0_f64);
auto g = (-0.02_f64).round_ties();
EXPECT_EQ(g.is_sign_negative(), true);
EXPECT_EQ(g, -0_f64);
auto h = f64::NAN.round_ties();
EXPECT_EQ(h.is_nan(), true);
}

TEST(f64, RoundTo) {
Expand Down

0 comments on commit db4bb87

Please sign in to comment.