[PATCH 1/2] Fix LTO bootstrap documentation

[PATCH 1/2] Fix LTO bootstrap documentation

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

BUILD_CONFIG=bootstrap-lto does not work currently and causes build
failures.  Remove a reference to that in the install documentation and instead
document a working method using BOOT_CFLAGS.

Bootstrapped on x86-64 together with the -fwhopr=jobserver patchkit

v2: Fix grammar/texinfo based on feedback

Ok to commit?

2010-08-17  Andi Kleen	<ak@linux.intel.com>

	* doc/install.texi (Building a native compiler): Remove
	reference to broken BUILD_CONFIG=bootstrap-lto.
	Document working method instead.
---
 gcc/doc/install.texi |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index fc9b988..e98675d 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -2108,6 +2108,13 @@ to work around this, by choosing @code{BOOT_CFLAGS} to avoid the parts
 of the stage1 compiler that were miscompiled, or by using @samp{make
 bootstrap4} to increase the number of stages of bootstrap.
 
+You can also use @code{BOOT_CFLAGS} to force an LTO bootstrap with
+@samp{BOOT_CFLAGS='-O2 -fwhopr=jobserver -fuse-linker-plugin -frandom-seed=1'}. 
+Configure the compiler with @samp{--enable-stage1-languages=c,lto}. This 
+will only work with the @command{gold} linker and if linker plugins are
+correctly configured.  The @option{-frandom-seed=1} is needed to avoid 
+bootstrap comparison failures.
+
 @code{BOOT_CFLAGS} does not apply to bootstrapped target libraries.
 Since these are always compiled with the compiler currently being
 bootstrapped, you can use @code{CFLAGS_FOR_TARGET} to modify their
@@ -2155,11 +2162,6 @@ Removes any @option{-O}-started option from @code{BOOT_CFLAGS}, and adds
 @item @samp{bootstrap-O3}
 Analogous to @code{bootstrap-O1}.
 
-@item @samp{bootstrap-lto}
-Enables Link-Time Optimization for host tools during bootstrapping.
-@samp{BUILD_CONFIG=bootstrap-lto} is equivalent to adding
-@option{-flto} to @samp{BOOT_CFLAGS}.
-
 @item @samp{bootstrap-debug}
 Verifies that the compiler generates the same executable code, whether
 or not it is asked to emit debug information.  To this end, this
-- 
1.7.1

[PATCH 2/2] -fwhopr=jobserver

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

This patch adds a new -fwhopr=jobserver mode. This is useful
in parallel builds to let the submake started by lto-driver communicate
with the parent make that controls the whole build to control
the number of parallel jobs.

Currently -fwhopr=N does not pass through the jobserver options
because it cannot guarantee that the submake is GNU make.
With -fwhopr=jobserver the user specifies this explicitely,
so it's safe to do.

Based on discussions with Richard Guenther.

Passed normal boot strap and testing on x86-64 and a full
LTO bootstrap with -fwhopr=jobserver and a parallel build,
on x86-64-linux.

v2: Fix documentation based on feedback

Ok to commit?
-Andi

2010-08-17  Andi Kleen      <ak@linux.intel.com>

	* common.opt (fwhopr=): Update for -fwhopr=jobserver
	* doc/invoke.text (fwhopr): Document -fwhopr=jobserver.
	* lto-wrapper.c (run_gcc): Add jobserver mode.
	* opts.c (common_handle_option): Fix OPT_fwhopr for non numeric
	argument.
---
 gcc/common.opt      |    4 ++--
 gcc/doc/invoke.texi |    7 +++++++
 gcc/lto-wrapper.c   |   38 ++++++++++++++++++++++++++++----------
 gcc/opts.c          |    2 +-
 4 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/gcc/common.opt b/gcc/common.opt
index 1285ff0..1e753c9 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1516,8 +1516,8 @@ Common
 Enable partitioned link-time optimization
 
 fwhopr=
-Common RejectNegative UInteger Joined Var(flag_whopr)
-Enable partitioned link-time optimization with specified number of parallel jobs
+Common RejectNegative Joined Var(flag_whopr)
+Partitioned link-time optimization with number of parallel jobs or jobserver.
 
 ftree-builtin-call-dce
 Common Report Var(flag_tree_builtin_call_dce) Init(0) Optimization
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 6099b30..c72938d 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -7556,6 +7556,13 @@ parallel using @var{n} parallel jobs by utilizing an installed
 @command{make} program.  The environment variable @env{MAKE} may be
 used to override the program used.
 
+You can also specify @option{-fwhopr=jobserver} to use GNU make's 
+job server mode to determine the number of parallel jobs. This 
+is useful when the Makefile calling GCC is already parallel.
+The parent Makefile will need a '+' prepended to the command recipe
+for this to work. This will likely only work if @env{MAKE} is 
+GNU make.
+
 Disabled by default.
 
 @item -fwpa
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index da120b3..d0f1256 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -303,6 +303,7 @@ run_gcc (unsigned argc, char *argv[])
   struct obstack env_obstack;
   bool seen_o = false;
   int parallel = 0;
+  int jobserver = 0;
 
   /* Get the driver and options.  */
   collect_gcc = getenv ("COLLECT_GCC");
@@ -373,9 +374,17 @@ run_gcc (unsigned argc, char *argv[])
 	    lto_mode = LTO_MODE_WHOPR;
 	    if (option[7] == '=')
 	      {
-		parallel = atoi (option+8);
-		if (parallel <= 1)
-		  parallel = 0;
+		if (!strcmp (option + 8, "jobserver"))
+		  {
+		    jobserver = 1;
+		    parallel = 1;
+		  }
+		else
+		  {
+		    parallel = atoi (option+8);
+		    if (parallel <= 1)
+		      parallel = 0;
+		  }
 	      }
 	  }
 	else
@@ -567,23 +576,32 @@ cont:
 	{
 	  struct pex_obj *pex;
 	  char jobs[32];
+
 	  fprintf (mstream, "all:");
 	  for (i = 0; i < nr; ++i)
 	    fprintf (mstream, " \\\n\t%s", output_names[i]);
 	  fprintf (mstream, "\n");
 	  fclose (mstream);
-	  /* Avoid passing --jobserver-fd= and similar flags.  */
-	  putenv (xstrdup ("MAKEFLAGS="));
-	  putenv (xstrdup ("MFLAGS="));
+	  if (!jobserver)
+	    {
+	      /* Avoid passing --jobserver-fd= and similar flags 
+		 unless jobserver mode is explicitely enabled.*/
+	      putenv (xstrdup ("MAKEFLAGS="));
+	      putenv (xstrdup ("MFLAGS="));
+	    }
 	  new_argv[0] = getenv ("MAKE");
 	  if (!new_argv[0])
 	    new_argv[0] = "make";
 	  new_argv[1] = "-f";
 	  new_argv[2] = makefile;
-	  snprintf (jobs, 31, "-j%d", parallel);
-	  new_argv[3] = jobs;
-	  new_argv[4] = "all";
-	  new_argv[5] = NULL;
+	  i = 3;
+	  if (!jobserver)
+	    {
+	      snprintf (jobs, 31, "-j%d", parallel);
+	      new_argv[i++] = jobs;
+	    }
+	  new_argv[i++] = "all";
+	  new_argv[i++] = NULL;
 	  pex = collect_execute (CONST_CAST (char **, new_argv));
 	  collect_wait (new_argv[0], pex);
 	  maybe_unlink_file (makefile);
diff --git a/gcc/opts.c b/gcc/opts.c
index 6e52805..399bd4a 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2077,7 +2077,7 @@ common_handle_option (const struct cl_decoded_option *decoded,
       break;
 
     case OPT_fwhopr:
-      flag_whopr = value;
+      flag_whopr = arg;
       break;
 
     case OPT_w:
-- 
1.7.1

Re: [PATCH 2/2] -fwhopr=jobserver

[Joseph S. Myers]

On Tue, 31 Aug 2010, Andi Kleen wrote:

> 	* doc/invoke.text (fwhopr): Document -fwhopr=jobserver.
                     ^ texi

> +The parent Makefile will need a '+' prepended to the command recipe

@samp{+}

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH 2/2] -fwhopr=jobserver

[Diego Novillo]

On 10-08-31 06:33 , Andi Kleen wrote:

> 2010-08-17  Andi Kleen<ak@linux.intel.com>
>
> 	* common.opt (fwhopr=): Update for -fwhopr=jobserver
> 	* doc/invoke.text (fwhopr): Document -fwhopr=jobserver.
> 	* lto-wrapper.c (run_gcc): Add jobserver mode.
> 	* opts.c (common_handle_option): Fix OPT_fwhopr for non numeric
> 	argument.

OK with

> +	  if (!jobserver)
> +	    {
> +	      /* Avoid passing --jobserver-fd= and similar flags
> +		 unless jobserver mode is explicitely enabled.*/

s/explicitely/explicitly/

Two spaces after '.'.


Diego.

Re: [PATCH 1/2] Fix LTO bootstrap documentation

[Diego Novillo]

On 10-08-31 06:33 , Andi Kleen wrote:

> @@ -2108,6 +2108,13 @@ to work around this, by choosing @code{BOOT_CFLAGS} to avoid the parts
>   of the stage1 compiler that were miscompiled, or by using @samp{make
>   bootstrap4} to increase the number of stages of bootstrap.
>
> +You can also use @code{BOOT_CFLAGS} to force an LTO bootstrap with
> +@samp{BOOT_CFLAGS='-O2 -fwhopr=jobserver -fuse-linker-plugin -frandom-seed=1'}.
> +Configure the compiler with @samp{--enable-stage1-languages=c,lto}. This
> +will only work with the @command{gold} linker and if linker plugins are
> +correctly configured.  The @option{-frandom-seed=1} is needed to avoid
> +bootstrap comparison failures.

Could you add an explanation on why frandom-seed avoids the comparison 
failures?  It's not immediately obvious.

OK with that change.


Diego.

Re: [PATCH 1/2] Fix LTO bootstrap documentation

[Paolo Bonzini]

On 08/31/2010 02:24 PM, Diego Novillo wrote:
> On 10-08-31 06:33 , Andi Kleen wrote:
>
>> @@ -2108,6 +2108,13 @@ to work around this, by choosing
>> @code{BOOT_CFLAGS} to avoid the parts
>> of the stage1 compiler that were miscompiled, or by using @samp{make
>> bootstrap4} to increase the number of stages of bootstrap.
>>
>> +You can also use @code{BOOT_CFLAGS} to force an LTO bootstrap with
>> +@samp{BOOT_CFLAGS='-O2 -fwhopr=jobserver -fuse-linker-plugin
>> -frandom-seed=1'}.
>> +Configure the compiler with @samp{--enable-stage1-languages=c,lto}. This
>> +will only work with the @command{gold} linker and if linker plugins are
>> +correctly configured. The @option{-frandom-seed=1} is needed to avoid
>> +bootstrap comparison failures.
>
> Could you add an explanation on why frandom-seed avoids the comparison
> failures? It's not immediately obvious.
>
> OK with that change.

No, config/bootstrap-lto.mk should be adjusted instead to use the 
options that Andi specifies.  The doc change should be limited to the 
--enable-stage1-languages=c,lto part.

Paolo

Re: [PATCH 1/2] Fix LTO bootstrap documentation

[Andi Kleen]

> No, config/bootstrap-lto.mk should be adjusted instead to use the 
> options that Andi specifies.  The doc change should be limited to the 
> --enable-stage1-languages=c,lto part.

The problem is that config/bootstrap-lto.mk doesn't work in the first place
(and I don't know how to fix it). It never built for me.
That is why I documented an alternative method.

-Andi

Re: [PATCH 1/2] Fix LTO bootstrap documentation

[Paolo Bonzini]

On 08/31/2010 03:14 PM, Andi Kleen wrote:
>
>> No, config/bootstrap-lto.mk should be adjusted instead to use the
>> options that Andi specifies. The doc change should be limited to the
>> --enable-stage1-languages=c,lto part.
>
> The problem is that config/bootstrap-lto.mk doesn't work in the first place
> (and I don't know how to fix it). It never built for me.
> That is why I documented an alternative method.

Have you tried s/-flto/flags-you-proposed-to-use-instead/ in that file?

Paolo

Re: [PATCH 1/2] Fix LTO bootstrap documentation

[Alexandre Oliva]

On Aug 31, 2010, Andi Kleen <ak@linux.intel.com> wrote:

>> No, config/bootstrap-lto.mk should be adjusted instead to use the
>> options that Andi specifies.  The doc change should be limited to
>> the --enable-stage1-languages=c,lto part.

> The problem is that config/bootstrap-lto.mk doesn't work in the first place
> (and I don't know how to fix it). It never built for me.

Somehow it worked for me.  That said, I had lto in
--enable-stage1-languages, but that was a mistake: it caused me to
forget to adjust bootstrap-lto.mk to add lto to stage1 languages itself,
which it should.

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

Re: [PATCH 1/2] Fix LTO bootstrap documentation

[Andi Kleen]

On Tue, Aug 31, 2010 at 03:17:56PM +0200, Paolo Bonzini wrote:
> On 08/31/2010 03:14 PM, Andi Kleen wrote:
> >
> >>No, config/bootstrap-lto.mk should be adjusted instead to use the
> >>options that Andi specifies. The doc change should be limited to the
> >>--enable-stage1-languages=c,lto part.
> >
> >The problem is that config/bootstrap-lto.mk doesn't work in the first place
> >(and I don't know how to fix it). It never built for me.
> >That is why I documented an alternative method.
> 
> Have you tried s/-flto/flags-you-proposed-to-use-instead/ in that file?

Hi Paolo,

I reran the test with BUILD_CONFIG and I cannot reproduce the earlier
failures I had (I had multiple earlier, always in the early built
of stage2). I added -frandom-seed=1 and -fuse-linker-plugin.

It might be related to -j* races. I had to drop down my -j
to small values because I found that without using -fwhopr it 
is unusable slow because multiple lto1s will make any reasonable machine swap.

Honza also had some issue with it maybe he can elaborate.

With the old failures not reproducible I guess the documentation
change is obsolete (unless I can reproduce this again) 
and it would be reasonable to simply change 
the build option to this:

Comments?
-Andi


diff --git a/config/bootstrap-lto.mk b/config/bootstrap-lto.mk
index 14099a0..785d15f 100644
--- a/config/bootstrap-lto.mk
+++ b/config/bootstrap-lto.mk
@@ -1,8 +1,8 @@
 # This option enables LTO for stage2 and stage3.  It requires lto to
 # be enabled for stage1 with --enable-stage1-languages.
 
-STAGE2_CFLAGS += -flto
-STAGE3_CFLAGS += -flto
+STAGE2_CFLAGS += -fwhopr=jobserver -fuse-linker-plugin -frandom-seed=1
+STAGE3_CFLAGS += -fwhopr=jobserver -fuse-linker-plugin -frandom-seed=1
 
 # Ada fails to build with LTO, turn it off for now.
 BOOT_ADAFLAGS += -fno-lto


-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH 1/2] Fix LTO bootstrap documentation

[Paolo Bonzini]

On 09/01/2010 09:57 AM, Andi Kleen wrote:
> With the old failures not reproducible I guess the documentation
> change is obsolete (unless I can reproduce this again)
> and it would be reasonable to simply change
> the build option to this:
>
> diff --git a/config/bootstrap-lto.mk b/config/bootstrap-lto.mk
> index 14099a0..785d15f 100644
> --- a/config/bootstrap-lto.mk
> +++ b/config/bootstrap-lto.mk
> @@ -1,8 +1,8 @@
>   # This option enables LTO for stage2 and stage3.  It requires lto to
>   # be enabled for stage1 with --enable-stage1-languages.
>
> -STAGE2_CFLAGS += -flto
> -STAGE3_CFLAGS += -flto
> +STAGE2_CFLAGS += -fwhopr=jobserver -fuse-linker-plugin -frandom-seed=1
> +STAGE3_CFLAGS += -fwhopr=jobserver -fuse-linker-plugin -frandom-seed=1
>
>   # Ada fails to build with LTO, turn it off for now.
>   BOOT_ADAFLAGS += -fno-lto

Yes, this is okay when your patch 2 goes in.  Thanks!

Paolo

Re: [PATCH 1/2] Fix LTO bootstrap documentation

[Andi Kleen]

Paolo Bonzini <bonzini@gnu.org> writes:

> On 09/01/2010 09:57 AM, Andi Kleen wrote:
>> With the old failures not reproducible I guess the documentation
>> change is obsolete (unless I can reproduce this again)
>> and it would be reasonable to simply change
>> the build option to this:
>>
>> diff --git a/config/bootstrap-lto.mk b/config/bootstrap-lto.mk
>> index 14099a0..785d15f 100644
>> --- a/config/bootstrap-lto.mk
>> +++ b/config/bootstrap-lto.mk
>> @@ -1,8 +1,8 @@
>>   # This option enables LTO for stage2 and stage3.  It requires lto to
>>   # be enabled for stage1 with --enable-stage1-languages.
>>
>> -STAGE2_CFLAGS += -flto
>> -STAGE3_CFLAGS += -flto
>> +STAGE2_CFLAGS += -fwhopr=jobserver -fuse-linker-plugin -frandom-seed=1
>> +STAGE3_CFLAGS += -fwhopr=jobserver -fuse-linker-plugin -frandom-seed=1
>>
>>   # Ada fails to build with LTO, turn it off for now.
>>   BOOT_ADAFLAGS += -fno-lto
>
> Yes, this is okay when your patch 2 goes in.  Thanks!

Patch 2 is already in. I'll commit it then with a suitable changelog
after I reran testing.

Hmm thinking about it should probably fix the documentation
to say that gold is required too.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH 1/2] Fix LTO bootstrap documentation

[Paolo Bonzini]

On 09/01/2010 10:07 AM, Andi Kleen wrote:
> Paolo Bonzini<bonzini@gnu.org>  writes:
>
>> On 09/01/2010 09:57 AM, Andi Kleen wrote:
>>> With the old failures not reproducible I guess the documentation
>>> change is obsolete (unless I can reproduce this again)
>>> and it would be reasonable to simply change
>>> the build option to this:
>>>
>>> diff --git a/config/bootstrap-lto.mk b/config/bootstrap-lto.mk
>>> index 14099a0..785d15f 100644
>>> --- a/config/bootstrap-lto.mk
>>> +++ b/config/bootstrap-lto.mk
>>> @@ -1,8 +1,8 @@
>>>    # This option enables LTO for stage2 and stage3.  It requires lto to
>>>    # be enabled for stage1 with --enable-stage1-languages.
>>>
>>> -STAGE2_CFLAGS += -flto
>>> -STAGE3_CFLAGS += -flto
>>> +STAGE2_CFLAGS += -fwhopr=jobserver -fuse-linker-plugin -frandom-seed=1
>>> +STAGE3_CFLAGS += -fwhopr=jobserver -fuse-linker-plugin -frandom-seed=1
>>>
>>>    # Ada fails to build with LTO, turn it off for now.
>>>    BOOT_ADAFLAGS += -fno-lto
>>
>> Yes, this is okay when your patch 2 goes in.  Thanks!
>
> Patch 2 is already in. I'll commit it then with a suitable changelog
> after I reran testing.
>
> Hmm thinking about it should probably fix the documentation
> to say that gold is required too.

Is -fuse-linker-plugin required for correctness?  Losing the ability to 
bootstrap-lto on Windows is a bit ugly.  We could use makefile magic to 
disable it when gold is not in use.

Paolo

Re: [PATCH 1/2] Fix LTO bootstrap documentation

[Andi Kleen]

> Is -fuse-linker-plugin required for correctness?  Losing the ability

Yes it's needed for the ar files (libbackend.a etc.)

collect2 without linker help doesn't handle ar.

> to bootstrap-lto on Windows is a bit ugly.  We could use makefile
> magic to disable it when gold is not in use.

I doubt it will work. That is at least if you really want
LTO code generation.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH 1/2] Fix LTO bootstrap documentation

[Richard Guenther]

On Wed, Sep 1, 2010 at 10:40 AM, Andi Kleen <andi@firstfloor.org> wrote:
>> Is -fuse-linker-plugin required for correctness?  Losing the ability
>
> Yes it's needed for the ar files (libbackend.a etc.)

But not for correctness.  As long as we do not use -fwhole-program you'll
just not get any link-time optimization for libbackend.a and friends.

> collect2 without linker help doesn't handle ar.
>
>> to bootstrap-lto on Windows is a bit ugly.  We could use makefile
>> magic to disable it when gold is not in use.
>
> I doubt it will work. That is at least if you really want
> LTO code generation.

Disabling libbackend.a generation for LTO bootstrap might be a
working idea.

Richard.

> -Andi
>
> --
> ak@linux.intel.com -- Speaking for myself only.
>

Re: [PATCH 1/2] Fix LTO bootstrap documentation

[Andi Kleen]

> > collect2 without linker help doesn't handle ar.
> >
> >> to bootstrap-lto on Windows is a bit ugly.  We could use makefile
> >> magic to disable it when gold is not in use.
> >
> > I doubt it will work. That is at least if you really want
> > LTO code generation.
> 
> Disabling libbackend.a generation for LTO bootstrap might be a
> working idea.

There are more: libcpp (although I think that doesn't get
the correct options anyways, PR45227) and libiberty

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH 1/2] Fix LTO bootstrap documentation

[Richard Guenther]

On Wed, Sep 1, 2010 at 10:50 AM, Andi Kleen <andi@firstfloor.org> wrote:
>> > collect2 without linker help doesn't handle ar.
>> >
>> >> to bootstrap-lto on Windows is a bit ugly.  We could use makefile
>> >> magic to disable it when gold is not in use.
>> >
>> > I doubt it will work. That is at least if you really want
>> > LTO code generation.
>>
>> Disabling libbackend.a generation for LTO bootstrap might be a
>> working idea.
>
> There are more: libcpp (although I think that doesn't get
> the correct options anyways, PR45227) and libiberty

But they do not call back into gcc, so using -fwhole-program and linking
against them will just work.

Richard,

> -Andi
> --
> ak@linux.intel.com -- Speaking for myself only.
>