| Message ID | CAN+Y5ujnB5_+BZ-V8eFV9yZ5rmcDUYRZn6Ty5sc1JozDz13wXw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [libphobos] Fix compilation dependencies on s390x-linux-musl | expand |
On 28/01/2020 05:00, Mathias Lang wrote: > Hi, > > This patch fixes GDC on s390x-linux-musl targets. It was specifically > tested under Alpine Linux (see > https://gitlab.alpinelinux.org/alpine/aports/commit/c123e0f14ab73976a36c651d47d134f249413f29 > ). > The patch fixes two issues: First, Musl always provide > `__tls_get_addr`, so we can always use it to get the TLS range instead > of the internal function (which is glibc-specific). > Second, druntime provide an ASM implementation for > `fiber_switchContext` for most platform under > libphobos/libdruntime/config/$ARCH/switchcontext.S, and default to > `swapcontext` when not available, which is the case on s390x. > However, the configure script did not depend on `swapcontext` being > present, as it's part of glibc, but not Musl (there is a libucontext > available on Alpine for this), which is added here. > > @Iain: Any chance those could be backported to v9 ? > Thanks, I see no reason not to backport. > --- > Mathias Lang > --- > libphobos/ChangeLog: > > * libdruntime/gcc/sections/elf_shared.d Always use > __tls_get_addr on Musl. > * configure.ac: Search librairies for swapcontext when > LIBDRUNTIME_NEEDS_UCONTEXT is yes. > * configure.tgt: Set LIBDRUNTIME_NEEDS_UCONTEXT on s390*-linux*. > * configure: Regenerate. > --- > diff -Nurp a/libphobos/libdruntime/gcc/sections/elf_shared.d > b/libphobos/libdruntime/gcc/sections/elf_shared.d > --- a/libphobos/libdruntime/gcc/sections/elf_shared.d > +++ b/libphobos/libdruntime/gcc/sections/elf_shared.d > @@ -1084,7 +1084,9 @@ void[] getTLSRange(size_t mod, size_t sz) nothrow @nogc > > // base offset > auto ti = tls_index(mod, 0); > - version (IBMZ_Any) > + version (CRuntime_Musl) > + return (__tls_get_addr(&ti)-TLS_DTV_OFFSET)[0 .. sz]; > + else version (IBMZ_Any) > { > auto idx = cast(void *)__tls_get_addr_internal(&ti) > + cast(ulong)__builtin_thread_pointer(); This is fine. > diff -Nurp a/libphobos/configure.ac b/libphobos/configure.ac > --- a/libphobos/configure.ac > +++ b/libphobos/configure.ac > @@ -140,6 +140,14 @@ case ${host} in > esac > AC_MSG_RESULT($LIBPHOBOS_SUPPORTED) > > +AC_MSG_CHECKING([if target needs to link in swapcontext]) > +AC_MSG_RESULT($LIBDRUNTIME_NEEDS_UCONTEXT) > +AS_IF([test "x$LIBDRUNTIME_NEEDS_UCONTEXT" = xyes], [ > + AC_SEARCH_LIBS([swapcontext], [c ucontext], [], [ > + AC_MSG_ERROR([[can't find library providing swapcontext]]) > + ]) > +]) > + Rather than adding LIBDRUNTIME_NEEDS_UCONTEXT, couldn't you just add a one-liner AC_SEARCH_LIBS to the WITH_LOCAL_DRUNTIME list? Testing as I suggest locally, there is no problems on x86 and x86_64. Iain.
On 08/04/2020 10:14, Iain Buclaw via Gcc-patches wrote: > On 28/01/2020 05:00, Mathias Lang wrote: >> diff -Nurp a/libphobos/configure.ac b/libphobos/configure.ac >> --- a/libphobos/configure.ac >> +++ b/libphobos/configure.ac >> @@ -140,6 +140,14 @@ case ${host} in >> esac >> AC_MSG_RESULT($LIBPHOBOS_SUPPORTED) >> >> +AC_MSG_CHECKING([if target needs to link in swapcontext]) >> +AC_MSG_RESULT($LIBDRUNTIME_NEEDS_UCONTEXT) >> +AS_IF([test "x$LIBDRUNTIME_NEEDS_UCONTEXT" = xyes], [ >> + AC_SEARCH_LIBS([swapcontext], [c ucontext], [], [ >> + AC_MSG_ERROR([[can't find library providing swapcontext]]) >> + ]) >> +]) >> + > > Rather than adding LIBDRUNTIME_NEEDS_UCONTEXT, couldn't you just add a one-liner AC_SEARCH_LIBS to the WITH_LOCAL_DRUNTIME list? > > Testing as I suggest locally, there is no problems on x86 and x86_64. > > Iain. > Hi Matthias, Does the following change work well for you on s390x-musl? Regards Iain. --- diff --git a/libphobos/configure b/libphobos/configure index a6f5aec2bfd..333ac86c977 100755 --- a/libphobos/configure +++ b/libphobos/configure @@ -15043,6 +15043,77 @@ $as_echo "$druntime_cv_lib_sockets" >&6; } LIBS="$LIBS $druntime_cv_lib_sockets" + # Keep this in sync with core/thread.d, set druntime_fiber_asm_external to + # "yes" for targets that have 'version = AsmExternal'. + druntime_fiber_asm_external=no + case "$target_cpu" in + aarch64* | \ + arm* | \ + i[34567]86|x86_64 | \ + powerpc) + druntime_fiber_asm_external=yes + ;; + esac + if test "$druntime_fiber_asm_external" = no; then + { $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing swapcontext" >&5 +$as_echo_n "checking for library containing swapcontext... " >&6; } +if ${ac_cv_search_swapcontext+:} false; then : + $as_echo_n "(cached) " >&6 +else + ac_func_search_save_LIBS=$LIBS +cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + +/* Override any GCC internal prototype to avoid an error. + Use char because int might match the return type of a GCC + builtin and then its argument prototype would still apply. */ +#ifdef __cplusplus +extern "C" +#endif +char swapcontext (); +int +main () +{ +return swapcontext (); + ; + return 0; +} +_ACEOF +for ac_lib in '' c ucontext; do + if test -z "$ac_lib"; then + ac_res="none required" + else + ac_res=-l$ac_lib + LIBS="-l$ac_lib $ac_func_search_save_LIBS" + fi + if ac_fn_c_try_link "$LINENO"; then : + ac_cv_search_swapcontext=$ac_res +fi +rm -f core conftest.err conftest.$ac_objext \ + conftest$ac_exeext + if ${ac_cv_search_swapcontext+:} false; then : + break +fi +done +if ${ac_cv_search_swapcontext+:} false; then : + +else + ac_cv_search_swapcontext=no +fi +rm conftest.$ac_ext +LIBS=$ac_func_search_save_LIBS +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_search_swapcontext" >&5 +$as_echo "$ac_cv_search_swapcontext" >&6; } +ac_res=$ac_cv_search_swapcontext +if test "$ac_res" != no; then : + test "$ac_res" = "none required" || LIBS="$ac_res $LIBS" + +fi + + fi + + ac_ext=c ac_cpp='$CPP $CPPFLAGS' ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5' diff --git a/libphobos/configure.ac b/libphobos/configure.ac index ffd12981d0b..47591668d9d 100644 --- a/libphobos/configure.ac +++ b/libphobos/configure.ac @@ -144,6 +144,7 @@ DRUNTIME_LIBRARIES_ATOMIC DRUNTIME_LIBRARIES_BACKTRACE DRUNTIME_LIBRARIES_DLOPEN DRUNTIME_LIBRARIES_NET +DRUNTIME_LIBRARIES_UCONTEXT DRUNTIME_LIBRARIES_ZLIB DRUNTIME_INSTALL_DIRECTORIES diff --git a/libphobos/m4/druntime/libraries.m4 b/libphobos/m4/druntime/libraries.m4 index 9e8e210df5a..79a45d9fc3e 100644 --- a/libphobos/m4/druntime/libraries.m4 +++ b/libphobos/m4/druntime/libraries.m4 @@ -212,3 +212,26 @@ AC_DEFUN([DRUNTIME_LIBRARIES_CLIB], AC_SUBST(DCFG_HAVE_QSORT_R) AC_LANG_POP([C]) ]) + +# DRUNTIME_LIBRARIES_UCONTEXT +# ------------------------------ +# Autodetect and add ucontext library to LIBS if necessary. +# This is only required if fiber_switchContext does not have +# its own internal asm implementation. +AC_DEFUN([DRUNTIME_LIBRARIES_UCONTEXT], +[ + # Keep this in sync with core/thread.d, set druntime_fiber_asm_external to + # "yes" for targets that have 'version = AsmExternal'. + druntime_fiber_asm_external=no + case "$target_cpu" in + aarch64* | \ + arm* | \ + i[[34567]]86|x86_64 | \ + powerpc) + druntime_fiber_asm_external=yes + ;; + esac + if test "$druntime_fiber_asm_external" = no; then + AC_SEARCH_LIBS([swapcontext], [c ucontext]) + fi +])
On 13/04/2020 15:36, Iain Buclaw via Gcc-patches wrote: > On 08/04/2020 10:14, Iain Buclaw via Gcc-patches wrote: >> On 28/01/2020 05:00, Mathias Lang wrote: >>> diff -Nurp a/libphobos/configure.ac b/libphobos/configure.ac >>> --- a/libphobos/configure.ac >>> +++ b/libphobos/configure.ac >>> @@ -140,6 +140,14 @@ case ${host} in >>> esac >>> AC_MSG_RESULT($LIBPHOBOS_SUPPORTED) >>> >>> +AC_MSG_CHECKING([if target needs to link in swapcontext]) >>> +AC_MSG_RESULT($LIBDRUNTIME_NEEDS_UCONTEXT) >>> +AS_IF([test "x$LIBDRUNTIME_NEEDS_UCONTEXT" = xyes], [ >>> + AC_SEARCH_LIBS([swapcontext], [c ucontext], [], [ >>> + AC_MSG_ERROR([[can't find library providing swapcontext]]) >>> + ]) >>> +]) >>> + >> >> Rather than adding LIBDRUNTIME_NEEDS_UCONTEXT, couldn't you just add a one-liner AC_SEARCH_LIBS to the WITH_LOCAL_DRUNTIME list? >> >> Testing as I suggest locally, there is no problems on x86 and x86_64. >> >> Iain. >> > > Hi Matthias, > > Does the following change work well for you on s390x-musl? > > Spoke off channel, and we're fine with the proposed patch. Tested on x86_64-linux-gnu and s390x-linux-musl, committed to mainline.
diff -Nurp a/libphobos/libdruntime/gcc/sections/elf_shared.d b/libphobos/libdruntime/gcc/sections/elf_shared.d --- a/libphobos/libdruntime/gcc/sections/elf_shared.d +++ b/libphobos/libdruntime/gcc/sections/elf_shared.d @@ -1084,7 +1084,9 @@ void[] getTLSRange(size_t mod, size_t sz) nothrow @nogc // base offset auto ti = tls_index(mod, 0); - version (IBMZ_Any) + version (CRuntime_Musl) + return (__tls_get_addr(&ti)-TLS_DTV_OFFSET)[0 .. sz]; + else version (IBMZ_Any) { auto idx = cast(void *)__tls_get_addr_internal(&ti) + cast(ulong)__builtin_thread_pointer(); diff -Nurp a/libphobos/configure.ac b/libphobos/configure.ac --- a/libphobos/configure.ac +++ b/libphobos/configure.ac @@ -140,6 +140,14 @@ case ${host} in esac AC_MSG_RESULT($LIBPHOBOS_SUPPORTED) +AC_MSG_CHECKING([if target needs to link in swapcontext]) +AC_MSG_RESULT($LIBDRUNTIME_NEEDS_UCONTEXT) +AS_IF([test "x$LIBDRUNTIME_NEEDS_UCONTEXT" = xyes], [ + AC_SEARCH_LIBS([swapcontext], [c ucontext], [], [ + AC_MSG_ERROR([[can't find library providing swapcontext]]) + ]) +]) + # Decide if it's usable. case $LIBPHOBOS_SUPPORTED:$enable_libphobos in *:no) use_libphobos=no ;; diff -Nurp a/libphobos/configure.tgt b/libphobos/configure.tgt --- a/libphobos/configure.tgt +++ b/libphobos/configure.tgt @@ -22,6 +22,13 @@ # Disable the libphobos or libdruntime components on untested or known # broken systems. More targets shall be added after testing. LIBPHOBOS_SUPPORTED=no + +# Check if we require 'ucontext' or if we have a custom solution. +# Most platform uses a custom assembly solution for context switches, +# see `core.thread` and grep for `AsmExternal`. +# Definitions are in config/ARCH/ +LIBDRUNTIME_NEEDS_UCONTEXT=no + case "${target}" in aarch64*-*-linux*) LIBPHOBOS_SUPPORTED=yes @@ -37,6 +44,7 @@ case "${target}" in ;; s390*-linux*) LIBPHOBOS_SUPPORTED=yes + LIBDRUNTIME_NEEDS_UCONTEXT=yes ;; x86_64-*-kfreebsd*-gnu | i?86-*-kfreebsd*-gnu) LIBPHOBOS_SUPPORTED=yes
Hi, This patch fixes GDC on s390x-linux-musl targets. It was specifically tested under Alpine Linux (see https://gitlab.alpinelinux.org/alpine/aports/commit/c123e0f14ab73976a36c651d47d134f249413f29 ). The patch fixes two issues: First, Musl always provide `__tls_get_addr`, so we can always use it to get the TLS range instead of the internal function (which is glibc-specific). Second, druntime provide an ASM implementation for `fiber_switchContext` for most platform under libphobos/libdruntime/config/$ARCH/switchcontext.S, and default to `swapcontext` when not available, which is the case on s390x. However, the configure script did not depend on `swapcontext` being present, as it's part of glibc, but not Musl (there is a libucontext available on Alpine for this), which is added here. @Iain: Any chance those could be backported to v9 ? --- Mathias Lang --- libphobos/ChangeLog: * libdruntime/gcc/sections/elf_shared.d Always use __tls_get_addr on Musl. * configure.ac: Search librairies for swapcontext when LIBDRUNTIME_NEEDS_UCONTEXT is yes. * configure.tgt: Set LIBDRUNTIME_NEEDS_UCONTEXT on s390*-linux*. * configure: Regenerate. ---