public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [patch] make default linker --hash-style configurable option
@ 2011-04-04 18:17 Paul Pluzhnikov
  2011-04-04 18:22 ` Rainer Orth
                   ` (2 more replies)
  0 siblings, 3 replies; 26+ messages in thread
From: Paul Pluzhnikov @ 2011-04-04 18:17 UTC (permalink / raw)
  To: gcc-patches; +Cc: satorux, iant, ppluzhnikov

Greetings,

Several Linux distributions (e.g. Fedora) carry local patches that turn
on --hash-style=gnu for all links.

Attached is a proposed patch (originally by Satoru Takabayashi) that makes
default hash style a configure option.

Tested by doing native bootstrap and verifying that no --hash-style is
passed to the linker, and also configuring with --with-linker-hash-style=gnu
and verifying that --hash-style=gnu is then passed to the linker.

Ok for trunk?

Thanks,

P.S. Google has a blanket copyright assignment to FSF.

--
Paul Pluzhnikov


2011-04-04  Satoru Takabayashi  <satorux@google.com>
	    Paul Pluzhnikov  <ppluzhnikov@google.com>

	* gcc/doc/install.texi (Configuration): Document
	--with-linker-hash-style.
	* gcc/gcc.c (init_spec): Handle LINKER_HASH_STYLE.
	* gcc/config.in: Add LINKER_HASH_STYLE.
	* gcc/configure.ac: Add --with-linker-hash-style.
	* gcc/configure: Regenerate.


Index: gcc/doc/install.texi
===================================================================
--- gcc/doc/install.texi	(revision 171942)
+++ gcc/doc/install.texi	(working copy)
@@ -1657,6 +1657,11 @@
 support @option{--build-id} option, a warning is issued and the
 @option{--enable-linker-build-id} option is ignored.  The default is off.
 
+@item --with-linker-hash-style=@var{choice}
+Tells GCC to pass @option{--hash-style=@var{choice}} option to the
+linker for all final links. @var{choice} can be one of
+@samp{sysv}, @samp{gnu}, and @samp{both} where @samp{sysv} is the default.
+
 @item --enable-gnu-unique-object
 @itemx --disable-gnu-unique-object
 Tells GCC to use the gnu_unique_object relocation for C++ template
Index: gcc/gcc.c
===================================================================
--- gcc/gcc.c	(revision 171942)
+++ gcc/gcc.c	(working copy)
@@ -1438,7 +1438,8 @@
   }
 #endif
 
-#if defined LINK_EH_SPEC || defined LINK_BUILDID_SPEC
+#if defined LINK_EH_SPEC || defined LINK_BUILDID_SPEC || \
+    defined LINKER_HASH_STYLE
 # ifdef LINK_BUILDID_SPEC
   /* Prepend LINK_BUILDID_SPEC to whatever link_spec we had before.  */
   obstack_grow (&obstack, LINK_BUILDID_SPEC, sizeof(LINK_BUILDID_SPEC) - 1);
@@ -1447,6 +1448,16 @@
   /* Prepend LINK_EH_SPEC to whatever link_spec we had before.  */
   obstack_grow (&obstack, LINK_EH_SPEC, sizeof(LINK_EH_SPEC) - 1);
 # endif
+# ifdef LINKER_HASH_STYLE
+  /* Prepend --hash-style=LINKER_HASH_STYLE to whatever link_spec we had
+     before.  */
+  {
+    static const char hash_style[] = "--hash-style=";
+    obstack_grow (&obstack, hash_style, sizeof(hash_style) - 1);
+    obstack_grow (&obstack, LINKER_HASH_STYLE, sizeof(LINKER_HASH_STYLE) - 1);
+    obstack_1grow (&obstack, ' ');
+  }
+# endif
   obstack_grow0 (&obstack, link_spec, strlen (link_spec));
   link_spec = XOBFINISH (&obstack, const char *);
 #endif
Index: gcc/config.in
===================================================================
--- gcc/config.in	(revision 171942)
+++ gcc/config.in	(working copy)
@@ -1580,6 +1580,12 @@
 #endif
 
 
+/* The linker hash style */
+#ifndef USED_FOR_TARGET
+#undef LINKER_HASH_STYLE
+#endif
+
+
 /* Define to the name of the LTO plugin DSO that must be passed to the
    linker's -plugin=LIB option. */
 #ifndef USED_FOR_TARGET
Index: gcc/configure.ac
===================================================================
--- gcc/configure.ac	(revision 171942)
+++ gcc/configure.ac	(working copy)
@@ -4936,6 +4936,30 @@
 fi
 
 
+# Specify what hash style to use by default.
+AC_ARG_WITH([linker-hash-style],
+[AC_HELP_STRING([--with-linker-hash-style={sysv,gnu,both}],
+                [specify the linker hash style])],
+[case x"$withval" in
+   xsysv)
+     LINKER_HASH_STYLE=sysv
+     ;;
+   xgnu)
+     LINKER_HASH_STYLE=gnu
+     ;;
+   xboth)
+     LINKER_HASH_STYLE=both
+     ;;
+   *)
+     AC_MSG_ERROR([$withval is an invalid option to --with-linker-hash-style])
+     ;;
+ esac],
+[LINKER_HASH_STYLE=''])
+if test x"${LINKER_HASH_STYLE}" != x; then
+  AC_DEFINE_UNQUOTED(LINKER_HASH_STYLE, "$LINKER_HASH_STYLE",
+                                         [The linker hash style])
+fi
+
 # Configure the subdirectories
 # AC_CONFIG_SUBDIRS($subdirs)
 
Index: gcc/configure
===================================================================
--- gcc/configure	(revision 171942)
+++ gcc/configure	(working copy)
@@ -913,6 +913,7 @@
 with_slibdir
 enable_plugin
 enable_libquadmath_support
+with_linker_hash_style
 '
       ac_precious_vars='build_alias
 host_alias
@@ -1663,6 +1664,8 @@
                           with the compiler
   --with-system-zlib      use installed libz
   --with-slibdir=DIR      shared libraries in DIR [LIBDIR]
+  --with-linker-hash-style={sysv,gnu,both}
+                          specify the linker hash style
 
 Some influential environment variables:
   CC          C compiler command
@@ -17505,7 +17508,7 @@
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 17508 "configure"
+#line 17511 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -17611,7 +17614,7 @@
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 17614 "configure"
+#line 17617 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -26461,6 +26464,36 @@
 fi
 
 
+# Specify what hash style to use by default.
+
+# Check whether --with-linker-hash-style was given.
+if test "${with_linker_hash_style+set}" = set; then :
+  withval=$with_linker_hash_style; case x"$withval" in
+     xsysv)
+       LINKER_HASH_STYLE=sysv
+       ;;
+     xgnu)
+       LINKER_HASH_STYLE=gnu
+       ;;
+     xboth)
+       LINKER_HASH_STYLE=both
+       ;;
+     *)
+       as_fn_error "$withval is an invalid option to --with-linker-hash-style" "$LINENO" 5
+       ;;
+   esac
+else
+  LINKER_HASH_STYLE=''
+fi
+
+if test x"${LINKER_HASH_STYLE}" != x; then
+
+cat >>confdefs.h <<_ACEOF
+#define LINKER_HASH_STYLE "$LINKER_HASH_STYLE"
+_ACEOF
+
+fi
+
 # Configure the subdirectories
 # AC_CONFIG_SUBDIRS($subdirs)
 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch] make default linker --hash-style configurable option
  2011-04-04 18:17 [patch] make default linker --hash-style configurable option Paul Pluzhnikov
@ 2011-04-04 18:22 ` Rainer Orth
  2011-04-04 18:26   ` Ian Lance Taylor
  2011-04-04 22:53 ` Matthias Klose
  2011-05-12  9:10 ` Eric Botcazou
  2 siblings, 1 reply; 26+ messages in thread
From: Rainer Orth @ 2011-04-04 18:22 UTC (permalink / raw)
  To: Paul Pluzhnikov; +Cc: gcc-patches, satorux, iant

ppluzhnikov@google.com (Paul Pluzhnikov) writes:

> Several Linux distributions (e.g. Fedora) carry local patches that turn
> on --hash-style=gnu for all links.

Shouldn't configure verify that the linker used actually understands
that option?

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch] make default linker --hash-style configurable option
  2011-04-04 18:22 ` Rainer Orth
@ 2011-04-04 18:26   ` Ian Lance Taylor
  0 siblings, 0 replies; 26+ messages in thread
From: Ian Lance Taylor @ 2011-04-04 18:26 UTC (permalink / raw)
  To: Rainer Orth; +Cc: Paul Pluzhnikov, gcc-patches, satorux

Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:

> ppluzhnikov@google.com (Paul Pluzhnikov) writes:
>
>> Several Linux distributions (e.g. Fedora) carry local patches that turn
>> on --hash-style=gnu for all links.
>
> Shouldn't configure verify that the linker used actually understands
> that option?

No, I don't think so.  This is an explicit configure option.  People who
use explicit configure options should get what they request, whether or
not it actually works.

Ian

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch] make default linker --hash-style configurable option
  2011-04-04 18:17 [patch] make default linker --hash-style configurable option Paul Pluzhnikov
  2011-04-04 18:22 ` Rainer Orth
@ 2011-04-04 22:53 ` Matthias Klose
  2011-04-05  0:01   ` Joseph S. Myers
  2011-05-12  9:10 ` Eric Botcazou
  2 siblings, 1 reply; 26+ messages in thread
From: Matthias Klose @ 2011-04-04 22:53 UTC (permalink / raw)
  To: Paul Pluzhnikov; +Cc: gcc-patches, satorux, iant

On 04.04.2011 20:17, Paul Pluzhnikov wrote:
> Greetings,
> 
> Several Linux distributions (e.g. Fedora) carry local patches that turn
> on --hash-style=gnu for all links.
> 
> Attached is a proposed patch (originally by Satoru Takabayashi) that makes
> default hash style a configure option.
> 
> Tested by doing native bootstrap and verifying that no --hash-style is
> passed to the linker, and also configuring with --with-linker-hash-style=gnu
> and verifying that --hash-style=gnu is then passed to the linker.

Linux distributions pass more than that by default to the linker, e.g.
--as-needed and --no-copy-dt-needed-entries.  Wouldn't it make more sense to add
something like --with-linker-default-options=... ?

  Matthias

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch] make default linker --hash-style configurable option
  2011-04-04 22:53 ` Matthias Klose
@ 2011-04-05  0:01   ` Joseph S. Myers
  2011-04-11 18:01     ` Paul Pluzhnikov
  0 siblings, 1 reply; 26+ messages in thread
From: Joseph S. Myers @ 2011-04-05  0:01 UTC (permalink / raw)
  To: Matthias Klose; +Cc: Paul Pluzhnikov, gcc-patches, satorux, iant

On Tue, 5 Apr 2011, Matthias Klose wrote:

> On 04.04.2011 20:17, Paul Pluzhnikov wrote:
> > Greetings,
> > 
> > Several Linux distributions (e.g. Fedora) carry local patches that turn
> > on --hash-style=gnu for all links.
> > 
> > Attached is a proposed patch (originally by Satoru Takabayashi) that makes
> > default hash style a configure option.
> > 
> > Tested by doing native bootstrap and verifying that no --hash-style is
> > passed to the linker, and also configuring with --with-linker-hash-style=gnu
> > and verifying that --hash-style=gnu is then passed to the linker.
> 
> Linux distributions pass more than that by default to the linker, e.g.
> --as-needed and --no-copy-dt-needed-entries.  Wouldn't it make more sense to add
> something like --with-linker-default-options=... ?

We have --with-specs that configures specs for the driver's own command 
line - maybe there's a use for something like that for other specs.  But 
that's very much a last-resort option where there isn't a more structured 
way of configuring something; I'd prefer common use cases to be easier 
than that.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch] make default linker --hash-style configurable option
  2011-04-05  0:01   ` Joseph S. Myers
@ 2011-04-11 18:01     ` Paul Pluzhnikov
  2011-04-18 17:20       ` Paul Pluzhnikov
  0 siblings, 1 reply; 26+ messages in thread
From: Paul Pluzhnikov @ 2011-04-11 18:01 UTC (permalink / raw)
  To: Joseph S. Myers; +Cc: Matthias Klose, gcc-patches, satorux, iant

Ping?

On Mon, Apr 4, 2011 at 5:01 PM, Joseph S. Myers <joseph@codesourcery.com> wrote:
> On Tue, 5 Apr 2011, Matthias Klose wrote:
...
>> Linux distributions pass more than that by default to the linker, e.g.
>> --as-needed and --no-copy-dt-needed-entries.  Wouldn't it make more sense to add
>> something like --with-linker-default-options=... ?
>
> We have --with-specs that configures specs for the driver's own command
> line - maybe there's a use for something like that for other specs.  But
> that's very much a last-resort option where there isn't a more structured
> way of configuring something; I'd prefer common use cases to be easier
> than that.

The first version of this patch Satoru proposed was a general "do anything
to specs" patch. Ian voted that down as being too generic and
difficult to use correctly.

Thanks,
-- 
Paul Pluzhnikov

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch] make default linker --hash-style configurable option
  2011-04-11 18:01     ` Paul Pluzhnikov
@ 2011-04-18 17:20       ` Paul Pluzhnikov
  2011-04-25 17:03         ` Paul Pluzhnikov
  0 siblings, 1 reply; 26+ messages in thread
From: Paul Pluzhnikov @ 2011-04-18 17:20 UTC (permalink / raw)
  To: Joseph S. Myers; +Cc: Matthias Klose, gcc-patches, satorux, iant

Ping? Ping?

On Mon, Apr 11, 2011 at 11:00 AM, Paul Pluzhnikov
<ppluzhnikov@google.com> wrote:
> Ping?

-- 
Paul Pluzhnikov

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch] make default linker --hash-style configurable option
  2011-04-18 17:20       ` Paul Pluzhnikov
@ 2011-04-25 17:03         ` Paul Pluzhnikov
  2011-05-02 14:23           ` Paul Pluzhnikov
  0 siblings, 1 reply; 26+ messages in thread
From: Paul Pluzhnikov @ 2011-04-25 17:03 UTC (permalink / raw)
  To: Joseph S. Myers; +Cc: Matthias Klose, gcc-patches, satorux, iant

Ping? Ping? Ping?

On Mon, Apr 18, 2011 at 9:45 AM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> Ping? Ping?
>
> On Mon, Apr 11, 2011 at 11:00 AM, Paul Pluzhnikov
> <ppluzhnikov@google.com> wrote:
>> Ping?

-- 
Paul Pluzhnikov

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch] make default linker --hash-style configurable option
  2011-04-25 17:03         ` Paul Pluzhnikov
@ 2011-05-02 14:23           ` Paul Pluzhnikov
  2011-05-02 15:00             ` Joseph S. Myers
  0 siblings, 1 reply; 26+ messages in thread
From: Paul Pluzhnikov @ 2011-05-02 14:23 UTC (permalink / raw)
  To: Joseph S. Myers; +Cc: Matthias Klose, gcc-patches, satorux, iant

Ping? Ping? Ping? Ping?

This is getting ridiculous. Would someone please accept the patch,
tell me what to fix in it to make it acceptable, or explain why it is
a bad idea?

Thanks!

On Mon, Apr 25, 2011 at 9:08 AM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> Ping? Ping? Ping?
>
> On Mon, Apr 18, 2011 at 9:45 AM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
>> Ping? Ping?
>>
>> On Mon, Apr 11, 2011 at 11:00 AM, Paul Pluzhnikov
>> <ppluzhnikov@google.com> wrote:
>>> Ping?
>
> --
> Paul Pluzhnikov
>



-- 
Paul Pluzhnikov

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch] make default linker --hash-style configurable option
  2011-05-02 14:23           ` Paul Pluzhnikov
@ 2011-05-02 15:00             ` Joseph S. Myers
  2011-05-02 15:57               ` Paul Pluzhnikov
  0 siblings, 1 reply; 26+ messages in thread
From: Joseph S. Myers @ 2011-05-02 15:00 UTC (permalink / raw)
  To: Paul Pluzhnikov; +Cc: Matthias Klose, gcc-patches, satorux, iant

On Mon, 2 May 2011, Paul Pluzhnikov wrote:

> Ping? Ping? Ping? Ping?
> 
> This is getting ridiculous. Would someone please accept the patch,
> tell me what to fix in it to make it acceptable, or explain why it is
> a bad idea?

When pinging, please include the URL of the patch being pinged and CC 
relevant maintainers (in this case, build system maintainers).

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch] make default linker --hash-style configurable option
  2011-05-02 15:00             ` Joseph S. Myers
@ 2011-05-02 15:57               ` Paul Pluzhnikov
  2011-05-09 17:01                 ` Paul Pluzhnikov
  0 siblings, 1 reply; 26+ messages in thread
From: Paul Pluzhnikov @ 2011-05-02 15:57 UTC (permalink / raw)
  To: Joseph S. Myers
  Cc: Matthias Klose, gcc-patches, satorux, iant, bonzini, aoliva

On Mon, May 2, 2011 at 7:59 AM, Joseph S. Myers <joseph@codesourcery.com> wrote:

Thanks for your comments.

> When pinging, please include the URL of the patch being pinged and CC

http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00246.html

> relevant maintainers (in this case, build system maintainers).

All of them? [I've started with two ...]

Thanks!
-- 
Paul Pluzhnikov

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch] make default linker --hash-style configurable option
  2011-05-02 15:57               ` Paul Pluzhnikov
@ 2011-05-09 17:01                 ` Paul Pluzhnikov
  2011-05-09 17:04                   ` Paolo Bonzini
  0 siblings, 1 reply; 26+ messages in thread
From: Paul Pluzhnikov @ 2011-05-09 17:01 UTC (permalink / raw)
  To: Joseph S. Myers
  Cc: Matthias Klose, gcc-patches, satorux, iant, bonzini, aoliva, dj,
	neroden, Ralf.Wildenhues

Ping? Ping? Ping? Ping? Ping?

http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00246.html

CC'ing the rest of build system maintainers.

On Mon, May 2, 2011 at 8:56 AM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> On Mon, May 2, 2011 at 7:59 AM, Joseph S. Myers <joseph@codesourcery.com> wrote:
>
> Thanks for your comments.
>
>> When pinging, please include the URL of the patch being pinged and CC
>
> http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00246.html
>
>> relevant maintainers (in this case, build system maintainers).
>
> All of them? [I've started with two ...]


Thanks!

-- 
Paul Pluzhnikov

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch] make default linker --hash-style configurable option
  2011-05-09 17:01                 ` Paul Pluzhnikov
@ 2011-05-09 17:04                   ` Paolo Bonzini
  2011-05-09 17:26                     ` Joseph S. Myers
  0 siblings, 1 reply; 26+ messages in thread
From: Paolo Bonzini @ 2011-05-09 17:04 UTC (permalink / raw)
  To: Paul Pluzhnikov
  Cc: Joseph S. Myers, Matthias Klose, gcc-patches, satorux, iant,
	aoliva, dj, neroden, Ralf.Wildenhues

On 05/09/2011 05:59 PM, Paul Pluzhnikov wrote:
> Ping? Ping? Ping? Ping? Ping?
>
> http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00246.html
>
> CC'ing the rest of build system maintainers.

None of the build system maintainers can approve gcc.c changes.  And 
those can be approved only by either a global reviewer, or by Joseph. 
That's why I haven't replied anything up to now.

Paolo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch] make default linker --hash-style configurable option
  2011-05-09 17:04                   ` Paolo Bonzini
@ 2011-05-09 17:26                     ` Joseph S. Myers
  2011-05-09 17:31                       ` Paolo Bonzini
  0 siblings, 1 reply; 26+ messages in thread
From: Joseph S. Myers @ 2011-05-09 17:26 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Paul Pluzhnikov, Matthias Klose, gcc-patches, satorux, iant,
	aoliva, dj, neroden, Ralf.Wildenhues

On Mon, 9 May 2011, Paolo Bonzini wrote:

> On 05/09/2011 05:59 PM, Paul Pluzhnikov wrote:
> > Ping? Ping? Ping? Ping? Ping?
> > 
> > http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00246.html
> > 
> > CC'ing the rest of build system maintainers.
> 
> None of the build system maintainers can approve gcc.c changes.  And those can
> be approved only by either a global reviewer, or by Joseph. That's why I
> haven't replied anything up to now.

I'm thinking of it as a build-system patch with a driver bit - where build 
system maintainers need to decide the general principle of the 
desirability of the feature and what all of the implementation outside 
gcc.c should look like, before it makes sense to review the details of the 
gcc.c bit.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch] make default linker --hash-style configurable option
  2011-05-09 17:31                       ` Paolo Bonzini
@ 2011-05-09 17:31                         ` Joseph S. Myers
  2011-05-09 18:00                           ` Paul Pluzhnikov
  0 siblings, 1 reply; 26+ messages in thread
From: Joseph S. Myers @ 2011-05-09 17:31 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Paul Pluzhnikov, Matthias Klose, gcc-patches, satorux, iant,
	aoliva, dj, neroden, Ralf.Wildenhues

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1304 bytes --]

On Mon, 9 May 2011, Paolo Bonzini wrote:

> On Mon, May 9, 2011 at 18:45, Joseph S. Myers <joseph@codesourcery.com> wrote:
> > On Mon, 9 May 2011, Paolo Bonzini wrote:
> >
> >> On 05/09/2011 05:59 PM, Paul Pluzhnikov wrote:
> >> > Ping? Ping? Ping? Ping? Ping?
> >> >
> >> > http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00246.html
> >> >
> >> > CC'ing the rest of build system maintainers.
> >>
> >> None of the build system maintainers can approve gcc.c changes.  And those can
> >> be approved only by either a global reviewer, or by Joseph. That's why I
> >> haven't replied anything up to now.
> >
> > I'm thinking of it as a build-system patch with a driver bit - where build
> > system maintainers need to decide the general principle of the
> > desirability of the feature and what all of the implementation outside
> > gcc.c should look like, before it makes sense to review the details of the
> > gcc.c bit.
> 
> Uhm, so we deadlocked, I thought the other way.  I cannot really
> express any opinion about the desirability of the feature, but the
> configure syntax is certainly okay with me, and I gather from the
> thread that you are fine with that as well.

Given the build system changes, the gcc.c changes are OK.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch] make default linker --hash-style configurable option
  2011-05-09 17:26                     ` Joseph S. Myers
@ 2011-05-09 17:31                       ` Paolo Bonzini
  2011-05-09 17:31                         ` Joseph S. Myers
  0 siblings, 1 reply; 26+ messages in thread
From: Paolo Bonzini @ 2011-05-09 17:31 UTC (permalink / raw)
  To: Joseph S. Myers
  Cc: Paul Pluzhnikov, Matthias Klose, gcc-patches, satorux, iant,
	aoliva, dj, neroden, Ralf.Wildenhues

On Mon, May 9, 2011 at 18:45, Joseph S. Myers <joseph@codesourcery.com> wrote:
> On Mon, 9 May 2011, Paolo Bonzini wrote:
>
>> On 05/09/2011 05:59 PM, Paul Pluzhnikov wrote:
>> > Ping? Ping? Ping? Ping? Ping?
>> >
>> > http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00246.html
>> >
>> > CC'ing the rest of build system maintainers.
>>
>> None of the build system maintainers can approve gcc.c changes.  And those can
>> be approved only by either a global reviewer, or by Joseph. That's why I
>> haven't replied anything up to now.
>
> I'm thinking of it as a build-system patch with a driver bit - where build
> system maintainers need to decide the general principle of the
> desirability of the feature and what all of the implementation outside
> gcc.c should look like, before it makes sense to review the details of the
> gcc.c bit.

Uhm, so we deadlocked, I thought the other way.  I cannot really
express any opinion about the desirability of the feature, but the
configure syntax is certainly okay with me, and I gather from the
thread that you are fine with that as well.

Paolo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch] make default linker --hash-style configurable option
  2011-05-09 17:31                         ` Joseph S. Myers
@ 2011-05-09 18:00                           ` Paul Pluzhnikov
  2011-05-10 11:36                             ` Richard Guenther
  0 siblings, 1 reply; 26+ messages in thread
From: Paul Pluzhnikov @ 2011-05-09 18:00 UTC (permalink / raw)
  To: Joseph S. Myers
  Cc: Paolo Bonzini, Matthias Klose, gcc-patches, satorux, iant,
	aoliva, dj, neroden, Ralf.Wildenhues

On Mon, May 9, 2011 at 9:51 AM, Joseph S. Myers <joseph@codesourcery.com> wrote:
> On Mon, 9 May 2011, Paolo Bonzini wrote:
>
>> Uhm, so we deadlocked, I thought the other way.  I cannot really
>> express any opinion about the desirability of the feature, but the
>> configure syntax is certainly okay with me, and I gather from the
>> thread that you are fine with that as well.
>
> Given the build system changes, the gcc.c changes are OK.

Ok for trunk then?

I'll wait till tomorrow in case someone has additional comments on the
desirability part.

Thanks!
-- 
Paul Pluzhnikov

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch] make default linker --hash-style configurable option
  2011-05-09 18:00                           ` Paul Pluzhnikov
@ 2011-05-10 11:36                             ` Richard Guenther
  2011-05-10 15:00                               ` Ian Lance Taylor
  0 siblings, 1 reply; 26+ messages in thread
From: Richard Guenther @ 2011-05-10 11:36 UTC (permalink / raw)
  To: Paul Pluzhnikov
  Cc: Joseph S. Myers, Paolo Bonzini, Matthias Klose, gcc-patches,
	satorux, iant, aoliva, dj, neroden, Ralf.Wildenhues

On Mon, May 9, 2011 at 7:11 PM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> On Mon, May 9, 2011 at 9:51 AM, Joseph S. Myers <joseph@codesourcery.com> wrote:
>> On Mon, 9 May 2011, Paolo Bonzini wrote:
>>
>>> Uhm, so we deadlocked, I thought the other way.  I cannot really
>>> express any opinion about the desirability of the feature, but the
>>> configure syntax is certainly okay with me, and I gather from the
>>> thread that you are fine with that as well.
>>
>> Given the build system changes, the gcc.c changes are OK.
>
> Ok for trunk then?
>
> I'll wait till tomorrow in case someone has additional comments on the
> desirability part.

I wonder why this is a GCC specific patch and not a linker patch.  Why
not change the linker(s) to accept such configure option that changes its
default behavior?

Otherwise if people link with ld they suddenly get different hash-style.
That looks wrong to me.

Richard.

> Thanks!
> --
> Paul Pluzhnikov
>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch] make default linker --hash-style configurable option
  2011-05-10 11:36                             ` Richard Guenther
@ 2011-05-10 15:00                               ` Ian Lance Taylor
  2011-05-10 15:08                                 ` Jakub Jelinek
  2011-05-10 15:23                                 ` Paul Pluzhnikov
  0 siblings, 2 replies; 26+ messages in thread
From: Ian Lance Taylor @ 2011-05-10 15:00 UTC (permalink / raw)
  To: Richard Guenther
  Cc: Paul Pluzhnikov, Joseph S. Myers, Paolo Bonzini, Matthias Klose,
	gcc-patches, satorux, aoliva, dj, neroden, Ralf.Wildenhues

Richard Guenther <richard.guenther@gmail.com> writes:

> On Mon, May 9, 2011 at 7:11 PM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
>> On Mon, May 9, 2011 at 9:51 AM, Joseph S. Myers <joseph@codesourcery.com> wrote:
>>> On Mon, 9 May 2011, Paolo Bonzini wrote:
>>>
>>>> Uhm, so we deadlocked, I thought the other way.  I cannot really
>>>> express any opinion about the desirability of the feature, but the
>>>> configure syntax is certainly okay with me, and I gather from the
>>>> thread that you are fine with that as well.
>>>
>>> Given the build system changes, the gcc.c changes are OK.
>>
>> Ok for trunk then?
>>
>> I'll wait till tomorrow in case someone has additional comments on the
>> desirability part.
>
> I wonder why this is a GCC specific patch and not a linker patch.  Why
> not change the linker(s) to accept such configure option that changes its
> default behavior?

It is traditionally gcc which tells the linker what to do.  E.g., Fedora
has patched gcc to pass --hash-style=gnu to the linker.

> Otherwise if people link with ld they suddenly get different hash-style.
> That looks wrong to me.

That turns out not to be the case.  Both gold and GNU ld accept the same
set of --hash-style options.

Ian

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch] make default linker --hash-style configurable option
  2011-05-10 15:00                               ` Ian Lance Taylor
@ 2011-05-10 15:08                                 ` Jakub Jelinek
  2011-05-10 15:23                                 ` Paul Pluzhnikov
  1 sibling, 0 replies; 26+ messages in thread
From: Jakub Jelinek @ 2011-05-10 15:08 UTC (permalink / raw)
  To: Ian Lance Taylor
  Cc: Richard Guenther, Paul Pluzhnikov, Joseph S. Myers,
	Paolo Bonzini, Matthias Klose, gcc-patches, satorux, aoliva, dj,
	neroden, Ralf.Wildenhues

On Tue, May 10, 2011 at 07:02:44AM -0700, Ian Lance Taylor wrote:
> > Otherwise if people link with ld they suddenly get different hash-style.
> > That looks wrong to me.
> 
> That turns out not to be the case.  Both gold and GNU ld accept the same
> set of --hash-style options.

And we keep telling people they shouldn't be invoking ld directly, they
usually don't pass correct arguments.

	Jakub

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch] make default linker --hash-style configurable option
  2011-05-10 15:00                               ` Ian Lance Taylor
  2011-05-10 15:08                                 ` Jakub Jelinek
@ 2011-05-10 15:23                                 ` Paul Pluzhnikov
  2011-05-10 15:26                                   ` Richard Guenther
  1 sibling, 1 reply; 26+ messages in thread
From: Paul Pluzhnikov @ 2011-05-10 15:23 UTC (permalink / raw)
  To: Ian Lance Taylor
  Cc: Richard Guenther, Joseph S. Myers, Paolo Bonzini, Matthias Klose,
	gcc-patches, satorux, aoliva, dj, neroden, Ralf.Wildenhues

On Tue, May 10, 2011 at 7:02 AM, Ian Lance Taylor <iant@google.com> wrote:
> Richard Guenther <richard.guenther@gmail.com> writes:

>> I wonder why this is a GCC specific patch and not a linker patch.  Why
>> not change the linker(s) to accept such configure option that changes its
>> default behavior?
>
> It is traditionally gcc which tells the linker what to do.  E.g., Fedora
> has patched gcc to pass --hash-style=gnu to the linker.

I think 'traditionally' is the key here.

The same argument applies to e.g. --build-id. We already have GCC
supply that, but we could have changed the default in
binutils/{ld,gold}.

Also, the argument is discoverable (by observing 'gcc -v' output).
Changing the default in {ld,gold} makes it hard(er) to understand why
e.g. binaries linked from the same object files using two different
versions of GCC are different.

>> Otherwise if people link with ld they suddenly get different hash-style.
>> That looks wrong to me.
>
> That turns out not to be the case.

I think Richard meant "if people link directly with either ld or gold
(i.e. bypassing 'gcc' driver) get different hash-style". That's kind
of expected though -- if you want to get the same output you get from
'gcc', you need to examine 'gcc -v' very carefully. And, as Jakub
noted, linking directly with 'ld' is discouraged.

Thanks,
-- 
Paul Pluzhnikov

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch] make default linker --hash-style configurable option
  2011-05-10 15:23                                 ` Paul Pluzhnikov
@ 2011-05-10 15:26                                   ` Richard Guenther
  0 siblings, 0 replies; 26+ messages in thread
From: Richard Guenther @ 2011-05-10 15:26 UTC (permalink / raw)
  To: Paul Pluzhnikov
  Cc: Ian Lance Taylor, Joseph S. Myers, Paolo Bonzini, Matthias Klose,
	gcc-patches, satorux, aoliva, dj, neroden, Ralf.Wildenhues

On Tue, May 10, 2011 at 4:24 PM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> On Tue, May 10, 2011 at 7:02 AM, Ian Lance Taylor <iant@google.com> wrote:
>> Richard Guenther <richard.guenther@gmail.com> writes:
>
>>> I wonder why this is a GCC specific patch and not a linker patch.  Why
>>> not change the linker(s) to accept such configure option that changes its
>>> default behavior?
>>
>> It is traditionally gcc which tells the linker what to do.  E.g., Fedora
>> has patched gcc to pass --hash-style=gnu to the linker.
>
> I think 'traditionally' is the key here.
>
> The same argument applies to e.g. --build-id. We already have GCC
> supply that, but we could have changed the default in
> binutils/{ld,gold}.

I think we patched binutils for hash-style (we use =both) and gcc for build-id.

Richard.

> Also, the argument is discoverable (by observing 'gcc -v' output).
> Changing the default in {ld,gold} makes it hard(er) to understand why
> e.g. binaries linked from the same object files using two different
> versions of GCC are different.
>
>>> Otherwise if people link with ld they suddenly get different hash-style.
>>> That looks wrong to me.
>>
>> That turns out not to be the case.
>
> I think Richard meant "if people link directly with either ld or gold
> (i.e. bypassing 'gcc' driver) get different hash-style". That's kind
> of expected though -- if you want to get the same output you get from
> 'gcc', you need to examine 'gcc -v' very carefully. And, as Jakub
> noted, linking directly with 'ld' is discouraged.
>
> Thanks,
> --
> Paul Pluzhnikov
>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch] make default linker --hash-style configurable option
  2011-04-04 18:17 [patch] make default linker --hash-style configurable option Paul Pluzhnikov
  2011-04-04 18:22 ` Rainer Orth
  2011-04-04 22:53 ` Matthias Klose
@ 2011-05-12  9:10 ` Eric Botcazou
  2011-05-12  9:19   ` Eric Botcazou
  2011-05-12  9:23   ` Paul Pluzhnikov
  2 siblings, 2 replies; 26+ messages in thread
From: Eric Botcazou @ 2011-05-12  9:10 UTC (permalink / raw)
  To: Paul Pluzhnikov; +Cc: gcc-patches, gcc-patches, satorux, iant

> 2011-04-04  Satoru Takabayashi  <satorux@google.com>
> 	    Paul Pluzhnikov  <ppluzhnikov@google.com>
>
> 	* gcc/doc/install.texi (Configuration): Document
> 	--with-linker-hash-style.
> 	* gcc/gcc.c (init_spec): Handle LINKER_HASH_STYLE.
> 	* gcc/config.in: Add LINKER_HASH_STYLE.
> 	* gcc/configure.ac: Add --with-linker-hash-style.
> 	* gcc/configure: Regenerate.

No gcc/ prefix in the ChangeLog file of the gcc/ directory.  See other entries.

-- 
Eric Botcazou

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch] make default linker --hash-style configurable option
  2011-05-12  9:10 ` Eric Botcazou
@ 2011-05-12  9:19   ` Eric Botcazou
  2011-05-12  9:23   ` Paul Pluzhnikov
  1 sibling, 0 replies; 26+ messages in thread
From: Eric Botcazou @ 2011-05-12  9:19 UTC (permalink / raw)
  To: Paul Pluzhnikov; +Cc: gcc-patches, gcc-patches, satorux, iant

> 2011-04-04  Satoru Takabayashi  <satorux@google.com>
> 	    Paul Pluzhnikov  <ppluzhnikov@google.com>
>
> 	* gcc/doc/install.texi (Configuration): Document
> 	--with-linker-hash-style.
> 	* gcc/gcc.c (init_spec): Handle LINKER_HASH_STYLE.
> 	* gcc/config.in: Add LINKER_HASH_STYLE.
> 	* gcc/configure.ac: Add --with-linker-hash-style.
> 	* gcc/configure: Regenerate.

No gcc/ prefix in the ChangeLog file of the gcc/ directory.  See other entries.

-- 
Eric Botcazou

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch] make default linker --hash-style configurable option
  2011-05-12  9:10 ` Eric Botcazou
  2011-05-12  9:19   ` Eric Botcazou
@ 2011-05-12  9:23   ` Paul Pluzhnikov
  2011-05-12  9:32     ` Paul Pluzhnikov
  1 sibling, 1 reply; 26+ messages in thread
From: Paul Pluzhnikov @ 2011-05-12  9:23 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches, gcc-patches, satorux, iant

On Wed, May 11, 2011 at 4:01 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:

> No gcc/ prefix in the ChangeLog file of the gcc/ directory.  See other entries.

Fixed. Sorry about that.

-- 
Paul Pluzhnikov

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch] make default linker --hash-style configurable option
  2011-05-12  9:23   ` Paul Pluzhnikov
@ 2011-05-12  9:32     ` Paul Pluzhnikov
  0 siblings, 0 replies; 26+ messages in thread
From: Paul Pluzhnikov @ 2011-05-12  9:32 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches, gcc-patches, satorux, iant

On Wed, May 11, 2011 at 4:01 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:

> No gcc/ prefix in the ChangeLog file of the gcc/ directory.  See other entries.

Fixed. Sorry about that.

-- 
Paul Pluzhnikov

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2011-05-11 23:17 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-04 18:17 [patch] make default linker --hash-style configurable option Paul Pluzhnikov
2011-04-04 18:22 ` Rainer Orth
2011-04-04 18:26   ` Ian Lance Taylor
2011-04-04 22:53 ` Matthias Klose
2011-04-05  0:01   ` Joseph S. Myers
2011-04-11 18:01     ` Paul Pluzhnikov
2011-04-18 17:20       ` Paul Pluzhnikov
2011-04-25 17:03         ` Paul Pluzhnikov
2011-05-02 14:23           ` Paul Pluzhnikov
2011-05-02 15:00             ` Joseph S. Myers
2011-05-02 15:57               ` Paul Pluzhnikov
2011-05-09 17:01                 ` Paul Pluzhnikov
2011-05-09 17:04                   ` Paolo Bonzini
2011-05-09 17:26                     ` Joseph S. Myers
2011-05-09 17:31                       ` Paolo Bonzini
2011-05-09 17:31                         ` Joseph S. Myers
2011-05-09 18:00                           ` Paul Pluzhnikov
2011-05-10 11:36                             ` Richard Guenther
2011-05-10 15:00                               ` Ian Lance Taylor
2011-05-10 15:08                                 ` Jakub Jelinek
2011-05-10 15:23                                 ` Paul Pluzhnikov
2011-05-10 15:26                                   ` Richard Guenther
2011-05-12  9:10 ` Eric Botcazou
2011-05-12  9:19   ` Eric Botcazou
2011-05-12  9:23   ` Paul Pluzhnikov
2011-05-12  9:32     ` Paul Pluzhnikov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).