Skip to content
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

Weird behaviour, possibly bug, in comparison functions for f32x4 #34

Open
nrc opened this issue Nov 12, 2016 · 5 comments
Open

Weird behaviour, possibly bug, in comparison functions for f32x4 #34

nrc opened this issue Nov 12, 2016 · 5 comments

Comments

@nrc
Copy link

nrc commented Nov 12, 2016

The result of le and ge seems to be either 0 or -1, I'm not sure which is true and which is false. If elements are equal, I seem to see different values for le and ge. E.g., I'd expect the following to hold:

f32x4::splat(0.0).le(f32x4::splat(0.0)) == f32x4::splat(0.0).ge(f32x4::splat(0.0))

That doesn't seem to hold, and furthermore, neither does

f32x4::splat(0.0).le(f32x4::splat(0.0)).all() && f32x4::splat(0.0).ge(f32x4::splat(0.0)).all()

Finally, the docs for these functions are all wrong - they are described as "equality" but clearly shouldn't be.

@liebharc
Copy link
Contributor

I haven't seen the author of this lib responding in a while, so I will try to help with some information.

This crate is basically a wrapper for platform intrinsics. The ge and le are simply implemented by using the Rust compiler intrinsic simd_ge and simd_le. The Rust intrinsics again use llvm intrinsics and in the end your code will be compiled into a single SIMD instruction. That just means that you can take a look at the SSE and AVX instruction sets (e.g. the Intel documentation).

For _m128 _mm_cmpge_ps there is this example:

FOR j := 0 to 3 i := j*32 
    dst[i+31:i] := ( a[i+31:i] >= b[i+31:i] ) ? 0xffffffff : 0 
ENDFOR

So 0xffffffff = -1 is true while 0 is false. I agree with your comment that the documentation would need an update. Going back to the Intel documentation was my workaround for the documentation issue while I was working with this. So that's the reason why I'm bringing it up.

For your examples:

f32x4::splat(0.0).le(f32x4::splat(0.0)) == f32x4::splat(0.0).ge(f32x4::splat(0.0))

That doesn't compile for me since simd::bool32fx4 doesn't implement std::cmp::PartialEq. Would however be nice if it would.

f32x4::splat(0.0).le(f32x4::splat(0.0)).all() && f32x4::splat(0.0).ge(f32x4::splat(0.0)).all()

This compiled and results in true which I would expect since 0.0 is indeed greater or equal to 0.0.

I'm right now on rustc 1.14.0-nightly (f09420685 2016-10-20) and did the tests withsimd 0.1.1.

Your github profile seems to indicate that you are working for Mozilla. The author of this lib wrote it while doing an internship at Mozilla. Do you by chance know if there are further plans for this lib?

@nrc
Copy link
Author

nrc commented Nov 13, 2016

Thanks for the explanations! I should try and get my exact examples, the ones I gave were simplifications. To summarise I had (x, y, z, 0) where x, y, z could be any number. Since all my values have this form, I'd expect the last land of any of these comparisons to always be true, but that did not seem to be the case.

I'm not on the Rust libs team, so I don't know exactly what is happening here. I do know that SIMD is very much on peoples' mind though, so something should be happening soon. Huon is no longer able to work on Rust stuff, sadly.

@liebharc
Copy link
Contributor

liebharc commented Nov 13, 2016

You're welcome. Yes that's sad news, Huon did some interesting things and was nice to engage with.

If I understand your problem correctly then you were using something like this:
f32x4::new(0.0, 1.0, 2.0, 0.0).ge(f32x4::new(1.0, 0.0, 2.0, 0.0))
which for me results in bool32fx4(0, -1, -1, -1) and that looks indeed quite wrong - or not matching the definition I quoted from the Intel specification. Glancing through the code ge just calls the intrinsic simd_ge. Not sure what is going wrong here correct. Sorry I thought I would have seen the issue you described but in fact I was only confused myself and the result seems to be okay.

I watch this project since about a year now and it seems that people are using it and occasionally people are even coming back with pull requests or questions how they could enhance this lib. So it seems that there is a community interest in keeping this lib alive. Would it perhaps make sense to rehost this lib under a different account (perhaps a generic account such as e.g. the num lib uses)? So while Mozilla might have not the resources to keep working on this right now, there might be still some progress achieved by other contributors. Is this a question best address to https://internals.rust-lang.org/?

@nrc
Copy link
Author

nrc commented Nov 14, 2016

Sounds like a good idea. You could ask around on #rust-libs to see if there are plans already. Otherwise, posting on internals is a good next step. I expect the GH orgs would be either rust-nursery or rust-unofficial

@BurntSushi
Copy link

@liebharc I know you popped into #rust-libs, but for anyone else following along at home, there is a fairly recent thread on internals about the next course of action: https://internals.rust-lang.org/t/getting-explicit-simd-on-stable-rust/4380

For the most part, we're electing to punt on abstractions like you find in the simd crate here, and instead want to get the fundamentals available on stable Rust so that folks can start iterating. I don't think there are any specific plans (as far as the libs team goes) for this particular crate, and I think that's mostly because there is a lot of variation on what the right abstraction should be. Indeed, I'm vaguely aware that some of the nicer abstractions are actually possible in Rust yet (like integer generics?).

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

3 participants