-
Notifications
You must be signed in to change notification settings - Fork 309
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
Test errors on PowerPC: 85% tests passed, 16 tests failed out of 110 #316
Comments
@jmr Could you please take a look? |
The ones involving encoding/decoding are expected. Most of that code assumes little endian. There should be more checks like this. They could be fixed if you can find all the places and do it in a way that doesn't affect little-endian performance.
Some of the other failures are surprising.
You'll have to debug these since I don't have access to the architecture. |
@jmr By the way, assumed LE is a bug introduced in the current version or it existed all through? |
I wouldn't call it a bug. It's an intended limitation since big endian is no longer common. It's always been like this, it's just that more encode/decode classes/functions have been added over the years. |
Well, I am not hugely surprised that MacOS PPC is not specifically supported, but there are plenty of current BE systems around: several BSDs, several versions of Linux, AIX. And modern CPUs supporting BE (even though being bi-endian). Also, if something genuinely cannot be supported, it should be rather skipped in tests or marked as known fail. |
@jmr Maybe you could use Qemu on Linux? Big-endian issues will be common to any OS, so any Linux distro should do, or a BSD system. |
Test results from
I.e. nothing changed. |
At this point big endian is uncommon enough that it's not a priority. Whoever wants this will need to send a PR. If you want to give this a shot, the little-endian code path should be unaffected or close to it. There are some classes that could help, like: https://github.com/google/s2geometry/blob/master/src/s2/util/endian/endian.h |
You need to find places that are doing a plain load / unaligned load and change them to use For example: s2geometry/src/s2/encoded_uint_vector.h Line 177 in 418c558
Based on the tests that are failing, it should be easy to find most of the places. Calls to |
The text was updated successfully, but these errors were encountered: