On 8/2/19 11:55 AM, Richard Biener wrote:
> On Fri, Aug 2, 2019 at 11:19 AM Martin Liška <mli...@suse.cz> wrote:
>>
>> On 8/2/19 11:15 AM, Jan Hubicka wrote:
>>>> On Fri, Aug 2, 2019 at 10:50 AM Jakub Jelinek <ja...@redhat.com> wrote:
>>>>>
>>>>> On Fri, Aug 02, 2019 at 10:47:10AM +0200, Martin Liška wrote:
>>>>>>> Can you strace if other fds are opened and not closed in the spot you 
>>>>>>> had it
>>>>>>> before?  Advantage of doing it there is that it will not be done for 
>>>>>>> all the
>>>>>>> -E/-S/-c compilations when the linker is not spawned.
>>>>>>
>>>>>> I've used the same trick which you used and I'm attaching the output.
>>>>>> I believe it's fine, I can't see any opened fd by GCC.
>>>>>
>>>>> LGTM.
>>>>
>>
>> Hi.
>>
>>>> Btw, we discussed yesterday on the phone and the conclusion was to
>>>> make -flto auto-detect a job-server (but not fall back to # of threads)
>>>> and add -flto=auto to auto-detect a job-server and fall back to # of 
>>>> threads.
>>>> That basically makes -flto=jobserver the default behavior which means
>>>> we should document -flto=1 as a way to override jobserver detection.
>>>
>>> And concerning to the yesterday discussion, my preference would still be
>>> -flto to first try presence of jobserver and default to number of
>>> threads otherwise. It seems like user friendly default to me and other
>>> tools with reasonable parallelism support usually behaves this way, too.
>>
>> I also like the default as Honza defined.
>>
>>>
>>> Sure one can use -flto=auto everywhere but then one needs to use
>>> different options for differnt compilers.  It would be nice to make
>>> -flto to do right hting for majority of users including those like me
>>> who cut&paste command lines from Make output and execute them by hand.
>>
>> Note that our ambition is to ideally backport the patches to our gcc9 
>> package.
>> The auto-detection of job-server with -flto is a behavior change that will 
>> happen
>> in the gcc9 package.
>>
>> To be honest I don't like the invention of 'auto' value for -flto command.
>> That would be useful just for gcc9 right now :/
> 
> Well.  We teached people they need to use -flto=N or -flto=jobserver to
> get parallelism.  Like we teached people they need to use -O2 to get
> any optimization.
> 
> Other compilers behave differently.  So what.
> 
> Auto-detecting threads is nice.  Suddenly trashing your system
> because you happen to invoke two links in parallel is not.
> Which is at least why changing this kind of behavior for 9.2 (or 9.3)
> vs. 9.1 is out of the question.  And I'm not sure it is OK for 10.
> 
> As with defaulting to use a job-server when present that's more
> reasonable.
> 
> For cut&pasting you then can use -flto=auto.
> 
> But it's just my opinion - I surely can adapt.

Hi.

Based on what was written, I feel OK with -flto=auto. I would
like to see jobserver detection to be only enabled with the 'auto'
option value. That will make it consistent in GCC 9.x branch.

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

Ready to be installed?
Thanks,
Martin

> 
> Richard.
> 
>> Martin
>>
>>>
>>> The fork bombing is IMO relatively rare and it was not what Jakub ran
>>> into (it was broken jobserv detection).
>>> After all whole distro built with Martin's orriginal approach of
>>> -flto=<nthreads> which forkbombs on every possible occasion.
>>>
>>> Said that, I could live with more conservative defaults.
>>> Honza
>>>
>>

>From 41f13e1890cc78292afb36a54982b097a3c6abdf Mon Sep 17 00:00:00 2001
From: Martin Liska <mli...@suse.cz>
Date: Mon, 5 Aug 2019 06:44:25 +0200
Subject: [PATCH] Add -flto=auto option value.

gcc/ChangeLog:

2019-08-05  Martin Liska  <mli...@suse.cz>

	* doc/invoke.texi: Document the option value.
	* lto-wrapper.c (run_gcc): Set auto_parallel
	only with -flto=auto.

gcc/testsuite/ChangeLog:

2019-08-05  Martin Liska  <mli...@suse.cz>

	* g++.dg/lto/devirt-19_0.C: Add -flto=auto.
---
 gcc/doc/invoke.texi                    |  8 ++++++--
 gcc/lto-wrapper.c                      | 18 ++++++++----------
 gcc/testsuite/g++.dg/lto/devirt-19_0.C |  2 +-
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 7b3c77b8033..5ea56fbb23a 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -10396,8 +10396,7 @@ If you specify the optional @var{n}, the optimization and code
 generation done at link time is executed in 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.  The default value for @var{n} is automatically detected based
-on number of cores.
+used.
 
 You can also specify @option{-flto=jobserver} to use GNU make's
 job server mode to determine the number of parallel jobs. This
@@ -10406,6 +10405,11 @@ You must prepend a @samp{+} to the command recipe in the parent Makefile
 for this to work.  This option likely only works if @env{MAKE} is
 GNU make.
 
+One can also use @option{-flto=auto} to either use GNU make's
+job server mode to determine the number of parallel jobs, if available.
+Or the default value for @var{n} is automatically detected based
+on number of cores.
+
 @item -flto-partition=@var{alg}
 @opindex flto-partition
 Specify the partitioning algorithm used by the link-time optimizer.
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index 3414adedd26..b825e0a0aa0 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -1252,8 +1252,7 @@ run_gcc (unsigned argc, char *argv[])
   char *list_option_full = NULL;
   const char *linker_output = NULL;
   const char *collect_gcc, *collect_gcc_options;
-  /* Make linking parallel by default.  */
-  int parallel = 1;
+  int parallel = 0;
   int jobserver = 0;
   int auto_parallel = 0;
   bool no_partition = false;
@@ -1380,6 +1379,11 @@ run_gcc (unsigned argc, char *argv[])
 	case OPT_flto_:
 	  if (strcmp (option->arg, "jobserver") == 0)
 	    jobserver = 1;
+	  else if (strcmp (option->arg, "auto") == 0)
+	    {
+	      parallel = 1;
+	      auto_parallel = 1;
+	    }
 	  else
 	    {
 	      parallel = atoi (option->arg);
@@ -1423,14 +1427,8 @@ run_gcc (unsigned argc, char *argv[])
       auto_parallel = 0;
       parallel = 0;
     }
-  else if (!jobserver && parallel)
-    {
-      /* If there's no explicit usage of jobserver and
-	 parallel is enabled, then automatically detect
-	 jobserver or number of cores.  */
-      auto_parallel = 1;
-      jobserver = jobserver_active_p ();
-    }
+  else if (!jobserver && auto_parallel)
+    jobserver = jobserver_active_p ();
 
   if (linker_output)
     {
diff --git a/gcc/testsuite/g++.dg/lto/devirt-19_0.C b/gcc/testsuite/g++.dg/lto/devirt-19_0.C
index 696d8c0fc83..b43527e324e 100644
--- a/gcc/testsuite/g++.dg/lto/devirt-19_0.C
+++ b/gcc/testsuite/g++.dg/lto/devirt-19_0.C
@@ -1,5 +1,5 @@
 /* { dg-lto-do link } */
 /* { dg-lto-options { "-O2 -fdump-ipa-cp -Wno-return-type -flto -r -nostdlib" } } */
-/* { dg-extra-ld-options "-flinker-output=nolto-rel" } */
+/* { dg-extra-ld-options "-flinker-output=nolto-rel -flto=auto" } */
 #include "../ipa/devirt-19.C"
 /* { dg-final { scan-wpa-ipa-dump-times "Discovered a virtual call to a known target" 1 "cp"  } } */
-- 
2.22.0

Reply via email to