Open barracuda156 opened 1 year ago
@jmr Could you please take a look? s2_ppc_tests.log
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.
F0000 00:00:1684583674.764805 1 encoded_s2point_vector.cc:172] Not implemented on big-endian architectures
Some of the other failures are surprising.
[ RUN ] S2Cap.GetCellUnionBoundLevel1Radius
/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_math_s2geometry/s2geometry/work/s2geometry-7773d518b1f29caa1c2045eb66ec519e025be108/src/s2/s2cap_test.cc:308: Failure
Expected equality of these values:
3
covering.size()
Which is: 6
[ FAILED ] S2Cap.GetCellUnionBoundLevel1Radius (0 ms)
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.
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). Healthy code should be bi-endian as long as technology is bi-endian. IMO.
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 s2geometry
v. 0.11.1:
85% tests passed, 16 tests failed out of 110
Total Test time (real) = 828.31 sec
The following tests FAILED:
1 - encoded_s2cell_id_vector_test (Failed)
2 - encoded_s2point_vector_test (Subprocess aborted)
3 - encoded_s2shape_index_test (Subprocess aborted)
4 - encoded_string_vector_test (Failed)
5 - encoded_uint_vector_test (Failed)
7 - mutable_s2shape_index_test (Failed)
11 - s1chord_angle_test (Failed)
28 - s2cap_test (Failed)
51 - s2edge_crossings_test (Failed)
62 - s2lax_polygon_shape_test (Subprocess aborted)
63 - s2lax_polyline_shape_test (Subprocess aborted)
72 - s2point_test (Failed)
78 - s2polygon_test (Failed)
83 - s2predicates_test (Failed)
86 - s2region_test (Failed)
97 - s2shapeutil_coding_test (Subprocess aborted)
Errors while running CTest
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 LittleEndian::Load64/32/...()
.
For example: https://github.com/google/s2geometry/blob/418c55893f6123b90f2768ed2ec9f5f47fa512de/src/s2/encoded_uint_vector.h#L177
Based on the tests that are failing, it should be easy to find most of the places.
Calls to Encoder::putn
and Decoder::getn
will need to be updated to to handle the byte swapping. It couldn't hurt to look at all uses of Encoder
/Decoder
.