rust-lang / packed_simd

Portable Packed SIMD Vectors for Rust standard library
https://rust-lang.github.io/packed_simd/packed_simd_2/
Apache License 2.0
589 stars 74 forks source link

Account some warnings to fix CI #315

Closed JohnTitor closed 3 years ago

JohnTitor commented 3 years ago

Fixes #5

Lokathor commented 3 years ago

Looks like CI is less than fully fixed :/

JohnTitor commented 3 years ago

Ouch, checking now...

JohnTitor commented 3 years ago

Failures:

---- v128::i8x16_ops_scalar_shifts::ops_scalar_shifts stdout ----

thread 'v128::i8x16_ops_scalar_shifts::ops_scalar_shifts' panicked at 'assertion failed: `(left == right)`

  left: `i8x16(1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0)`,

 right: `i8x16(1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1)`', src/v128.rs:6:1

---- v128::i8x16_ops_vector_rotate::rotate_ops stdout ----

thread 'v128::i8x16_ops_vector_rotate::rotate_ops' panicked at 'assertion failed: `(left == right)`

  left: `i8x16(1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0)`,

 right: `i8x16(1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1)`', src/v128.rs:6:1

---- v128::i8x16_ops_vector_shifts::ops_vector_shifts stdout ----

thread 'v128::i8x16_ops_vector_shifts::ops_vector_shifts' panicked at 'assertion failed: `(left == right)`

  left: `i8x16(1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0)`,

 right: `i8x16(1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1)`', src/v128.rs:6:1

---- v128::u8x16_ops_scalar_shifts::ops_scalar_shifts stdout ----

thread 'v128::u8x16_ops_scalar_shifts::ops_scalar_shifts' panicked at 'assertion failed: `(left == right)`

  left: `u8x16(1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0)`,

 right: `u8x16(1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1)`', src/v128.rs:11:1

---- v128::u8x16_ops_vector_rotate::rotate_ops stdout ----

thread 'v128::u8x16_ops_vector_rotate::rotate_ops' panicked at 'assertion failed: `(left == right)`

  left: `u8x16(1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0)`,

 right: `u8x16(1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1)`', src/v128.rs:11:1

---- v128::u8x16_ops_vector_shifts::ops_vector_shifts stdout ----

thread 'v128::u8x16_ops_vector_shifts::ops_vector_shifts' panicked at 'assertion failed: `(left == right)`

  left: `u8x16(1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0)`,

 right: `u8x16(1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1)`', src/v128.rs:11:1

failures:

    v128::i8x16_ops_scalar_shifts::ops_scalar_shifts

    v128::i8x16_ops_vector_rotate::rotate_ops

    v128::i8x16_ops_vector_shifts::ops_vector_shifts

    v128::u8x16_ops_scalar_shifts::ops_scalar_shifts

    v128::u8x16_ops_vector_rotate::rotate_ops

    v128::u8x16_ops_vector_shifts::ops_vector_shifts

test result: FAILED. 2544 passed; 6 failed; 0 ignored; 0 measured; 0 filtered out; finished in 2.26s

So something goes wrong on AArch64 since CI passed, maybe related to the recent LLVM bumps? I don't see any related commits here. I'd like to file this as an issue and skip tests for now if it's reasonable. @Lokathor Thoughts?

workingjubilee commented 3 years ago

We've uncovered aarch64 optimization-related LLVM bugs in https://github.com/rust-lang/stdsimd/pull/80 so I would agree: it seems that something in aarch64 codegen is busted (maybe always, maybe only now) and it's likely not our fault. But the thought of shrugging off what appears to be a new behavioral deviation for tier 1 targets makes me itch.

I wouldn't find skipping the tests entirely unacceptable, just... wow, the bit rot.

JohnTitor commented 3 years ago

But the thought of shrugging off what appears to be a new behavioral deviation for tier 1 targets makes me itch.

But on the other hand, this also hinders further tests on CI (https://travis-ci.com/github/rust-lang/packed_simd/builds/220445170). I totally agree we shouldn't ignore this issue but... I'm not sure the fix here is straightforward.

workingjubilee commented 3 years ago

Well, this is blocking landing any other maintenance, so might as well I guess.

Lokathor commented 3 years ago

Looking at the changes, it's just tests changing right? So the code itself is no worse with this PR, it's only as broken as it might have been before. If we want to merge this we should and then we can continue later once the llvm bug is addressed.

JohnTitor commented 3 years ago

Okay, CI still has a lot of failures but at least we pass all the non-allow-failure jobs. I'm going to fix some of them but meanwhile, we can merge this to unblock other PRs?