-
Notifications
You must be signed in to change notification settings - Fork 5
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
Two tests fail on s390x #6
Comments
Hi, at first glance, I suspect the hash function used in the tests does not match the hash function in the crate itself. The spec encodes everything in LE from my understanding. I'll need to inspect further. |
Thanks! I don't have big endian machine to offer either, sorry - all I have is a way to kick off builds in the Fedora build system. |
I ran the two functions side by side on my Windows machine (LE) and they seem fine. I will download qemu and try to set up a BE environment to further test things. I don't know if this repo is still maintained. |
Maintained but not actively so. If there's clear issues though, we can investigate and fix. |
Ah, thank you for your response. I'll investigate the issue further and update here. LMK if you have any idea where things go wrong. |
Yes, my suspicion was correct. fn hash_iter(px: [u8; 4]) -> u8 {
px.into_iter()
.zip([0x03u8, 0x05, 0x07, 0x0b].into_iter())
.map(|p| p.0.wrapping_mul(p.1))
.fold(0u8, |acc, x| acc.wrapping_add(x)) & 63
} This code works as intended on both LE and BE. |
My code seems to be slower than the bitwise solution:
|
With some help, I was able to fix the bitwise code to be cross-platform. I ran all the tests on both LE and BE (miri) systems. |
I can confirm that this makes the two tests pass on s390x. Thanks a lot, @AregevDev! |
I am packaging qoi-rust for Fedora and as part of that, we try to run the test suite on all architectures that are available in the build system. On s390x, two tests fail:
Possibly some endianness issue as it is the only big endian architecture that we have?
The text was updated successfully, but these errors were encountered: