Skip to content
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

some 32 bit systems (armel, m68k, powerpc, sh4) don't link to atomic symbols #169

Open
drew-parsons opened this issue Nov 16, 2023 · 3 comments

Comments

@drew-parsons
Copy link

Some 32 bit systems (armel, m68k, powerpc, sh4) don't link to atomic symbols. The problem is under discussion in gcc at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81358

This patch works around it for netgen, linking libngcore to libatomic where needed

Index: netgen/libsrc/core/CMakeLists.txt
===================================================================
--- netgen.orig/libsrc/core/CMakeLists.txt      2023-11-12 16:18:12.039784202 +0100
+++ netgen/libsrc/core/CMakeLists.txt   2023-11-12 16:20:53.737110429 +0100
@@ -60,6 +60,24 @@
     target_link_libraries(ngcore PRIVATE ${NUMA_LIBRARY})
 endif(USE_NUMA)

+# some 32-bit arches do not automatically link to atomic symbols (util.hpp)
+# cf. https://github.com/google/highway/pull/1008
+check_cxx_source_compiles(
+"#include <atomic>
+#include <cstdint>
+std::atomic<uint8_t> n8 (0);   // basic (should not fail)
+std::atomic<uint64_t> n64 (0); // expected to fail on armel, mipsel, powerpc (undefined reference to `__atomic_fetch_add_8')
+int main() {
+  ++n8;
+  ++n64;
+  return 0;
+}"
+ HAS_INTERNAL_ATOMIC
+)
+if(NOT HAS_INTERNAL_ATOMIC)
+  target_link_libraries(ngcore INTERFACE atomic)
+endif()
+
 install(TARGETS ngcore DESTINATION ${NG_INSTALL_DIR} COMPONENT netgen)

 target_link_libraries(ngcore PUBLIC netgen_mpi PRIVATE "$<BUILD_INTERFACE:netgen_python>" ${CMAKE_THREAD_LIBS_INIT})

atomic needs to be linked to ngcore as INTERFACE not PRIVATE (or PUBLIC) since libngcore.so itself does not use atomic symbols (they are used inline in core/utils.hpp).

The cmake test for atomic linking was adapted from google/highway#1008

Originally posted by @drew-parsons in #168 (comment)

@barracuda156
Copy link

@drew-parsons Is this still an issue? I have just built netgen on MacOS PowerPC (32-bit), which normally does need libatomic linking. Looks like the build does not use 8-byte atomics.

P. S. Or maybe some of our Macports options disable usage of those.

@drew-parsons
Copy link
Author

Still needed for 6.2.2305 building with gcc 13.2.0.

@barracuda156
Copy link

ngsolve 6.2.2307 by the way needs it for sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants