BrunoLevy / geogram

a programming library with geometric algorithms
Other
1.8k stars 122 forks source link

fixup! Add support for non-x86 arches (tested on riscv64) #139

Closed kxxt closed 5 months ago

kxxt commented 5 months ago

I am dumb and forgot to commit this change in my previous PR.

BrunoLevy commented 5 months ago

Merged ! Once you confirm that everything works with the ArchLinux package, I can make a new release.

kxxt commented 5 months ago

Once you confirm that everything works with the ArchLinux package, I can make a new release.

Thank very much! I didn't set up the required dependencies to run the tests. But the demos runs fine on riscv64 Arch Linux:

image

image

BrunoLevy commented 5 months ago

Awesome ! Thanks ! (I have twitted the image ;-) If you want to run the testsuite, instructions are here

kxxt commented 5 months ago

If you want to run the testsuite, instructions are here

There are 2 failing tests in smoke tests due to segfault. The test logs are attached here:

smoke-test-logs.zip

BrunoLevy commented 5 months ago

Thanks ! I think it will be easy to debug. It seems it crashes when using a multithread algoritm (Delaunay) with a very small pointset (8 points), how many threads do you have ?

To see what happens, you can compile in debug mode and restart the test suite, there are many assertion checks in debug mode, so maybe we'll be lucky and it will directly tell us what's going on.

else it is also possible to run manually the two commands that crashed in the debugger, as follows:

gdb --args /home/kxxt/geogram/build/Linux64-nonx86-gcc-dynamic-Debug/bin/compute_delaunay /home/kxxt/geogram/tests/data/Small/cube.obj dbg:delaunay_benchmark=true dbg:delaunay_verbose=true
run
where

and

gdb --args /home/kxxt/geogram/build/Linux64-nonx86-gcc-dynamic-Release/bin/compute_delaunay /home/kxxt/geogram/tests/data/Small/cube.obj dbg:delaunay_benchmark=true convex_hull=true dbg:delaunay_verbose=true
run
where
kxxt commented 5 months ago

how many threads do you have ?

There are 64 riscv cores

there are many assertion checks in debug mode, so maybe we'll be lucky and it will directly tell us what's going on.

Yes. It's an out of bound access:

(gdb) bt
#0  0x0000003f86f12200 in ?? () from /usr/lib/libc.so.6
#1  0x0000003f86edcade in raise () from /usr/lib/libc.so.6
#2  0x0000003f86ecc314 in abort () from /usr/lib/libc.so.6
#3  0x0000003f87506132 in GEO::geo_abort () at /home/kxxt/geogram/src/lib/geogram/basic/assert.cpp:89
#4  0x0000003f8750638c in GEO::geo_assertion_failed (condition_string="i < size()", file="/home/kxxt/geogram/src/lib/geogram/basic/memory.h", line=688) at /home/kxxt/geogram/src/lib/geogram/basic/assert.cpp:117
#5  0x0000003f8750cbf0 in GEO::vector<unsigned int>::operator[] (this=0x2af9679678, i=56) at /home/kxxt/geogram/src/lib/geogram/basic/memory.h:688
#6  0x0000003f878006ec in GEO::Delaunay3dThread::Delaunay3dThread (this=0x2af967ccd0, master=0x2af9679560, pool_begin=0, pool_end=0) at /home/kxxt/geogram/src/lib/geogram/delaunay/parallel_delaunay_3d.cpp:166
#7  0x0000003f877fcde6 in GEO::ParallelDelaunay3d::set_vertices (this=0x2af9679560, nb_vertices=8, vertices=0x2af967afc0) at /home/kxxt/geogram/src/lib/geogram/delaunay/parallel_delaunay_3d.cpp:2716
#8  0x0000002acad7226c in main (argc=5, argv=0x3fde142438) at /home/kxxt/geogram/src/examples/geogram/compute_delaunay/main.cpp:339
BrunoLevy commented 5 months ago

Wow, 64 cores !

Oooh, I think I see what happens: we have more cores than vertices in the testcase, which is a situation I never encountered before. I'll be able to fix it (and to test on one of the big machines of the lab)