Failing doctests on mipsel and mips64el for FloatCore::{min,max}

[silwol]

The float::FloatCore::max and float::FloatCore::min doctests fail on mipsel and mips64el when building the Debian package of num-traits.

I was able to reproduce the behavior on a mipsel vm with Debian unstable installed. Some investigation with added debug output shows that it occurs when calculating max or min with a NaN value involved. In this case, a NaN value gets returned instead of the other value It doesn't matter whether it's the first or the second parameter that is NaN. The error only occurs when the std feature is enabled. It is orthogonal to the libm feature as well as the build mode (release or debug). From my perception, it looks as if the if self.is_nan() and if other.is_nan() never evaluates to true, but the is_nan() doctests succeed, so that's probably not the case.

What is really strange is that even when embedding an identical trait method into the doctest, it behaves as it should. I was not able to get any debug output from inside the failing method due to the type restrictions, not having the dbg!() macro available inside.

Example code for the modified doctest of the min method:

use num_traits::float::FloatCore;
use std::{f32, f64};

fn check<T: FloatCore + std::fmt::Debug>(x: T, y: T, min: T) {
    println!("x=self={:?} y=other={:?}, min={:?}", x, y, min);
    trait XMin: FloatCore + std::fmt::Debug {
        #[inline]
        fn xmin(self, other: Self) -> Self {
            dbg!(self, other);
            dbg!(other.is_nan());
            if self.is_nan() {
                dbg!(other);
                return other;
            }
            dbg!(other.is_nan());
            if other.is_nan() {
                dbg!(self);
                return self;
            }
            dbg!(self < other);
            if self < other {
                dbg!(self);
                self
            } else {
                dbg!(other);
                other
            }
        }
    }
    impl<F: FloatCore + std::fmt::Debug> XMin for F {}
    // Original, not working:
    //assert!(x.min(y) == min);
    // Embedded, working:
    assert!(x.xmin(y) == min);
    dbg!();
}

check(f64::NAN, 2.0f64, 2.0f64);
check(f32::NAN, 2.0, 2.0);
check(1.0f64, -2.0, -2.0);
check(1.0f64, f64::NAN, 1.0);

A log of the failing build on Debian infrastructure:
https://buildd.debian.org/status/fetch.php?pkg=rust-num-traits&arch=mips64el&ver=0.2.11-1&stamp=1578937262&raw=0

[cuviper]

Thank you for the very detailed report! I have a feeling that this will end up being a compiler issue, either in rustc or LLVM, since there's nothing arch-specific about this code. Strange that it doesn't depend on optimization though.

I was able to reproduce the behavior on a mipsel vm with Debian unstable installed.

I assume this was with Debian's rustc -- can you also try it with the upstream binaries?

I was not able to get any debug output from inside the failing method due to the type restrictions, not having the dbg!() macro available inside.

For the purpose of testing, you could add Debug to FloatCore's list of supertraits.

[cuviper]

Small tip -- dbg! returns the value after printing it. You have a typo on which is_nan you're printing:

            dbg!(other.is_nan());
            if self.is_nan() {
                dbg!(other);
                return other;
            }

but you could write:

            if dbg!(self.is_nan()) {
                return dbg!(other);
            }

[silwol]

Thank you for the very detailed report! I have a feeling that this will end up being a compiler issue, either in rustc or LLVM, since there's nothing arch-specific about this code. Strange that it doesn't depend on optimization though.

I was able to reproduce the behavior on a mipsel vm with Debian unstable installed.

I assume this was with Debian's rustc -- can you also try it with the upstream binaries?

That's correct, I tested with Debian's rustc. Just ran the same test with the 1.40.0 version installed through rustup. It shows the same behavior.

I was not able to get any debug output from inside the failing method due to the type restrictions, not having the dbg!() macro available inside.
For the purpose of testing, you could add Debug to FloatCore's list of supertraits.

Will try this.

Small tip -- dbg! returns the value after printing it.

That's really good to know. I keep on being surprised how well-crafted the details in Rust are.

[silwol]

I explored a bit further, and it seems that the doc example doesn't even call the FloatCore implementation of the min and max methods. Even when simply putting a unimplemented!() inside, error occurs at the assert!() comparison.
Is it somehow possible that the call gets delegated to the compiler intrinsic code instead? I thought that shouldn't be possible due to the generic FloatCore parameter of check(), but maybe it is?

[cuviper]

Is it somehow possible that the call gets delegated to the compiler intrinsic code instead? I thought that shouldn't be possible due to the generic FloatCore parameter of check(), but maybe it is?

Oh, when the std feature is enabled, FloatCore forwards to the inherent methods:

num-traits/src/float.rs

Line 889 in 93ff6dc

Self::min(self, other: Self) -> Self;

Earlier you said:

The error only occurs when the std feature is enabled. It is orthogonal to the libm feature as well as the build mode (release or debug).

I missed this only-std detail before! So yes, we'll ultimately be using the intrinsic here, llvm.minnum. I guess you should probably be able to reproduce this without num-traits at all, just the std method, then we can take it to rust-lang/rust.

(And FWIW, libm isn't actually used when std is also enabled.)

[silwol]

Maybe it's a manifestation of rust-lang/rust#52746? It's reproducible with plain usage of std.
This piece of code succeeds on my amd64 box while failing on the mipsel vm.

#[cfg(test)]
mod tests {
    #[test]
    fn min() {
        assert_eq!(1f64.min(std::f64::NAN), 1f64);
    }

    #[test]
    fn max() {
        assert_eq!(1f64.max(std::f64::NAN), 1f64);
    }
}

Output:

user@debian:~/temp/floatextremestest$ cargo test
   Compiling floatextremestest v0.1.0 (/home/user/temp/floatextremestest)
    Finished dev [unoptimized + debuginfo] target(s) in 1m 59s
     Running target/debug/deps/floatextremestest-64fd86d102d84a43

running 2 tests
test tests::max ... FAILED
test tests::min ... FAILED

failures:

---- tests::max stdout ----
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `NaN`,
 right: `1.0`', src/lib.rs:10:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

---- tests::min stdout ----
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `NaN`,
 right: `1.0`', src/lib.rs:5:9


failures:
    tests::max
    tests::min

test result: FAILED. 0 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out

error: test failed, to rerun pass '--lib'
user@debian:~/temp/floatextremestest$ uname -a
Linux debian 5.4.0-3-4kc-malta #1 SMP Debian 5.4.13-1 (2020-01-19) mips GNU/Linux
user@debian:~/temp/floatextremestest$ 

[cuviper]

Ah, if f64::NAN is (accidentally) signaling NaN, I guess that does make some sense. Even on x86_64 (Fedora 31), I can see that difference by choosing explicit NaNs in C++:

#include <cmath>
#include <iostream>
#include <limits>

using namespace std;

int main() {
    double qnan = numeric_limits<double>::quiet_NaN();
    cout << "fmin with qnan: " << fmin(1.0, qnan) << endl;
    cout << "fmax with qnan: " << fmax(1.0, qnan) << endl;

    double snan = numeric_limits<double>::signaling_NaN();
    cout << "fmin with snan: " << fmin(1.0, snan) << endl;
    cout << "fmax with snan: " << fmax(1.0, snan) << endl;

    return 0;
}

$ g++ fminmax.cpp && ./a.out
fmin with qnan: 1
fmax with qnan: 1
fmin with snan: nan
fmax with snan: nan