Skip to content

Commit da8433d

Browse files
committed
libm-test: Fix unintentional skips in binop_common
`binop_common` emits a `SKIP` that is intended to apply only to `copysign`, but is instead applying to all binary operators. Correct the general case but leave the currently-failing `maximum_num` tests as a FIXME, to be resolved separately in [1]. Also simplify skip logic and NaN checking, and add a few more `copysign` checks. [1]: #939
1 parent e211ac6 commit da8433d

File tree

4 files changed

+34
-12
lines changed

4 files changed

+34
-12
lines changed

libm-test/src/generate/edge_cases.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ where
5151

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

5657
// Check around the maximum subnormal value

libm-test/src/precision.rs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -444,13 +444,18 @@ fn binop_common<F1: Float, F2: Float>(
444444
expected: F2,
445445
ctx: &CheckCtx,
446446
) -> CheckAction {
447-
// MPFR only has one NaN bitpattern; allow the default `.is_nan()` checks to validate. Skip if
448-
// the first input (magnitude source) is NaN and the output is also a NaN, or if the second
449-
// input (sign source) is NaN.
450-
if ctx.basis == CheckBasis::Mpfr
447+
// MPFR only has one NaN bitpattern; skip tests in cases where the first argument would take
448+
// the sign of a NaN second argument. The default NaN checks cover other cases.
449+
if ctx.base_name == BaseName::Copysign && ctx.basis == CheckBasis::Mpfr && input.1.is_nan() {
450+
return SKIP;
451+
}
452+
453+
// FIXME(#939): this should not be skipped, there is a bug in our implementationi.
454+
if ctx.base_name == BaseName::FmaximumNum
455+
&& ctx.basis == CheckBasis::Mpfr
451456
&& ((input.0.is_nan() && actual.is_nan() && expected.is_nan()) || input.1.is_nan())
452457
{
453-
return SKIP;
458+
return XFAIL_NOCHECK;
454459
}
455460

456461
/* FIXME(#439): our fmin and fmax do not compare signed zeros */

libm-test/src/test_traits.rs

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -312,12 +312,9 @@ where
312312
let mut inner = || -> TestResult {
313313
let mut allowed_ulp = ctx.ulp;
314314

315-
// Forbid overrides if the items came from an explicit list, as long as we are checking
316-
// against either MPFR or the result itself.
317-
let require_biteq = ctx.gen_kind == GeneratorKind::List && ctx.basis != CheckBasis::Musl;
318-
319315
match SpecialCase::check_float(input, actual, expected, ctx) {
320-
_ if require_biteq => (),
316+
// Forbid overrides if the items came from an explicit list
317+
_ if ctx.gen_kind == GeneratorKind::List => (),
321318
CheckAction::AssertSuccess => (),
322319
CheckAction::AssertFailure(msg) => assert_failure_msg = Some(msg),
323320
CheckAction::Custom(res) => return res,
@@ -327,9 +324,20 @@ where
327324

328325
// Check when both are NaNs
329326
if actual.is_nan() && expected.is_nan() {
330-
if require_biteq && ctx.basis == CheckBasis::None {
327+
// Don't assert NaN bitwise equality if:
328+
//
329+
// * Testing against MPFR (there is a single NaN representation)
330+
// * Testing against Musl except for explicit tests (Musl does some NaN quieting)
331+
//
332+
// In these cases, just the check that actual and expected are both NaNs is
333+
// sufficient.
334+
let skip_nan_biteq = ctx.basis == CheckBasis::Mpfr
335+
|| (ctx.basis == CheckBasis::Musl && ctx.gen_kind != GeneratorKind::List);
336+
337+
if !skip_nan_biteq {
331338
ensure!(actual.biteq(expected), "mismatched NaN bitpatterns");
332339
}
340+
333341
// By default, NaNs have nothing special to check.
334342
return Ok(());
335343
} else if actual.is_nan() || expected.is_nan() {

libm/src/math/copysign.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,17 @@ mod tests {
5959

6060
// Not required but we expect it
6161
assert_biteq!(f(F::NAN, F::NAN), F::NAN);
62-
assert_biteq!(f(F::NEG_NAN, F::NAN), F::NAN);
62+
assert_biteq!(f(F::NAN, F::ONE), F::NAN);
63+
assert_biteq!(f(F::NAN, F::NEG_ONE), F::NEG_NAN);
6364
assert_biteq!(f(F::NAN, F::NEG_NAN), F::NEG_NAN);
65+
assert_biteq!(f(F::NEG_NAN, F::NAN), F::NAN);
66+
assert_biteq!(f(F::NEG_NAN, F::ONE), F::NAN);
67+
assert_biteq!(f(F::NEG_NAN, F::NEG_ONE), F::NEG_NAN);
6468
assert_biteq!(f(F::NEG_NAN, F::NEG_NAN), F::NEG_NAN);
69+
assert_biteq!(f(F::ONE, F::NAN), F::ONE);
70+
assert_biteq!(f(F::ONE, F::NEG_NAN), F::NEG_ONE);
71+
assert_biteq!(f(F::NEG_ONE, F::NAN), F::ONE);
72+
assert_biteq!(f(F::NEG_ONE, F::NEG_NAN), F::NEG_ONE);
6573
}
6674

6775
#[test]

0 commit comments

Comments
 (0)