diff mbox series

lto-plugin: use -pthread only for detected targets

Message ID dddeb843-4f94-4432-768f-19dccb2af2da@suse.cz
State New
Headers
Series lto-plugin: use -pthread only for detected targets |

Commit Message

Martin Liška July 13, 2022, 8:23 a.m. UTC 
  Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

Use -pthread only if we are going to use pthread functionality.

	PR bootstrap/106156

lto-plugin/ChangeLog:

	* configure: Regenerate.
	* configure.ac: Use -pthread only why use_locking == true.
	* Makefile.in: Rely on ac_lto_plugin_ldflags.
---
 lto-plugin/Makefile.in  | 2 +-
 lto-plugin/configure    | 6 ++++--
 lto-plugin/configure.ac | 2 ++
 3 files changed, 7 insertions(+), 3 deletions(-)

Comments

Richard Biener July 13, 2022, 12:15 p.m. UTC | #1 
On Wed, Jul 13, 2022 at 10:24 AM Martin Liška <mliska@suse.cz> wrote:
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?

Didn't we have it that way and not work?  IIRC LDFLAGS is only
used during configure link tests and _not_ substituted?

> Thanks,
> Martin
>
> Use -pthread only if we are going to use pthread functionality.
>
>         PR bootstrap/106156
>
> lto-plugin/ChangeLog:
>
>         * configure: Regenerate.
>         * configure.ac: Use -pthread only why use_locking == true.
>         * Makefile.in: Rely on ac_lto_plugin_ldflags.
> ---
>  lto-plugin/Makefile.in  | 2 +-
>  lto-plugin/configure    | 6 ++++--
>  lto-plugin/configure.ac | 2 ++
>  3 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/lto-plugin/Makefile.in b/lto-plugin/Makefile.in
> index 9453bc7d607..6b161c01683 100644
> --- a/lto-plugin/Makefile.in
> +++ b/lto-plugin/Makefile.in
> @@ -345,7 +345,7 @@ libexecsubdir := $(libexecdir)/gcc/$(real_target_noncanonical)/$(gcc_version)$(a
>  AM_CPPFLAGS = -I$(top_srcdir)/../include $(DEFS)
>  AM_CFLAGS = @ac_lto_plugin_warn_cflags@ $(CET_HOST_FLAGS) -DBASE_VERSION='"$(gcc_version)"'
>  # The plug-in depends on pthreads.
> -AM_LDFLAGS = -pthread @ac_lto_plugin_ldflags@
> +AM_LDFLAGS = @ac_lto_plugin_ldflags@
>  AM_LIBTOOLFLAGS = --tag=disable-static
>  libexecsub_LTLIBRARIES = liblto_plugin.la
>  in_gcc_libs = $(foreach lib, $(libexecsub_LTLIBRARIES), $(gcc_build_dir)/$(lib))
> diff --git a/lto-plugin/configure b/lto-plugin/configure
> index 870e49b2e62..0967ba1c798 100755
> --- a/lto-plugin/configure
> +++ b/lto-plugin/configure
> @@ -6023,6 +6023,8 @@ case $target in
>  esac
>
>  if test x$use_locking = xyes; then
> +  LDFLAGS="$LDFLAGS -pthread"
> +
>    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 :
>
> @@ -12104,7 +12106,7 @@ else
>    lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
>    lt_status=$lt_dlunknown
>    cat > conftest.$ac_ext <<_LT_EOF
> -#line 12107 "configure"
> +#line 12109 "configure"
>  #include "confdefs.h"
>
>  #if HAVE_DLFCN_H
> @@ -12210,7 +12212,7 @@ else
>    lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
>    lt_status=$lt_dlunknown
>    cat > conftest.$ac_ext <<_LT_EOF
> -#line 12213 "configure"
> +#line 12215 "configure"
>  #include "confdefs.h"
>
>  #if HAVE_DLFCN_H
> diff --git a/lto-plugin/configure.ac b/lto-plugin/configure.ac
> index 18eb4f60b0a..a350aa56a8b 100644
> --- a/lto-plugin/configure.ac
> +++ b/lto-plugin/configure.ac
> @@ -100,6 +100,8 @@ case $target in
>  esac
>
>  if test x$use_locking = xyes; then
> +  LDFLAGS="$LDFLAGS -pthread"
> +
>    AC_CHECK_HEADER(pthread.h,
>      [AC_DEFINE(HAVE_PTHREAD_LOCKING, 1, [Define if the system provides pthread locking mechanism.])])
>  fi
> --
> 2.37.0
>
Martin Liška July 14, 2022, 10:03 a.m. UTC | #2 
On 7/13/22 14:15, Richard Biener wrote:
> Didn't we have it that way and not work?  IIRC LDFLAGS is only
> used during configure link tests and _not_ substituted?

You are right.

There's a proper fix that utilizes AM_CONDITIONAL and sets LDFLAGS properly
in Makefile.am.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin
Richard Biener July 14, 2022, 10:10 a.m. UTC | #3 
On Thu, Jul 14, 2022 at 12:03 PM Martin Liška <mliska@suse.cz> wrote:
>
> On 7/13/22 14:15, Richard Biener wrote:
> > Didn't we have it that way and not work?  IIRC LDFLAGS is only
> > used during configure link tests and _not_ substituted?
>
> You are right.
>
> There's a proper fix that utilizes AM_CONDITIONAL and sets LDFLAGS properly
> in Makefile.am.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?

I'm not really an expert on build issues but I'd have added
some lto_plugin_extra_ldflags variable, AC_SUBST ()ed it
and done

AM_LDFLAGS = @lto_plugin_extra_ldflags@ @ac_lto_plugin_ldflags@

or figure where we set ac_lto_plugin_ldflags and amend that during
configure instead:

# Need -Wc to get it through libtool.
if test "x$have_static_libgcc" = xyes; then
   ac_lto_plugin_ldflags="-Wc,-static-libgcc"
fi
AC_SUBST(ac_lto_plugin_ldflags)


> Thanks,
> Martin
Martin Liška July 14, 2022, 11:13 a.m. UTC | #4 
On 7/14/22 12:10, Richard Biener wrote:
> On Thu, Jul 14, 2022 at 12:03 PM Martin Liška <mliska@suse.cz> wrote:
>>
>> On 7/13/22 14:15, Richard Biener wrote:
>>> Didn't we have it that way and not work?  IIRC LDFLAGS is only
>>> used during configure link tests and _not_ substituted?
>>
>> You are right.
>>
>> There's a proper fix that utilizes AM_CONDITIONAL and sets LDFLAGS properly
>> in Makefile.am.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>
>> Ready to be installed?
> 
> I'm not really an expert on build issues but I'd have added
> some lto_plugin_extra_ldflags variable, AC_SUBST ()ed it
> and done
> 
> AM_LDFLAGS = @lto_plugin_extra_ldflags@ @ac_lto_plugin_ldflags@
> 
> or figure where we set ac_lto_plugin_ldflags and amend that during
> configure instead:
> 
> # Need -Wc to get it through libtool.
> if test "x$have_static_libgcc" = xyes; then
>    ac_lto_plugin_ldflags="-Wc,-static-libgcc"
> fi
> AC_SUBST(ac_lto_plugin_ldflags)

All right, so something like this?

Martin

> 
> 
>> Thanks,
>> Martin
Richard Biener July 14, 2022, 11:22 a.m. UTC | #5 
On Thu, Jul 14, 2022 at 1:13 PM Martin Liška <mliska@suse.cz> wrote:
>
> On 7/14/22 12:10, Richard Biener wrote:
> > On Thu, Jul 14, 2022 at 12:03 PM Martin Liška <mliska@suse.cz> wrote:
> >>
> >> On 7/13/22 14:15, Richard Biener wrote:
> >>> Didn't we have it that way and not work?  IIRC LDFLAGS is only
> >>> used during configure link tests and _not_ substituted?
> >>
> >> You are right.
> >>
> >> There's a proper fix that utilizes AM_CONDITIONAL and sets LDFLAGS properly
> >> in Makefile.am.
> >>
> >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> >>
> >> Ready to be installed?
> >
> > I'm not really an expert on build issues but I'd have added
> > some lto_plugin_extra_ldflags variable, AC_SUBST ()ed it
> > and done
> >
> > AM_LDFLAGS = @lto_plugin_extra_ldflags@ @ac_lto_plugin_ldflags@
> >
> > or figure where we set ac_lto_plugin_ldflags and amend that during
> > configure instead:
> >
> > # Need -Wc to get it through libtool.
> > if test "x$have_static_libgcc" = xyes; then
> >    ac_lto_plugin_ldflags="-Wc,-static-libgcc"
> > fi
> > AC_SUBST(ac_lto_plugin_ldflags)
>
> All right, so something like this?

LGTM with the changelog fixed

> Martin
>
> >
> >
> >> Thanks,
> >> Martin
diff mbox series

Patch

diff --git a/lto-plugin/Makefile.in b/lto-plugin/Makefile.in
index 9453bc7d607..6b161c01683 100644
--- a/lto-plugin/Makefile.in
+++ b/lto-plugin/Makefile.in
@@ -345,7 +345,7 @@  libexecsubdir := $(libexecdir)/gcc/$(real_target_noncanonical)/$(gcc_version)$(a
 AM_CPPFLAGS = -I$(top_srcdir)/../include $(DEFS)
 AM_CFLAGS = @ac_lto_plugin_warn_cflags@ $(CET_HOST_FLAGS) -DBASE_VERSION='"$(gcc_version)"'
 # The plug-in depends on pthreads.
-AM_LDFLAGS = -pthread @ac_lto_plugin_ldflags@
+AM_LDFLAGS = @ac_lto_plugin_ldflags@
 AM_LIBTOOLFLAGS = --tag=disable-static
 libexecsub_LTLIBRARIES = liblto_plugin.la
 in_gcc_libs = $(foreach lib, $(libexecsub_LTLIBRARIES), $(gcc_build_dir)/$(lib))
diff --git a/lto-plugin/configure b/lto-plugin/configure
index 870e49b2e62..0967ba1c798 100755
--- a/lto-plugin/configure
+++ b/lto-plugin/configure
@@ -6023,6 +6023,8 @@  case $target in
 esac
 
 if test x$use_locking = xyes; then
+  LDFLAGS="$LDFLAGS -pthread"
+
   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 :
 
@@ -12104,7 +12106,7 @@  else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 12107 "configure"
+#line 12109 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -12210,7 +12212,7 @@  else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 12213 "configure"
+#line 12215 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
diff --git a/lto-plugin/configure.ac b/lto-plugin/configure.ac
index 18eb4f60b0a..a350aa56a8b 100644
--- a/lto-plugin/configure.ac
+++ b/lto-plugin/configure.ac
@@ -100,6 +100,8 @@  case $target in
 esac
 
 if test x$use_locking = xyes; then
+  LDFLAGS="$LDFLAGS -pthread"
+
   AC_CHECK_HEADER(pthread.h,
     [AC_DEFINE(HAVE_PTHREAD_LOCKING, 1, [Define if the system provides pthread locking mechanism.])])
 fi