BrunoLevy / geogram

a programming library with geometric algorithms
Other
1.87k stars 128 forks source link

Missing support for Linux on aarch64 #74

Closed hesiod closed 8 months ago

hesiod commented 1 year ago

While packaging geogram for nixpkgs (see relevant PR), we noticed that Geogram is missing proper detection for Linux on aarch64 (even though Darwin on aarch64 and Linux on 32-bit ARM are supported). Geogram would probably compile fine on aarch64, but currently the atomics header assumes it is compiled for x86 when compiling on aarch64 (GEO_USE_X86_ATOMICS is set), resulting in compile errors (see log output).

BrunoLevy commented 1 year ago

It will require a bit of work (the other architectures requires some assembly), but I can try with std::atomic (but I'm a bit unsure of performance. I'll test that, and if it works it will remove a lot of #ifdefs.

BrunoLevy commented 1 year ago
kevinbentley commented 1 year ago

Any update on this issue? I'd love to be able to use geogram on a project I'm using. I'm no ARM assembly expert, but I think I can port the arm32 code in atomic.h to aarch64 assembly and add some ifdefs, but I don't want to go down that road if there's a better way. I'm willing to put in the work on this, but I have a lot to learn about the internals of geogram. Any advice/direction you could give me is appreciated!

BrunoLevy commented 1 year ago

Hi Kevin, I'd recommend to try using std::atomics, which probably generates assembly as good as one can do manually. I did not have the time to test, but I plan to remove most of these #ifdefs and replace them with portable C++. But I need to measure the impact of performance, which will require some time to do it seriously, and this is why I did not do it before (I am now on a project with more priority on mesh intersection, I plan to do that, but I cannot tell exactly when)

BrunoLevy commented 8 months ago

Since it is based on std::atomic, v1.8.8 should work with aarch64