Message ID | 31546a0d-9103-dba2-cca7-261905441aa5@suse.cz |
---|---|
State | New |
Headers | show |
Series | lto-plugin: use locking only for selected targets | expand |
On Thu, Jul 7, 2022 at 1:43 PM Martin Liška <mliska@suse.cz> wrote: > > For now, support locking only for linux targets that are different from > riscv* where the target depends on libatomic (and fails during > bootstrap). > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? OK - that also resolves the mingw issue, correct? I suppose we need to be careful to not advertise v1 API (which includes threadsafeness) when not HAVE_PTHREAD_LOCKING. Thanks, Richard. > Thanks, > Martin > > PR lto/106170 > > lto-plugin/ChangeLog: > > * configure.ac: Configure HAVE_PTHREAD_LOCKING. > * lto-plugin.c (LOCK_SECTION): New. > (UNLOCK_SECTION): New. > (claim_file_handler): Use the newly added macros. > (onload): Likewise. > * config.h.in: Regenerate. > * configure: Regenerate. > --- > lto-plugin/config.h.in | 4 ++-- > lto-plugin/configure | 20 ++++++++++++++------ > lto-plugin/configure.ac | 17 +++++++++++++++-- > lto-plugin/lto-plugin.c | 30 ++++++++++++++++++++---------- > 4 files changed, 51 insertions(+), 20 deletions(-) > > diff --git a/lto-plugin/config.h.in b/lto-plugin/config.h.in > index 029e782f1ee..bf269f000d2 100644 > --- a/lto-plugin/config.h.in > +++ b/lto-plugin/config.h.in > @@ -9,8 +9,8 @@ > /* Define to 1 if you have the <memory.h> header file. */ > #undef HAVE_MEMORY_H > > -/* Define to 1 if pthread.h is present. */ > -#undef HAVE_PTHREAD_H > +/* Define if the system-provided pthread locking mechanism. */ > +#undef HAVE_PTHREAD_LOCKING > > /* Define to 1 if you have the <stdint.h> header file. */ > #undef HAVE_STDINT_H > diff --git a/lto-plugin/configure b/lto-plugin/configure > index aaa91a63623..7ea54e6008f 100755 > --- a/lto-plugin/configure > +++ b/lto-plugin/configure > @@ -6011,14 +6011,22 @@ fi > > > # Check for thread headers. > -ac_fn_c_check_header_mongrel "$LINENO" "pthread.h" "ac_cv_header_pthread_h" "$ac_includes_default" > -if test "x$ac_cv_header_pthread_h" = xyes; then : > +use_locking=no > > -$as_echo "#define HAVE_PTHREAD_H 1" >>confdefs.h > +case $target in > + riscv*) > + # do not use locking as pthread depends on libatomic > + ;; > + *-linux*) > + use_locking=yes > + ;; > +esac > > -fi > +if test x$use_locking = xyes; then > > +$as_echo "#define HAVE_PTHREAD_LOCKING 1" >>confdefs.h > > +fi > > case `pwd` in > *\ * | *\ *) > @@ -12091,7 +12099,7 @@ else > lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 > lt_status=$lt_dlunknown > cat > conftest.$ac_ext <<_LT_EOF > -#line 12094 "configure" > +#line 12102 "configure" > #include "confdefs.h" > > #if HAVE_DLFCN_H > @@ -12197,7 +12205,7 @@ else > lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 > lt_status=$lt_dlunknown > cat > conftest.$ac_ext <<_LT_EOF > -#line 12200 "configure" > +#line 12208 "configure" > #include "confdefs.h" > > #if HAVE_DLFCN_H > diff --git a/lto-plugin/configure.ac b/lto-plugin/configure.ac > index c2ec512880f..69bc5139193 100644 > --- a/lto-plugin/configure.ac > +++ b/lto-plugin/configure.ac > @@ -88,8 +88,21 @@ AM_CONDITIONAL(LTO_PLUGIN_USE_SYMVER_GNU, [test "x$lto_plugin_use_symver" = xgnu > AM_CONDITIONAL(LTO_PLUGIN_USE_SYMVER_SUN, [test "x$lto_plugin_use_symver" = xsun]) > > # Check for thread headers. > -AC_CHECK_HEADER(pthread.h, > - [AC_DEFINE(HAVE_PTHREAD_H, 1, [Define to 1 if pthread.h is present.])]) > +use_locking=no > + > +case $target in > + riscv*) > + # do not use locking as pthread depends on libatomic > + ;; > + *-linux*) > + use_locking=yes > + ;; > +esac > + > +if test x$use_locking = xyes; then > + AC_DEFINE(HAVE_PTHREAD_LOCKING, 1, > + [Define if the system-provided pthread locking mechanism.]) > +fi > > AM_PROG_LIBTOOL > ACX_LT_HOST_FLAGS > diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c > index 635e126946b..cba58f5999b 100644 > --- a/lto-plugin/lto-plugin.c > +++ b/lto-plugin/lto-plugin.c > @@ -40,11 +40,7 @@ along with this program; see the file COPYING3. If not see > > #ifdef HAVE_CONFIG_H > #include "config.h" > -#if !HAVE_PTHREAD_H > -#error POSIX threads are mandatory dependency > #endif > -#endif > - > #if HAVE_STDINT_H > #include <stdint.h> > #endif > @@ -59,7 +55,9 @@ along with this program; see the file COPYING3. If not see > #include <unistd.h> > #include <fcntl.h> > #include <sys/types.h> > +#if HAVE_PTHREAD_LOCKING > #include <pthread.h> > +#endif > #ifdef HAVE_SYS_WAIT_H > #include <sys/wait.h> > #endif > @@ -162,9 +160,18 @@ enum symbol_style > ss_uscore, /* Underscore prefix all symbols. */ > }; > > +#if HAVE_PTHREAD_LOCKING > /* Plug-in mutex. */ > static pthread_mutex_t plugin_lock; > > +#define LOCK_SECTION pthread_mutex_lock (&plugin_lock) > +#define UNLOCK_SECTION pthread_mutex_unlock (&plugin_lock) > +#else > + > +#define LOCK_SECTION > +#define UNLOCK_SECTION > +#endif > + > static char *arguments_file_name; > static ld_plugin_register_claim_file register_claim_file; > static ld_plugin_register_all_symbols_read register_all_symbols_read; > @@ -1270,18 +1277,18 @@ claim_file_handler (const struct ld_plugin_input_file *file, int *claimed) > lto_file.symtab.syms); > check (status == LDPS_OK, LDPL_FATAL, "could not add symbols"); > > - pthread_mutex_lock (&plugin_lock); > + LOCK_SECTION; > num_claimed_files++; > claimed_files = > xrealloc (claimed_files, > num_claimed_files * sizeof (struct plugin_file_info)); > claimed_files[num_claimed_files - 1] = lto_file; > - pthread_mutex_unlock (&plugin_lock); > + UNLOCK_SECTION; > > *claimed = 1; > } > > - pthread_mutex_lock (&plugin_lock); > + LOCK_SECTION; > if (offload_files == NULL) > { > /* Add dummy item to the start of the list. */ > @@ -1344,14 +1351,15 @@ claim_file_handler (const struct ld_plugin_input_file *file, int *claimed) > offload_files_last_lto = ofld; > num_offload_files++; > } > - pthread_mutex_unlock (&plugin_lock); > + > + UNLOCK_SECTION; > > goto cleanup; > > err: > - pthread_mutex_lock (&plugin_lock); > + LOCK_SECTION; > non_claimed_files++; > - pthread_mutex_unlock (&plugin_lock); > + UNLOCK_SECTION; > free (lto_file.name); > > cleanup: > @@ -1429,11 +1437,13 @@ onload (struct ld_plugin_tv *tv) > struct ld_plugin_tv *p; > enum ld_plugin_status status; > > +#if HAVE_PTHREAD_LOCKING > if (pthread_mutex_init (&plugin_lock, NULL) != 0) > { > fprintf (stderr, "mutex init failed\n"); > abort (); > } > +#endif > > p = tv; > while (p->tv_tag) > -- > 2.36.1 >
Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes: >> +if test x$use_locking = xyes; then >> + AC_DEFINE(HAVE_PTHREAD_LOCKING, 1, >> + [Define if the system-provided pthread locking mechanism.]) This isn't even a sentence. At least I cannot parse it. Besides, it seems to be misnamed since the test doesn't check if pthread_mutex_lock and friends are present on the target, but if they should be used. Rainer
On 7/7/22 13:52, Rainer Orth wrote: > Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > >>> +if test x$use_locking = xyes; then >>> + AC_DEFINE(HAVE_PTHREAD_LOCKING, 1, >>> + [Define if the system-provided pthread locking mechanism.]) > > This isn't even a sentence. At least I cannot parse it. Besides, it > seems to be misnamed since the test doesn't check if pthread_mutex_lock > and friends are present on the target, but if they should be used. You are right, fixed in v2. Martin > > Rainer >
On 7/7/22 13:46, Richard Biener wrote: > OK - that also resolves the mingw issue, correct? I suppose we need Yes. > to be careful to not advertise v1 API (which includes threadsafeness) > when not HAVE_PTHREAD_LOCKING. Will reflect that in the patch. I'm going to push it now. Martin
diff --git a/lto-plugin/config.h.in b/lto-plugin/config.h.in index 029e782f1ee..bf269f000d2 100644 --- a/lto-plugin/config.h.in +++ b/lto-plugin/config.h.in @@ -9,8 +9,8 @@ /* Define to 1 if you have the <memory.h> header file. */ #undef HAVE_MEMORY_H -/* Define to 1 if pthread.h is present. */ -#undef HAVE_PTHREAD_H +/* Define if the system-provided pthread locking mechanism. */ +#undef HAVE_PTHREAD_LOCKING /* Define to 1 if you have the <stdint.h> header file. */ #undef HAVE_STDINT_H diff --git a/lto-plugin/configure b/lto-plugin/configure index aaa91a63623..7ea54e6008f 100755 --- a/lto-plugin/configure +++ b/lto-plugin/configure @@ -6011,14 +6011,22 @@ fi # Check for thread headers. -ac_fn_c_check_header_mongrel "$LINENO" "pthread.h" "ac_cv_header_pthread_h" "$ac_includes_default" -if test "x$ac_cv_header_pthread_h" = xyes; then : +use_locking=no -$as_echo "#define HAVE_PTHREAD_H 1" >>confdefs.h +case $target in + riscv*) + # do not use locking as pthread depends on libatomic + ;; + *-linux*) + use_locking=yes + ;; +esac -fi +if test x$use_locking = xyes; then +$as_echo "#define HAVE_PTHREAD_LOCKING 1" >>confdefs.h +fi case `pwd` in *\ * | *\ *) @@ -12091,7 +12099,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 12094 "configure" +#line 12102 "configure" #include "confdefs.h" #if HAVE_DLFCN_H @@ -12197,7 +12205,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 12200 "configure" +#line 12208 "configure" #include "confdefs.h" #if HAVE_DLFCN_H diff --git a/lto-plugin/configure.ac b/lto-plugin/configure.ac index c2ec512880f..69bc5139193 100644 --- a/lto-plugin/configure.ac +++ b/lto-plugin/configure.ac @@ -88,8 +88,21 @@ AM_CONDITIONAL(LTO_PLUGIN_USE_SYMVER_GNU, [test "x$lto_plugin_use_symver" = xgnu AM_CONDITIONAL(LTO_PLUGIN_USE_SYMVER_SUN, [test "x$lto_plugin_use_symver" = xsun]) # Check for thread headers. -AC_CHECK_HEADER(pthread.h, - [AC_DEFINE(HAVE_PTHREAD_H, 1, [Define to 1 if pthread.h is present.])]) +use_locking=no + +case $target in + riscv*) + # do not use locking as pthread depends on libatomic + ;; + *-linux*) + use_locking=yes + ;; +esac + +if test x$use_locking = xyes; then + AC_DEFINE(HAVE_PTHREAD_LOCKING, 1, + [Define if the system-provided pthread locking mechanism.]) +fi AM_PROG_LIBTOOL ACX_LT_HOST_FLAGS diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c index 635e126946b..cba58f5999b 100644 --- a/lto-plugin/lto-plugin.c +++ b/lto-plugin/lto-plugin.c @@ -40,11 +40,7 @@ along with this program; see the file COPYING3. If not see #ifdef HAVE_CONFIG_H #include "config.h" -#if !HAVE_PTHREAD_H -#error POSIX threads are mandatory dependency #endif -#endif - #if HAVE_STDINT_H #include <stdint.h> #endif @@ -59,7 +55,9 @@ along with this program; see the file COPYING3. If not see #include <unistd.h> #include <fcntl.h> #include <sys/types.h> +#if HAVE_PTHREAD_LOCKING #include <pthread.h> +#endif #ifdef HAVE_SYS_WAIT_H #include <sys/wait.h> #endif @@ -162,9 +160,18 @@ enum symbol_style ss_uscore, /* Underscore prefix all symbols. */ }; +#if HAVE_PTHREAD_LOCKING /* Plug-in mutex. */ static pthread_mutex_t plugin_lock; +#define LOCK_SECTION pthread_mutex_lock (&plugin_lock) +#define UNLOCK_SECTION pthread_mutex_unlock (&plugin_lock) +#else + +#define LOCK_SECTION +#define UNLOCK_SECTION +#endif + static char *arguments_file_name; static ld_plugin_register_claim_file register_claim_file; static ld_plugin_register_all_symbols_read register_all_symbols_read; @@ -1270,18 +1277,18 @@ claim_file_handler (const struct ld_plugin_input_file *file, int *claimed) lto_file.symtab.syms); check (status == LDPS_OK, LDPL_FATAL, "could not add symbols"); - pthread_mutex_lock (&plugin_lock); + LOCK_SECTION; num_claimed_files++; claimed_files = xrealloc (claimed_files, num_claimed_files * sizeof (struct plugin_file_info)); claimed_files[num_claimed_files - 1] = lto_file; - pthread_mutex_unlock (&plugin_lock); + UNLOCK_SECTION; *claimed = 1; } - pthread_mutex_lock (&plugin_lock); + LOCK_SECTION; if (offload_files == NULL) { /* Add dummy item to the start of the list. */ @@ -1344,14 +1351,15 @@ claim_file_handler (const struct ld_plugin_input_file *file, int *claimed) offload_files_last_lto = ofld; num_offload_files++; } - pthread_mutex_unlock (&plugin_lock); + + UNLOCK_SECTION; goto cleanup; err: - pthread_mutex_lock (&plugin_lock); + LOCK_SECTION; non_claimed_files++; - pthread_mutex_unlock (&plugin_lock); + UNLOCK_SECTION; free (lto_file.name); cleanup: @@ -1429,11 +1437,13 @@ onload (struct ld_plugin_tv *tv) struct ld_plugin_tv *p; enum ld_plugin_status status; +#if HAVE_PTHREAD_LOCKING if (pthread_mutex_init (&plugin_lock, NULL) != 0) { fprintf (stderr, "mutex init failed\n"); abort (); } +#endif p = tv; while (p->tv_tag)