Skip to content

libm-test: Fix unintentional skips in binop_common #941

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 1 commit into from
Jun 2, 2025
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
1 change: 1 addition & 0 deletions libm-test/src/generate/edge_cases.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ where

// Check some special values that aren't included in the above ranges
values.push(Op::FTy::NAN);
values.push(Op::FTy::NEG_NAN);
values.extend(Op::FTy::consts().iter());

// Check around the maximum subnormal value
Expand Down
15 changes: 10 additions & 5 deletions libm-test/src/precision.rs
Original file line number Diff line number Diff line change
Expand Up @@ -444,13 +444,18 @@ fn binop_common<F1: Float, F2: Float>(
expected: F2,
ctx: &CheckCtx,
) -> CheckAction {
// MPFR only has one NaN bitpattern; allow the default `.is_nan()` checks to validate. Skip if
// the first input (magnitude source) is NaN and the output is also a NaN, or if the second
// input (sign source) is NaN.
if ctx.basis == CheckBasis::Mpfr
// MPFR only has one NaN bitpattern; skip tests in cases where the first argument would take
// the sign of a NaN second argument. The default NaN checks cover other cases.
if ctx.base_name == BaseName::Copysign && ctx.basis == CheckBasis::Mpfr && input.1.is_nan() {
return SKIP;
}

// FIXME(#939): this should not be skipped, there is a bug in our implementationi.
if ctx.base_name == BaseName::FmaximumNum
&& ctx.basis == CheckBasis::Mpfr
&& ((input.0.is_nan() && actual.is_nan() && expected.is_nan()) || input.1.is_nan())
{
return SKIP;
return XFAIL_NOCHECK;
}

/* FIXME(#439): our fmin and fmax do not compare signed zeros */
Expand Down
20 changes: 14 additions & 6 deletions libm-test/src/test_traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,12 +312,9 @@ where
let mut inner = || -> TestResult {
let mut allowed_ulp = ctx.ulp;

// Forbid overrides if the items came from an explicit list, as long as we are checking
// against either MPFR or the result itself.
let require_biteq = ctx.gen_kind == GeneratorKind::List && ctx.basis != CheckBasis::Musl;

match SpecialCase::check_float(input, actual, expected, ctx) {
_ if require_biteq => (),
// Forbid overrides if the items came from an explicit list
_ if ctx.gen_kind == GeneratorKind::List => (),
CheckAction::AssertSuccess => (),
CheckAction::AssertFailure(msg) => assert_failure_msg = Some(msg),
CheckAction::Custom(res) => return res,
Expand All @@ -327,9 +324,20 @@ where

// Check when both are NaNs
if actual.is_nan() && expected.is_nan() {
if require_biteq && ctx.basis == CheckBasis::None {
// Don't assert NaN bitwise equality if:
//
// * Testing against MPFR (there is a single NaN representation)
// * Testing against Musl except for explicit tests (Musl does some NaN quieting)
//
// In these cases, just the check that actual and expected are both NaNs is
// sufficient.
let skip_nan_biteq = ctx.basis == CheckBasis::Mpfr
|| (ctx.basis == CheckBasis::Musl && ctx.gen_kind != GeneratorKind::List);

if !skip_nan_biteq {
ensure!(actual.biteq(expected), "mismatched NaN bitpatterns");
}

// By default, NaNs have nothing special to check.
return Ok(());
} else if actual.is_nan() || expected.is_nan() {
Expand Down
10 changes: 9 additions & 1 deletion libm/src/math/copysign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,17 @@ mod tests {

// Not required but we expect it
assert_biteq!(f(F::NAN, F::NAN), F::NAN);
assert_biteq!(f(F::NEG_NAN, F::NAN), F::NAN);
assert_biteq!(f(F::NAN, F::ONE), F::NAN);
assert_biteq!(f(F::NAN, F::NEG_ONE), F::NEG_NAN);
assert_biteq!(f(F::NAN, F::NEG_NAN), F::NEG_NAN);
assert_biteq!(f(F::NEG_NAN, F::NAN), F::NAN);
assert_biteq!(f(F::NEG_NAN, F::ONE), F::NAN);
assert_biteq!(f(F::NEG_NAN, F::NEG_ONE), F::NEG_NAN);
assert_biteq!(f(F::NEG_NAN, F::NEG_NAN), F::NEG_NAN);
assert_biteq!(f(F::ONE, F::NAN), F::ONE);
assert_biteq!(f(F::ONE, F::NEG_NAN), F::NEG_ONE);
assert_biteq!(f(F::NEG_ONE, F::NAN), F::ONE);
assert_biteq!(f(F::NEG_ONE, F::NEG_NAN), F::NEG_ONE);
}

#[test]
Expand Down