Hi all, hi Joseph & Jakub, BACKGROUND:
The main reason I am interested in this is offloading where OpenACC/OpenMP code might run into: unresolved symbol __atomic_compare_exchange_16 and it is nontrivial to find out that the solution is: -foffload=nvptx-none=-latomic And the atomic use can appear for innocent looking code. (The dependency comes via libgomp/config/nvptx/atomic.c for __sync_val_compare_and_swap_16.) However, as PR81358 shows, also for normal C (and C++) code it would be nice as atomics are part of the language. Target specific: for RISC-V (with -as-needed support only with pthread, otherwise unconditionally) – and seemingly for RTEMS – -latomic is already linked via LIB_SPEC. My initial plan was to add -latomic to libgomp.spec - if built for that target and using -as-needed if available, cf. last but one email in the thread https://gcc.gnu.org/pipermail/gcc-patches/2020-October/thread.html#555298 (my initial RFC question, quoted in the first email, was in September) However, adding it there was opposed by Jakub as: "I think for libgomp.spec we should add it solely for the offloading targets, neither GCC generated code for OpenMP construct nor libgomp itself needs -latomic on the hosts." (Albeit for nvptx, see above.) SOLUTION PART 1: Add configure option --enable-autolink-libatomic [See attached patch (only configure check)] Example for an explicit use: enable it for offloading targets (which enables it for nvptx but not for amdgcn which does not build libatomic). — And for the host, you may want to enable if you always install libatomic and -as-needed is supported and otherwise, you might want to disable it. QUESTION: * Should the default of --enable-autolink-libatomic default to 'yes' or 'no'? * Should the default additionally depend on having -as-needed support? (Which would exclude nvptx with default settings.) SOLUTION PART 2: Actually linking libatomic ? QUESTION: I have no idea where to add the -latomic. (a) Still do it in libgomp.spec but now with that configure flag to be able to disable it? Pro: + Solves problem for nvptx, which adds atomic code to libgomp/config/nvptx/atomic.c + rather minimal invasive and by tuning the config option it could be used when needed Con: - While OpenMP has atomics, libgomp by itself in general does not depend on libatomic for most targets - this can be mitigated by using the configure flag but it is not ideal, either. - C code can use atomic directly and thus it would be nice if it could be automatically linked. (Especially if the target supports the -as-needed linker flag.) (b) Do it in the driver Pro: + will automatically work for C/C++ atomic code + with -as-needed it will only be linked if really needed (caveat: lib file has to be present at link time.) Con: - If -as-needed is not supported and it is always linked, this adds unneccessary dependencies (shared lib) or file size (static lib) - Adding files for the linker to analyze does not help with the compile size - The file has to be present, even if -as-needed is supported, adding a hard dependency on the libatomic library for Linux distros For doing it in the driver, I am not sure when to add it. Used: - Direct consumer is C/C++ using atomics. - For Fortran, it (currently) is only used for with nvptx offloading as described above. - For offloading, the compilation/linking handling is slightly different and it needs to work there as well. - No idea about Ada, D, or other direct or indirect dependencies RISC-V + RTEMS use LIB_SPEC to add it. One possibility would be to add it to the init_gcc_specs call, but that feels like a sledge hammer solution. Question: Where do you think should it be in the driver? Other thoughts? Or is solution (a) better? (That is: previous patch + new --enable-autolink-libatomic for libgomp, only.) Which is kind of a complicated nvptx-offloading-only solution? Tobias PS: I assume -static-libatomic then has to be added as well when we add -latomic in the driver. ----------------- Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter
gcc/config.in | 6 ++++++ gcc/configure | 38 +++++++++++++++++++++++++++++++++++--- gcc/configure.ac | 26 +++++++++++++++++++++++++- 3 files changed, 66 insertions(+), 4 deletions(-) diff --git a/gcc/configure.ac b/gcc/configure.ac index 26a5d8e3619..55e29773f98 100644 --- a/gcc/configure.ac +++ b/gcc/configure.ac @@ -1102,6 +1102,31 @@ AC_ARG_WITH(multilib-list, :, with_multilib_list=default) +# If libatomic is available, whether it should be linked automatically +AC_ARG_ENABLE(autolink-libatomic, +[AS_HELP_STRING([--enable-autolink-libatomic], + [enable or disable the automatic linking of libatomic + if it is available (enabled by default)])], +[ + case $enable_autolink_libatomic in + yes | no) ;; + *) AC_MSG_ERROR(['$enable_autolink_libatomic' is an invalid value for +--enable-autolink-libatomic. Valid choices are 'yes' and 'no'.]) ;; + esac +], [enable_autolink_libatomic='']) + +if test x$enable_autolink_libatomic != xno; then + if echo " ${TARGET_CONFIGDIRS} " | grep " libatomic " > /dev/null 2>&1 ; then + AC_DEFINE(ENABLE_AUTOLINK_LIBATOMIC, 1, + [Define if libatomic should always be linked.]) + else + if test x$enable_autolink_libatomic = xyes; then + AC_MSG_WARN([libatomic is not build for this target, --enable-autolink-libatomic ignored]) + fi + fi +fi + + # ------------------------- # Checks for other programs # ------------------------- @@ -7106,4 +7131,3 @@ done ], [subdirs='$subdirs']) AC_OUTPUT - diff --git a/gcc/config.in b/gcc/config.in index 3657c46f349..ed4fefe35cd 100644 --- a/gcc/config.in +++ b/gcc/config.in @@ -106,6 +106,12 @@ #endif +/* Define if libatomic should always be linked. */ +#ifndef USED_FOR_TARGET +#undef ENABLE_AUTOLINK_LIBATOMIC +#endif + + /* Define to 1 to specify that we are using the BID decimal floating point format instead of DPD */ #ifndef USED_FOR_TARGET diff --git a/gcc/configure b/gcc/configure index abff47d30eb..4f0b5984544 100755 --- a/gcc/configure +++ b/gcc/configure @@ -972,6 +972,7 @@ with_documentation_root_url with_changes_root_url enable_languages with_multilib_list +enable_autolink_libatomic with_zstd with_zstd_include with_zstd_lib @@ -1698,6 +1699,9 @@ Optional Features: --disable-shared don't provide a shared libgcc --disable-gcov don't provide libgcov and related host tools --enable-languages=LIST specify which front-ends to build + --enable-autolink-libatomic + enable or disable the automatic linking of libatomic + if it is available (enabled by default) --disable-rpath do not hardcode runtime library paths --enable-sjlj-exceptions arrange to use setjmp/longjmp exception handling @@ -8002,6 +8006,35 @@ else fi +# If libatomic is available, whether it should be linked automatically +# Check whether --enable-autolink-libatomic was given. +if test "${enable_autolink_libatomic+set}" = set; then : + enableval=$enable_autolink_libatomic; + case $enable_autolink_libatomic in + yes | no) ;; + *) as_fn_error $? "'$enable_autolink_libatomic' is an invalid value for +--enable-autolink-libatomic. Valid choices are 'yes' and 'no'." "$LINENO" 5 ;; + esac + +else + enable_autolink_libatomic='' +fi + + +if test x$enable_autolink_libatomic != xno; then + if echo " ${TARGET_CONFIGDIRS} " | grep " libatomic " > /dev/null 2>&1 ; then + +$as_echo "#define ENABLE_AUTOLINK_LIBATOMIC 1" >>confdefs.h + + else + if test x$enable_autolink_libatomic = xyes; then + { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: libatomic is not build for this target, --enable-autolink-libatomic ignored" >&5 +$as_echo "$as_me: WARNING: libatomic is not build for this target, --enable-autolink-libatomic ignored" >&2;} + fi + fi +fi + + # ------------------------- # Checks for other programs # ------------------------- @@ -19018,7 +19051,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 19021 "configure" +#line 19054 "configure" #include "confdefs.h" #if HAVE_DLFCN_H @@ -19124,7 +19157,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 19127 "configure" +#line 19160 "configure" #include "confdefs.h" #if HAVE_DLFCN_H @@ -32715,4 +32748,3 @@ if test -n "$ac_unrecognized_opts" && test "$enable_option_checking" != no; then $as_echo "$as_me: WARNING: unrecognized options: $ac_unrecognized_opts" >&2;} fi -