Skip to content

PartialEq is fundamentally broken for stdsimd types #511

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
gamozolabs opened this issue Jun 29, 2018 · 3 comments
Closed

PartialEq is fundamentally broken for stdsimd types #511

gamozolabs opened this issue Jun 29, 2018 · 3 comments

Comments

@gamozolabs
Copy link

gamozolabs commented Jun 29, 2018

The following code triggers an assertion, which checks if !(==) is the same as !=.

In this case the issue only occurs when there are different values in the vector. The current tests only check that PartialEq is working in the splat against splat case, which it does work correctly. However when there is a different value in the vector (non-splatted), PartialEq is incorrect.

This issue also occurs when using u32x4 and mask values. It does not seem to be an issue with just the 512-bit emulation.

Tests should also be expanded to test non-splatted cases.

#![feature(stdsimd)]

use std::simd::u32x16;

fn main() {
    let foo = u32x16::new(
        0, 0, 0, 0, 0, 0, 0, 0,
        0, 0, 0, 0, 0, 0, 0, 1,
    );
    let target = u32x16::splat(1);
    
    print!("Are they different? {}\n", foo != target);
    print!("Are they equal?     {}\n", foo == target);
    
    assert!((foo != target) == !(foo == target));
}

This will result in

Standard Error
   Compiling playground v0.0.1 (file:///playground)
    Finished dev [unoptimized + debuginfo] target(s) in 1.31s
     Running `target/debug/playground`
thread 'main' panicked at 'assertion failed: (foo != target) == !(foo == target)', src/main.rs:15:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.

Standard Output
Are they different? false
Are they equal?     false

https://play.rust-lang.org/?gist=9e0799d170e488b7c691b6f98ccc72ca&version=nightly&mode=debug

-B

@gamozolabs
Copy link
Author

Related #457

Testing was done on x86_64 in my case.

@gamozolabs
Copy link
Author

Bug seems pretty simple. ne() is implemented as $id::ne(*self, *other).all() where it should be $id::ne(*self, *other).any()

This is in impl_partial_eq

I've never done a pull request but I will attempt one. Sorry if I do it wrong :P

-B

gamozolabs pushed a commit to gamozolabs/stdsimd that referenced this issue Jun 29, 2018
gnzlbg pushed a commit that referenced this issue Jul 2, 2018
@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 2, 2018

This was closed in 4751cb9

@gnzlbg gnzlbg closed this as completed Jul 2, 2018
lu-zero pushed a commit to lu-zero/stdarch that referenced this issue Jul 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants