Patchwork [2/2] -fwhopr=jobserver

login
register
mail settings
Submitter Andi Kleen
Date Aug. 17, 2010, 7 a.m.
Message ID <1282028446-19874-2-git-send-email-andi@firstfloor.org>
Download mbox | patch
Permalink /patch/61861/
State New
Headers show

Comments

Andi Kleen - Aug. 17, 2010, 7 a.m.
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.

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(-)
Jan Hubicka - Aug. 17, 2010, 10:53 a.m.
> 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.
> 
> Ok to commit?
Can't we get this detected by default? (I.e. it would be nice if
this was transparent and GCC just chose proper parallelizm when jobserver
is around)

Honza
Andi Kleen - Aug. 17, 2010, 11:09 a.m.
> Can't we get this detected by default? (I.e. it would be nice if
> this was transparent and GCC just chose proper parallelizm when jobserver
> is around)

I was considering parsing the make --version output to detect GNU make, 
but it seemed
a bit dodgy. If people think that's a good idea I can update the patch.

-Andi
Jan Hubicka - Aug. 17, 2010, 11:13 a.m.
>
>> Can't we get this detected by default? (I.e. it would be nice if
>> this was transparent and GCC just chose proper parallelizm when jobserver
>> is around)
>
> I was considering parsing the make --version output to detect GNU make,  
> but it seemed
> a bit dodgy. If people think that's a good idea I can update the patch.
Hmm, it is somewhat ugly :(
My secret plan is to get rid of -fwhopr option for 4.6 and make -flto default
to hopefully most sane setting - that IMO should be WHOPR doing parallel make
from Make's default with sane partitioning decisions (to reduce time and space
needed to stream out partitions).

So for this reason it would be really nice to have things as transparent
as possible.  I guess GNU Make is not nice enough to drop us some environment
variable or something? :(

Honza
>
> -Andi
Andi Kleen - Aug. 17, 2010, 11:23 a.m.
> So for this reason it would be really nice to have things as transparent
> as possible.  I guess GNU Make is not nice enough to drop us some environment
> variable or something? :(

There are some, but not sure they are unique.

Maybe we can fix make. The other problem currently is that you have to 
mark the rule
with + to force jobserver for the rule, otherwise it only does that when 
it thinks
you're invoking a submake. I was thinking about adding a new option to 
make to force
this for all rules, to avoid hacking makefiles everywhere.

While doing that could also add a new environment variable for this.

But short of hacking make I think my patch is a reasonable improvement, 
won't you agree?

-Andi
Jan Hubicka - Aug. 17, 2010, 12:43 p.m.
>
>> So for this reason it would be really nice to have things as transparent
>> as possible.  I guess GNU Make is not nice enough to drop us some environment
>> variable or something? :(
>
> There are some, but not sure they are unique.
>
> Maybe we can fix make. The other problem currently is that you have to  
> mark the rule
> with + to force jobserver for the rule, otherwise it only does that when  
> it thinks
> you're invoking a submake. I was thinking about adding a new option to  
> make to force
> this for all rules, to avoid hacking makefiles everywhere.
>
> While doing that could also add a new environment variable for this.
>
> But short of hacking make I think my patch is a reasonable improvement,  
> won't you agree?

Sure.  I guess we won't avoid some GNU make changes then that will be hard :(

Honzz
>
> -Andi
>
Gerald Pfeifer - Aug. 29, 2010, 11:54 p.m.
On Tue, 17 Aug 2010, Andi Kleen wrote:
> +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.

Please use "GCC" for the generic reference.  The Makefile then might
have @command{gcc}, @command{g++}, @command{gfortran},... of course.

I can't approve the general patch, just this small note on the 
documentation side.

Gerald

Patch

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..2c6e66a 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: