Skip to content

Unit tests are constant-folded #86

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

Closed
nominolo opened this issue Sep 30, 2017 · 7 comments
Closed

Unit tests are constant-folded #86

nominolo opened this issue Sep 30, 2017 · 7 comments

Comments

@nominolo
Copy link
Contributor

I noticed that some of our unit tests get constant folded which probably defeats their purpose. For example, when compiled with cargo +nightly test --release the test

    #[simd_test = "sse"]
    unsafe fn _mm_mul_ps() {
        let a = f32x4::new(-1.0, 5.0, 0.0, -10.0);
        let b = f32x4::new(-100.0, 20.0, 0.0, -5.0);
        let r = sse::_mm_mul_ps(a, b);
        assert_eq!(r, f32x4::new(100.0, 100.0, 0.0, 50.0));
    }

compiles to:

otool -tv target/release/deps/stdsimd-c75022df61552be7 | grep --color -A50 _mm_mul_ps
...
__ZN7stdsimd3x863sse5tests10_mm_mul_ps17h039fc36f66a51b81E:
0000000100003ea0	pushq	%rbp
0000000100003ea1	movq	%rsp, %rbp
0000000100003ea4	popq	%rbp
0000000100003ea5	retq

That looks like a no-op to me, so I suspect that this test is constant-folded by the compiler.

@alexcrichton
Copy link
Member

I suspect we can inhibit this with liberal usage of test::black_box, and we could perhaps provide some test-specific constructors which also automatically perform this sort of optimization inhibiting.

@alexcrichton
Copy link
Member

In #88 looks like black_box passed tests.

@BurntSushi do you think we should use black_box for all inputs on all tests?

@BurntSushi
Copy link
Member

Probably yeah :-/

@gnzlbg
Copy link
Contributor

gnzlbg commented Oct 12, 2017

If they are constant folded, doesn't that mean they are correct?

@nominolo
Copy link
Contributor Author

Under certain assumptions, yes.

We generally trust that the compiler's constant-folding rules are correct. However, they are actually not guaranteed to be correct under all circumstances.

On test case failed because of constant folding. Floating point instructions actually can have side effects on certain processor-specific status registers (MXCSR on x86 / _mm_getcsr instrinsic) which is not considered by the compiler. So a test case trying to observe that side effect will break if the compiler removes the floating point instruction through constant folding.

Another problem I can think of might be that an emitted instruction could actually fail at runtime under certain circumstances (e.g., CPU feature not supported / invalid instruction on certain platforms). You cannot rely on the compiler's constant folding code to take those specific details into account.

Platform-specific intrinsics are generally not on the highly-tested code paths of the compiler, so we want to make sure that our test cases actually test something more than just the constant-folding parts of the compiler.

@gnzlbg
Copy link
Contributor

gnzlbg commented Oct 12, 2017

@nominolo

On test case failed because of constant folding. Floating point instructions actually can have side effects on certain processor-specific status registers (MXCSR on x86 / _mm_getcsr instrinsic) which is not considered by the compiler. So a test case trying to observe that side effect will break if the compiler removes the floating point instruction through constant folding.

I'll be ok by working around compiler bugs by using black_box in these cases, but I'd like each place where black_box is used for this to have a comment with a link to the bug that it is working around (be it in LLVM, rustc, or wherever it is).

Platform-specific intrinsics are generally not on the highly-tested code paths of the compiler, so we want to make sure that our test cases actually test something more than just the constant-folding parts of the compiler.

We should arguably tests both cases because our users won't probably be using black_box to avoid constant folding.

@alexcrichton
Copy link
Member

I'm going to close this because I think these aren't constant folded in debug mode so we should be getting some good testing there, and otherwise the constant folding probably means that the intrinsic is correct!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants