32-bit tests failing on master

[alexcrichton]

Starting with last night's nightly our CI is failing on 32-bit platforms for reasons that look like:

---- coresimd::x86::sse2::tests::test_mm_add_sd stdout ----
thread 'main' panicked at '__m128d(NaN, 2.0) != __m128d(6.0, 2.0)', crates/coresimd/src/../../../coresimd/x86/test.rs:26:9
note: Run with `RUST_BACKTRACE=1` for a backtrace.

---- coresimd::x86::sse2::tests::test_mm_cvtsd_f64 stdout ----
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `NaN`,
 right: `-1.1`', crates/coresimd/src/../../../coresimd/x86/sse2.rs:4986:9

---- coresimd::x86::sse2::tests::test_mm_div_sd stdout ----
thread 'main' panicked at '__m128d(NaN, 2.0) != __m128d(0.2, 2.0)', crates/coresimd/src/../../../coresimd/x86/test.rs:26:9

---- coresimd::x86::sse2::tests::test_mm_mul_sd stdout ----
thread 'main' panicked at '__m128d(NaN, 2.0) != __m128d(5.0, 2.0)', crates/coresimd/src/../../../coresimd/x86/test.rs:26:9

---- coresimd::x86::sse2::tests::test_mm_sqrt_pd stdout ----
thread 'main' panicked at '__m128d(1.0, 1.4142135623730951) != __m128d(NaN, 1.4142135623730951)', crates/coresimd/src/../../../coresimd/x86/test.rs:26:9

---- coresimd::x86::sse::tests::test_mm_cvtss_f32 stdout ----
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `NaN`,
 right: `312.0134`', crates/coresimd/src/../../../coresimd/x86/sse.rs:3549:9

---- coresimd::x86::sse::tests::test_mm_rcp_ps stdout ----
thread 'main' panicked at 'assertion failed: `(left !== right)` (left: `NaN`, right: `0.24993896`, expect diff: `0.0009765625`, real diff: `NaN`)', crates/coresimd/src/../../../coresimd/x86/sse.rs:2670:13

---- coresimd::x86::sse::tests::test_mm_set1_ps stdout ----
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `NaN`,
 right: `4.25`', crates/coresimd/src/../../../coresimd/x86/sse.rs:3562:9

---- coresimd::x86::sse::tests::test_mm_stream_ps stdout ----
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `7.0`,
 right: `NaN`', crates/coresimd/src/../../../coresimd/x86/sse.rs:3973:13


failures:
    coresimd::x86::sse2::tests::test_mm_add_sd
    coresimd::x86::sse2::tests::test_mm_cvtsd_f64
    coresimd::x86::sse2::tests::test_mm_div_sd
    coresimd::x86::sse2::tests::test_mm_mul_sd
    coresimd::x86::sse2::tests::test_mm_sqrt_pd
    coresimd::x86::sse::tests::test_mm_cvtss_f32
    coresimd::x86::sse::tests::test_mm_rcp_ps
    coresimd::x86::sse::tests::test_mm_set1_ps
coresimd::x86::sse::tests::test_mm_stream_ps

I've done some debugging and I don't think this is a regression, but rather this has always been a bug and we just haven't exposed it until now. I've been able to reproduce some of the above failures with:

#![feature(stdsimd)]
#![feature(mmx_target_feature)]

use std::arch::x86::*;

fn main() {
    unsafe {
        something_mmx();
        foo();
    }
}

#[target_feature(enable = "mmx")]
unsafe fn something_mmx() {
    another(_mm_setzero_si64());

    #[target_feature(enable = "mmx")]
    unsafe fn another(_: __m64) {
    }
}


#[target_feature(enable = "sse2")]
unsafe fn foo() {
    let a = _mm_setr_pd(1.0, 2.0);
    let b = _mm_setr_pd(5.0, 10.0);
    let r = _mm_add_sd(a, b);
    assert_eq_m128d(r);
}


#[target_feature(enable = "sse2")]
unsafe fn assert_eq_m128d(a: __m128d) {
    println!("{:?}", a);
}

That program has an invalid NaN in the end, but if you comment out the call to something_mmx() it passes.

My best guess as to what's going on is that MMX is causing things to be sad. Something about floating point state and whatnot seems to be persisting across executions. I've been able to determine some bad stuff starts from a fld instruction, which apparently loads a floating point operand onto the floating point stack. The operand being loaded is 1.0 as a double, but when loaded onto the stack it loads as -NaN for whatever reasons. The fctrl register (according to GDB) differs between the program execute with the call to something_mmx and the program execution without something_mmx, and presumably that's like masking something or tweaking something?

I'm sort of lost at this point and curious if others have any idea what's going on here? Does using MMX things somehow break global state for the rest of the CPU?

[gnzlbg]

cc @rkruppe

[gnzlbg]

This example does not have the issue:

#![feature(stdsimd, asm)]
#![feature(mmx_target_feature)]

use std::arch::x86::*;

fn main() {
    unsafe {
        something_mmx();
        foo();
    }
}

#[target_feature(enable = "mmx")]
unsafe fn something_mmx() {
    another(_mm_setzero_si64());

    #[target_feature(enable = "mmx")]
    unsafe fn another(_: __m64) {
    }

    asm!("emms" : : : : "volatile")
}


#[target_feature(enable = "sse2")]
unsafe fn foo() {
    let a = _mm_setr_pd(1.0, 2.0);
    let b = _mm_setr_pd(5.0, 10.0);
    let r = _mm_add_sd(a, b);
    assert_eq_m128d(r);
}


#[target_feature(enable = "sse2")]
unsafe fn assert_eq_m128d(a: __m128d) {
    println!("{:?}", a);
}

[alexcrichton]

Fascinating! If that's the case then I suspect this is a "regression" from rust-lang/rust#56243 because all tests are run in the same thread now instead of all tests being run in a separate thread.

I'm still not really sure what's going on with these registers, but the documentation for the emms instruction say "you must always execute when you're done with MMX operations", so seems scary enough that we should pass it.

In any case I think I can now fix this on CI!

[gnzlbg]

I think we should also fill an LLVM bug about this. AFAICT we are using the x86_mmx type and the mmx target feature correctly, so I'd expect LLVM to handle the interfacing of MMX and x87 FPU code correctly. Users shouldn't need to clean up the MMX state before using floating point registers manually with emms.

[gnzlbg]

These seem to hint at this issue:

https://bugs.llvm.org/show_bug.cgi?id=15388
https://bugs.llvm.org/show_bug.cgi?id=35982

If they are not exactly the same issue, they are at least closely related with what's going on here.

[gnzlbg]

@alexcrichton did you had to use emms explicitly anywhere to fix this ?

#638 adds a proper intrinsic for that (_mm_empty()).

[alexcrichton]

Yes that wasn't bound but I wasn't keen on adding that to every test using mmx...

[gnzlbg]

I wasn't keen on adding that to every test using mmx...

I'll see if I can update the simd_test macro to this automatically if the target-feature list contains mmx (EDIT: #647 does exactly this).