diff mbox

Call GNU ld with -O*

Message ID alpine.DEB.2.10.1307121802190.7127@stedding.saclay.inria.fr
State New
Headers show

Commit Message

Marc Glisse July 12, 2013, 4:12 p.m. UTC 
On Fri, 12 Jul 2013, Jakub Jelinek wrote:

> On Fri, Jul 12, 2013 at 04:54:21PM +0200, Marc Glisse wrote:
>> +/* GNU ld has -O1 and gold -O2, but we only pass it with -O3, -Os or -Ofast. */
>> +#ifndef LINK_OPT_SPEC
>> +#if HAVE_GNU_LD
>> +#define LINK_OPT_SPEC "%{O*:%{!O0:%{!O1:%{!O2:%{!Og:%{!O:-O2}}}}}} "
>
> Wouldn't something like "%{O*:%{O|O0|O1|O2|Og:;:-O2}}" work?

It does (I only did a non-bootstrap c,c++ build and some manual tests with 
-v). Thanks, I didn't know this syntax.

Is the new version ok? Any tests I should do that would be more useful 
than running the testsuite? (I tried with several -O options in various 
orders and made sure that a -Wl,-On ended up later in the command line)

Comments

Jakub Jelinek July 12, 2013, 4:22 p.m. UTC | #1 
On Fri, Jul 12, 2013 at 06:12:26PM +0200, Marc Glisse wrote:
> On Fri, 12 Jul 2013, Jakub Jelinek wrote:
> 
> >On Fri, Jul 12, 2013 at 04:54:21PM +0200, Marc Glisse wrote:
> >>+/* GNU ld has -O1 and gold -O2, but we only pass it with -O3, -Os or -Ofast. */
> >>+#ifndef LINK_OPT_SPEC
> >>+#if HAVE_GNU_LD
> >>+#define LINK_OPT_SPEC "%{O*:%{!O0:%{!O1:%{!O2:%{!Og:%{!O:-O2}}}}}} "
> >
> >Wouldn't something like "%{O*:%{O|O0|O1|O2|Og:;:-O2}}" work?
> 
> It does (I only did a non-bootstrap c,c++ build and some manual
> tests with -v). Thanks, I didn't know this syntax.

gcc.c has a huge comment that describes the syntax.

> Is the new version ok? Any tests I should do that would be more
> useful than running the testsuite? (I tried with several -O options
> in various orders and made sure that a -Wl,-On ended up later in the
> command line)

I'd say it is important to try say -O17 -O0, -O -O5 etc. and see whether
it works as desired.  If that works, the patch is ok, with proper ChangeLog
entry.

	Jakub
Marc Glisse July 12, 2013, 5:02 p.m. UTC | #2 
On Fri, 12 Jul 2013, Jakub Jelinek wrote:

> gcc.c has a huge comment that describes the syntax.

I know, my first reflex after reading your email was to re-read that 
comment to understand what you had written ;-)

>> Is the new version ok? Any tests I should do that would be more
>> useful than running the testsuite? (I tried with several -O options
>> in various orders and made sure that a -Wl,-On ended up later in the
>> command line)
>
> I'd say it is important to try say -O17 -O0, -O -O5 etc. and see whether
> it works as desired.

Yes, that's what I had checked.

> If that works, the patch is ok, with proper ChangeLog entry.

Oups, it was in the first email and I forgot to copy it in the second 
version.

Thanks,
diff mbox

Patch

Index: gcc.c
===================================================================
--- gcc.c	(revision 200921)
+++ gcc.c	(working copy)
@@ -715,36 +715,45 @@  proper position among the other output f
 /* Linker command line options for -fsanitize= late on the command line.  */
 #ifndef SANITIZER_SPEC
 #define SANITIZER_SPEC "\
 %{!nostdlib:%{!nodefaultlibs:%{fsanitize=address:" LIBASAN_SPEC "\
     %{static:%ecannot specify -static with -fsanitize=address}\
     %{fsanitize=thread:%e-fsanitize=address is incompatible with -fsanitize=thread}}\
     %{fsanitize=thread:" LIBTSAN_SPEC "\
     %{!pie:%{!shared:%e-fsanitize=thread linking must be done with -pie or -shared}}}}}"
 #endif
 
+/* GNU ld has -O1 and gold -O2, but we only pass it with -O3, -Os or -Ofast. */
+#ifndef LINK_OPT_SPEC
+#if HAVE_GNU_LD
+#define LINK_OPT_SPEC "%{O*:%{O|O0|O1|O2|Og:;:-O2}} "
+#else
+#define LINK_OPT_SPEC ""
+#endif
+#endif
+
 /* -u* was put back because both BSD and SysV seem to support it.  */
 /* %{static:} simply prevents an error message if the target machine
    doesn't handle -static.  */
 /* We want %{T*} after %{L*} and %D so that it can be used to specify linker
    scripts which exist in user specified directories, or in standard
    directories.  */
 /* We pass any -flto flags on to the linker, which is expected
    to understand them.  In practice, this means it had better be collect2.  */
 /* %{e*} includes -export-dynamic; see comment in common.opt.  */
 #ifndef LINK_COMMAND_SPEC
 #define LINK_COMMAND_SPEC "\
 %{!fsyntax-only:%{!c:%{!M:%{!MM:%{!E:%{!S:\
     %(linker) " \
     LINK_PLUGIN_SPEC \
    "%{flto|flto=*:%<fcompare-debug*} \
-    %{flto} %{flto=*} %l " LINK_PIE_SPEC \
+    %{flto} %{flto=*} %l " LINK_PIE_SPEC LINK_OPT_SPEC \
    "%{fuse-ld=*:-fuse-ld=%*}\
     %X %{o*} %{e*} %{N} %{n} %{r}\
     %{s} %{t} %{u*} %{z} %{Z} %{!nostdlib:%{!nostartfiles:%S}}\
     %{static:} %{L*} %(mfwrap) %(link_libgcc) " SANITIZER_EARLY_SPEC " %o\
     %{fopenmp|ftree-parallelize-loops=*:%:include(libgomp.spec)%(link_gomp)}\
     %{fgnu-tm:%:include(libitm.spec)%(link_itm)}\
     %(mflib) " STACK_SPLIT_SPEC "\
     %{fprofile-arcs|fprofile-generate*|coverage:-lgcov} " SANITIZER_SPEC " \
     %{!nostdlib:%{!nodefaultlibs:%(link_ssp) %(link_gcc_c_sequence)}}\
     %{!nostdlib:%{!nostartfiles:%E}} %{T*} }}}}}}"