public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] LTO: fallback to -flto=N if -flto=jobserver does not work.
@ 2021-04-21 11:50 Martin Liška
  2021-04-21 15:11 ` Jeff Law
  2021-04-22  8:04 ` Richard Biener
  0 siblings, 2 replies; 10+ messages in thread
From: Martin Liška @ 2021-04-21 11:50 UTC (permalink / raw)
  To: gcc-patches

When -flto=jobserver is used and we cannot detect job server, then we can
still fallbackto -flto=N mode.

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

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

	* lto-wrapper.c (run_gcc): When -flto=jobserver is used, but the
	makeserver cannot be detected, then use -flto=N fallback.
---
 gcc/lto-wrapper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index 03a5922f8ea..0b626d7c811 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -1585,8 +1585,9 @@ run_gcc (unsigned argc, char *argv[])
       if (jobserver && jobserver_error != NULL)
 	{
 	  warning (0, jobserver_error);
-	  parallel = 0;
+	  /* Fall back to auto parallelism.  */
 	  jobserver = 0;
+	  auto_parallel = 1;
 	}
       else if (!jobserver && jobserver_error == NULL)
 	{
-- 
2.31.1


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

* Re: [PATCH] LTO: fallback to -flto=N if -flto=jobserver does not work.
  2021-04-21 11:50 [PATCH] LTO: fallback to -flto=N if -flto=jobserver does not work Martin Liška
@ 2021-04-21 15:11 ` Jeff Law
  2021-04-22  8:04 ` Richard Biener
  1 sibling, 0 replies; 10+ messages in thread
From: Jeff Law @ 2021-04-21 15:11 UTC (permalink / raw)
  To: Martin Liška, gcc-patches


On 4/21/2021 5:50 AM, Martin Liška wrote:
> When -flto=jobserver is used and we cannot detect job server, then we can
> still fallbackto -flto=N mode.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?
> Thanks,
> Martin
>
> gcc/ChangeLog:
>
> 	* lto-wrapper.c (run_gcc): When -flto=jobserver is used, but the
> 	makeserver cannot be detected, then use -flto=N fallback.

OK

jeff



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

* Re: [PATCH] LTO: fallback to -flto=N if -flto=jobserver does not work.
  2021-04-21 11:50 [PATCH] LTO: fallback to -flto=N if -flto=jobserver does not work Martin Liška
  2021-04-21 15:11 ` Jeff Law
@ 2021-04-22  8:04 ` Richard Biener
  2021-04-22  9:02   ` Martin Liška
  1 sibling, 1 reply; 10+ messages in thread
From: Richard Biener @ 2021-04-22  8:04 UTC (permalink / raw)
  To: Martin Liška; +Cc: GCC Patches

On Wed, Apr 21, 2021 at 3:08 PM Martin Liška <mliska@suse.cz> wrote:
>
> When -flto=jobserver is used and we cannot detect job server, then we can
> still fallbackto -flto=N mode.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?

I think this behavior needs to be documented - it falls back to a less
conservative (possibly system overloading) mode - which IMHO is
non-obvious and IMHO we shouldn't do.

Richard.

> Thanks,
> Martin
>
> gcc/ChangeLog:
>
>         * lto-wrapper.c (run_gcc): When -flto=jobserver is used, but the
>         makeserver cannot be detected, then use -flto=N fallback.
> ---
>  gcc/lto-wrapper.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
> index 03a5922f8ea..0b626d7c811 100644
> --- a/gcc/lto-wrapper.c
> +++ b/gcc/lto-wrapper.c
> @@ -1585,8 +1585,9 @@ run_gcc (unsigned argc, char *argv[])
>        if (jobserver && jobserver_error != NULL)
>         {
>           warning (0, jobserver_error);
> -         parallel = 0;
> +         /* Fall back to auto parallelism.  */
>           jobserver = 0;
> +         auto_parallel = 1;
>         }
>        else if (!jobserver && jobserver_error == NULL)
>         {
> --
> 2.31.1
>

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

* Re: [PATCH] LTO: fallback to -flto=N if -flto=jobserver does not work.
  2021-04-22  8:04 ` Richard Biener
@ 2021-04-22  9:02   ` Martin Liška
  2021-04-22 11:19     ` Richard Biener
  0 siblings, 1 reply; 10+ messages in thread
From: Martin Liška @ 2021-04-22  9:02 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

[-- Attachment #1: Type: text/plain, Size: 1535 bytes --]

On 4/22/21 10:04 AM, Richard Biener wrote:
> On Wed, Apr 21, 2021 at 3:08 PM Martin Liška <mliska@suse.cz> wrote:
>>
>> When -flto=jobserver is used and we cannot detect job server, then we can
>> still fallbackto -flto=N mode.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>
>> Ready to be installed?
> 
> I think this behavior needs to be documented - it falls back to a less
> conservative (possibly system overloading) mode - which IMHO is
> non-obvious and IMHO we shouldn't do.

Sure, I'm sending corresponding patch. Note that it's quite common mistake
that '+' is missing in Makefile rule. That was motivation for my change.

Martin

> 
> Richard.
> 
>> Thanks,
>> Martin
>>
>> gcc/ChangeLog:
>>
>>         * lto-wrapper.c (run_gcc): When -flto=jobserver is used, but the
>>         makeserver cannot be detected, then use -flto=N fallback.
>> ---
>>  gcc/lto-wrapper.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
>> index 03a5922f8ea..0b626d7c811 100644
>> --- a/gcc/lto-wrapper.c
>> +++ b/gcc/lto-wrapper.c
>> @@ -1585,8 +1585,9 @@ run_gcc (unsigned argc, char *argv[])
>>        if (jobserver && jobserver_error != NULL)
>>         {
>>           warning (0, jobserver_error);
>> -         parallel = 0;
>> +         /* Fall back to auto parallelism.  */
>>           jobserver = 0;
>> +         auto_parallel = 1;
>>         }
>>        else if (!jobserver && jobserver_error == NULL)
>>         {
>> --
>> 2.31.1
>>


[-- Attachment #2: 0001-Document-flto-jobserver-fallback.patch --]
[-- Type: text/x-patch, Size: 1017 bytes --]

From b81f5288fc81256e63d44e00793e2732eb2a2d88 Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Thu, 22 Apr 2021 10:59:51 +0200
Subject: [PATCH] Document -flto=jobserver fallback.

gcc/ChangeLog:

	* doc/invoke.texi: Document what happens when GCC can't detect
	make's job server with -flto=jobserver.
---
 gcc/doc/invoke.texi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index e98b0962b9f..1cfff4c14a9 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -12340,6 +12340,9 @@ Use @option{-flto=auto} to use GNU make's job server, if available,
 or otherwise fall back to autodetection of the number of CPU threads
 present in your system.
 
+If you specify @option{-flto=jobserver} and make's job server can't be detected,
+then well fall back to the number of CPU threads present in your system.
+
 @item -flto-partition=@var{alg}
 @opindex flto-partition
 Specify the partitioning algorithm used by the link-time optimizer.
-- 
2.31.1


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

* Re: [PATCH] LTO: fallback to -flto=N if -flto=jobserver does not work.
  2021-04-22  9:02   ` Martin Liška
@ 2021-04-22 11:19     ` Richard Biener
  2021-04-22 12:21       ` Martin Liška
  0 siblings, 1 reply; 10+ messages in thread
From: Richard Biener @ 2021-04-22 11:19 UTC (permalink / raw)
  To: Martin Liška; +Cc: GCC Patches

On Thu, Apr 22, 2021 at 11:02 AM Martin Liška <mliska@suse.cz> wrote:
>
> On 4/22/21 10:04 AM, Richard Biener wrote:
> > On Wed, Apr 21, 2021 at 3:08 PM Martin Liška <mliska@suse.cz> wrote:
> >>
> >> When -flto=jobserver is used and we cannot detect job server, then we can
> >> still fallbackto -flto=N mode.
> >>
> >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> >>
> >> Ready to be installed?
> >
> > I think this behavior needs to be documented - it falls back to a less
> > conservative (possibly system overloading) mode - which IMHO is
> > non-obvious and IMHO we shouldn't do.
>
> Sure, I'm sending corresponding patch. Note that it's quite common mistake
> that '+' is missing in Makefile rule. That was motivation for my change.

Sure, but that change won't get this fixed.  IMHO we should eventually
emit diagnostic like

warning: could not find jobserver, compiling N jobs serially

once N > 1 (or 2?).  Likewise if people just use -flto and auto-detection
finds nothing:

warning: using serial compilation of N LTRANS jobs
note: refer to http://.... for how to use parallel compile

using the URL diagnostics to point to -flto=... documentation.

That is, teach users rather than second-guessing and eventually
blowing things up.  IMHO only the jobserver mode is safe to
automatically use.

Richard.

> Martin
>
> >
> > Richard.
> >
> >> Thanks,
> >> Martin
> >>
> >> gcc/ChangeLog:
> >>
> >>         * lto-wrapper.c (run_gcc): When -flto=jobserver is used, but the
> >>         makeserver cannot be detected, then use -flto=N fallback.
> >> ---
> >>  gcc/lto-wrapper.c | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
> >> index 03a5922f8ea..0b626d7c811 100644
> >> --- a/gcc/lto-wrapper.c
> >> +++ b/gcc/lto-wrapper.c
> >> @@ -1585,8 +1585,9 @@ run_gcc (unsigned argc, char *argv[])
> >>        if (jobserver && jobserver_error != NULL)
> >>         {
> >>           warning (0, jobserver_error);
> >> -         parallel = 0;
> >> +         /* Fall back to auto parallelism.  */
> >>           jobserver = 0;
> >> +         auto_parallel = 1;
> >>         }
> >>        else if (!jobserver && jobserver_error == NULL)
> >>         {
> >> --
> >> 2.31.1
> >>
>

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

* Re: [PATCH] LTO: fallback to -flto=N if -flto=jobserver does not work.
  2021-04-22 11:19     ` Richard Biener
@ 2021-04-22 12:21       ` Martin Liška
  2021-04-22 12:47         ` Richard Biener
  0 siblings, 1 reply; 10+ messages in thread
From: Martin Liška @ 2021-04-22 12:21 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

On 4/22/21 1:19 PM, Richard Biener wrote:
> On Thu, Apr 22, 2021 at 11:02 AM Martin Liška <mliska@suse.cz> wrote:
>>
>> On 4/22/21 10:04 AM, Richard Biener wrote:
>>> On Wed, Apr 21, 2021 at 3:08 PM Martin Liška <mliska@suse.cz> wrote:
>>>>
>>>> When -flto=jobserver is used and we cannot detect job server, then we can
>>>> still fallbackto -flto=N mode.
>>>>
>>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>>>
>>>> Ready to be installed?
>>>
>>> I think this behavior needs to be documented - it falls back to a less
>>> conservative (possibly system overloading) mode - which IMHO is
>>> non-obvious and IMHO we shouldn't do.
>>
>> Sure, I'm sending corresponding patch. Note that it's quite common mistake
>> that '+' is missing in Makefile rule. That was motivation for my change.
> 
> Sure, but that change won't get this fixed.

It will as linker command line will contain (-flto=jobserver) and LTO will fallback to -flto=N.

> IMHO we should eventually
> emit diagnostic like
> 
> warning: could not find jobserver, compiling N jobs serially
> 
> once N > 1 (or 2?).

We do that now (for all N):
lto-wrapper: warning: jobserver is not available: ‘MAKEFLAGS’ environment variable is unset


> Likewise if people just use -flto and auto-detection
> finds nothing:

-flto != -flto=auto

Yes, -flto is a serial linking and we can emit a warning.

> 
> warning: using serial compilation of N LTRANS jobs
> note: refer to http://.... for how to use parallel compile
> 
> using the URL diagnostics to point to -flto=... documentation.

What about making that a proper warning (-Wlto)? We have diagnostics infrastructure
that prints URL links.

> 
> That is, teach users rather than second-guessing and eventually
> blowing things up.  IMHO only the jobserver mode is safe to
> automatically use.

Well, -flto=auto is also fine and document. I think there is no possibility
auto CPU deduction can fail. So -flto=jobserver (with missing make job server)
and -flto (equal to -flto=1) worth emitting a warning.

What do you think?
Martin

> 
> Richard.
> 
>> Martin
>>
>>>
>>> Richard.
>>>
>>>> Thanks,
>>>> Martin
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>>         * lto-wrapper.c (run_gcc): When -flto=jobserver is used, but the
>>>>         makeserver cannot be detected, then use -flto=N fallback.
>>>> ---
>>>>  gcc/lto-wrapper.c | 3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
>>>> index 03a5922f8ea..0b626d7c811 100644
>>>> --- a/gcc/lto-wrapper.c
>>>> +++ b/gcc/lto-wrapper.c
>>>> @@ -1585,8 +1585,9 @@ run_gcc (unsigned argc, char *argv[])
>>>>        if (jobserver && jobserver_error != NULL)
>>>>         {
>>>>           warning (0, jobserver_error);
>>>> -         parallel = 0;
>>>> +         /* Fall back to auto parallelism.  */
>>>>           jobserver = 0;
>>>> +         auto_parallel = 1;
>>>>         }
>>>>        else if (!jobserver && jobserver_error == NULL)
>>>>         {
>>>> --
>>>> 2.31.1
>>>>
>>


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

* Re: [PATCH] LTO: fallback to -flto=N if -flto=jobserver does not work.
  2021-04-22 12:21       ` Martin Liška
@ 2021-04-22 12:47         ` Richard Biener
  2021-04-22 14:30           ` Martin Liška
  0 siblings, 1 reply; 10+ messages in thread
From: Richard Biener @ 2021-04-22 12:47 UTC (permalink / raw)
  To: Martin Liška; +Cc: GCC Patches

On Thu, Apr 22, 2021 at 2:21 PM Martin Liška <mliska@suse.cz> wrote:
>
> On 4/22/21 1:19 PM, Richard Biener wrote:
> > On Thu, Apr 22, 2021 at 11:02 AM Martin Liška <mliska@suse.cz> wrote:
> >>
> >> On 4/22/21 10:04 AM, Richard Biener wrote:
> >>> On Wed, Apr 21, 2021 at 3:08 PM Martin Liška <mliska@suse.cz> wrote:
> >>>>
> >>>> When -flto=jobserver is used and we cannot detect job server, then we can
> >>>> still fallbackto -flto=N mode.
> >>>>
> >>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> >>>>
> >>>> Ready to be installed?
> >>>
> >>> I think this behavior needs to be documented - it falls back to a less
> >>> conservative (possibly system overloading) mode - which IMHO is
> >>> non-obvious and IMHO we shouldn't do.
> >>
> >> Sure, I'm sending corresponding patch. Note that it's quite common mistake
> >> that '+' is missing in Makefile rule. That was motivation for my change.
> >
> > Sure, but that change won't get this fixed.
>
> It will as linker command line will contain (-flto=jobserver) and LTO will fallback to -flto=N.
>
> > IMHO we should eventually
> > emit diagnostic like
> >
> > warning: could not find jobserver, compiling N jobs serially
> >
> > once N > 1 (or 2?).
>
> We do that now (for all N):
> lto-wrapper: warning: jobserver is not available: ‘MAKEFLAGS’ environment variable is unset
>
>
> > Likewise if people just use -flto and auto-detection
> > finds nothing:
>
> -flto != -flto=auto
>
> Yes, -flto is a serial linking and we can emit a warning.

I'd avoid warning if there's just a single ltrans unit.

> > warning: using serial compilation of N LTRANS jobs
> > note: refer to http://.... for how to use parallel compile
> >
> > using the URL diagnostics to point to -flto=... documentation.
>
> What about making that a proper warning (-Wlto)? We have diagnostics infrastructure
> that prints URL links.

Note that drivers like lto-wrapper do not have fully initialized diagnostic
machinery and use a "different" set of overloads (likewise gen* programs).

> >
> > That is, teach users rather than second-guessing and eventually
> > blowing things up.  IMHO only the jobserver mode is safe to
> > automatically use.
>
> Well, -flto=auto is also fine and document. I think there is no possibility
> auto CPU deduction can fail. So -flto=jobserver (with missing make job server)
> and -flto (equal to -flto=1) worth emitting a warning.
>
> What do you think?

Yes, that sounds reasonable.  I suspect that people might want to see
-flto default to -flto=auto but then I don't think that's good because there's
no system wide job scheduler limiting things (systemd-jobserver anyone?)

Richard.

> Martin
>
> >
> > Richard.
> >
> >> Martin
> >>
> >>>
> >>> Richard.
> >>>
> >>>> Thanks,
> >>>> Martin
> >>>>
> >>>> gcc/ChangeLog:
> >>>>
> >>>>         * lto-wrapper.c (run_gcc): When -flto=jobserver is used, but the
> >>>>         makeserver cannot be detected, then use -flto=N fallback.
> >>>> ---
> >>>>  gcc/lto-wrapper.c | 3 ++-
> >>>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
> >>>> index 03a5922f8ea..0b626d7c811 100644
> >>>> --- a/gcc/lto-wrapper.c
> >>>> +++ b/gcc/lto-wrapper.c
> >>>> @@ -1585,8 +1585,9 @@ run_gcc (unsigned argc, char *argv[])
> >>>>        if (jobserver && jobserver_error != NULL)
> >>>>         {
> >>>>           warning (0, jobserver_error);
> >>>> -         parallel = 0;
> >>>> +         /* Fall back to auto parallelism.  */
> >>>>           jobserver = 0;
> >>>> +         auto_parallel = 1;
> >>>>         }
> >>>>        else if (!jobserver && jobserver_error == NULL)
> >>>>         {
> >>>> --
> >>>> 2.31.1
> >>>>
> >>
>

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

* Re: [PATCH] LTO: fallback to -flto=N if -flto=jobserver does not work.
  2021-04-22 12:47         ` Richard Biener
@ 2021-04-22 14:30           ` Martin Liška
  2021-05-12  9:10             ` Martin Liška
  0 siblings, 1 reply; 10+ messages in thread
From: Martin Liška @ 2021-04-22 14:30 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

[-- Attachment #1: Type: text/plain, Size: 4397 bytes --]

On 4/22/21 2:47 PM, Richard Biener wrote:
> On Thu, Apr 22, 2021 at 2:21 PM Martin Liška <mliska@suse.cz> wrote:
>>
>> On 4/22/21 1:19 PM, Richard Biener wrote:
>>> On Thu, Apr 22, 2021 at 11:02 AM Martin Liška <mliska@suse.cz> wrote:
>>>>
>>>> On 4/22/21 10:04 AM, Richard Biener wrote:
>>>>> On Wed, Apr 21, 2021 at 3:08 PM Martin Liška <mliska@suse.cz> wrote:
>>>>>>
>>>>>> When -flto=jobserver is used and we cannot detect job server, then we can
>>>>>> still fallbackto -flto=N mode.
>>>>>>
>>>>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>>>>>
>>>>>> Ready to be installed?
>>>>>
>>>>> I think this behavior needs to be documented - it falls back to a less
>>>>> conservative (possibly system overloading) mode - which IMHO is
>>>>> non-obvious and IMHO we shouldn't do.
>>>>
>>>> Sure, I'm sending corresponding patch. Note that it's quite common mistake
>>>> that '+' is missing in Makefile rule. That was motivation for my change.
>>>
>>> Sure, but that change won't get this fixed.
>>
>> It will as linker command line will contain (-flto=jobserver) and LTO will fallback to -flto=N.
>>
>>> IMHO we should eventually
>>> emit diagnostic like
>>>
>>> warning: could not find jobserver, compiling N jobs serially
>>>
>>> once N > 1 (or 2?).
>>
>> We do that now (for all N):
>> lto-wrapper: warning: jobserver is not available: ‘MAKEFLAGS’ environment variable is unset
>>
>>
>>> Likewise if people just use -flto and auto-detection
>>> finds nothing:
>>
>> -flto != -flto=auto
>>
>> Yes, -flto is a serial linking and we can emit a warning.
> 
> I'd avoid warning if there's just a single ltrans unit.

That's doable and I've just done that in the attached patch.
Two disadvantages:
- one needs waiting for the warning after WPA
- source change (>1 LTRANS) can trigger the warning

> 
>>> warning: using serial compilation of N LTRANS jobs
>>> note: refer to http://.... for how to use parallel compile
>>>
>>> using the URL diagnostics to point to -flto=... documentation.
>>
>> What about making that a proper warning (-Wlto)? We have diagnostics infrastructure
>> that prints URL links.
> 
> Note that drivers like lto-wrapper do not have fully initialized diagnostic
> machinery and use a "different" set of overloads (likewise gen* programs).

I managed printing the warning:

lto-wrapper: warning: jobserver is not available: ‘MAKEFLAGS’ environment variable is unset

lto-wrapper: note: see the ‘-flto’ option documentation for more information


and

lto-wrapper: warning: using serial compilation of 128 LTRANS jobs

lto-wrapper: note: see the ‘-flto’ option documentation for more information


> 
>>>
>>> That is, teach users rather than second-guessing and eventually
>>> blowing things up.  IMHO only the jobserver mode is safe to
>>> automatically use.
>>
>> Well, -flto=auto is also fine and document. I think there is no possibility
>> auto CPU deduction can fail. So -flto=jobserver (with missing make job server)
>> and -flto (equal to -flto=1) worth emitting a warning.
>>
>> What do you think?
> 
> Yes, that sounds reasonable.  I suspect that people might want to see
> -flto default to -flto=auto but then I don't think that's good because there's
> no system wide job scheduler limiting things (systemd-jobserver anyone?)

Done that.

Thoughts?
Martin

> 
> Richard.
> 
>> Martin
>>
>>>
>>> Richard.
>>>
>>>> Martin
>>>>
>>>>>
>>>>> Richard.
>>>>>
>>>>>> Thanks,
>>>>>> Martin
>>>>>>
>>>>>> gcc/ChangeLog:
>>>>>>
>>>>>>         * lto-wrapper.c (run_gcc): When -flto=jobserver is used, but the
>>>>>>         makeserver cannot be detected, then use -flto=N fallback.
>>>>>> ---
>>>>>>  gcc/lto-wrapper.c | 3 ++-
>>>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
>>>>>> index 03a5922f8ea..0b626d7c811 100644
>>>>>> --- a/gcc/lto-wrapper.c
>>>>>> +++ b/gcc/lto-wrapper.c
>>>>>> @@ -1585,8 +1585,9 @@ run_gcc (unsigned argc, char *argv[])
>>>>>>        if (jobserver && jobserver_error != NULL)
>>>>>>         {
>>>>>>           warning (0, jobserver_error);
>>>>>> -         parallel = 0;
>>>>>> +         /* Fall back to auto parallelism.  */
>>>>>>           jobserver = 0;
>>>>>> +         auto_parallel = 1;
>>>>>>         }
>>>>>>        else if (!jobserver && jobserver_error == NULL)
>>>>>>         {
>>>>>> --
>>>>>> 2.31.1
>>>>>>
>>>>
>>


[-- Attachment #2: 0001-Print-warning-diagnostics-for-flto-issues.patch --]
[-- Type: text/x-patch, Size: 4042 bytes --]

From 7cd89e17c49c027027e2aed1722c0758331ac7ac Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Thu, 22 Apr 2021 16:27:19 +0200
Subject: [PATCH] Print warning diagnostics for -flto issues.

gcc/ChangeLog:

	* lto-wrapper.c (print_lto_docs_link): New function.
	(run_gcc): Print warning about missing job server detection
	after we know NR of partitions. Do the same for -flto{,=1}.
	* opts.c (get_option_html_page): Support -flto option.
---
 gcc/lto-wrapper.c | 39 +++++++++++++++++++++++++++++++++++++--
 gcc/opts.c        |  6 +++++-
 2 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index 0b626d7c811..f7ca084ef6c 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -48,6 +48,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "simple-object.h"
 #include "lto-section-names.h"
 #include "collect-utils.h"
+#include "opts-diagnostic.h"
 
 /* Environment variable, used for passing the names of offload targets from GCC
    driver to lto-wrapper.  */
@@ -1335,6 +1336,23 @@ jobserver_active_p (void)
     return JS_PREFIX "cannot access %<" JS_NEEDLE "%> file descriptors";
 }
 
+/* Print link to -flto documentation with a hint message.  */
+
+void
+print_lto_docs_link ()
+{
+  const char *url = get_option_url (NULL, OPT_flto);
+
+  pretty_printer pp;
+  pp.url_format = URL_FORMAT_DEFAULT;
+  pp_string (&pp, "see the ");
+  pp_begin_url (&pp, url);
+  pp_string (&pp, "%<-flto%> option documentation");
+  pp_end_url (&pp);
+  pp_string (&pp, " for more information");
+  inform (UNKNOWN_LOCATION, pp_formatted_text (&pp));
+}
+
 /* Test that a make command is present and working, return true if so.  */
 
 static bool
@@ -1369,8 +1387,10 @@ run_gcc (unsigned argc, char *argv[])
   char *collect_gcc_options;
   int parallel = 0;
   int jobserver = 0;
+  bool jobserver_requested = false;
   int auto_parallel = 0;
   bool no_partition = false;
+  const char *jobserver_error = NULL;
   struct cl_decoded_option *fdecoded_options = NULL;
   struct cl_decoded_option *offload_fdecoded_options = NULL;
   unsigned int fdecoded_options_count = 0;
@@ -1522,6 +1542,7 @@ run_gcc (unsigned argc, char *argv[])
 	    {
 	      parallel = 1;
 	      jobserver = 1;
+	      jobserver_requested = true;
 	    }
 	  else if (strcmp (option->arg, "auto") == 0)
 	    {
@@ -1576,15 +1597,15 @@ run_gcc (unsigned argc, char *argv[])
     {
       lto_mode = LTO_MODE_LTO;
       jobserver = 0;
+      jobserver_requested = false;
       auto_parallel = 0;
       parallel = 0;
     }
   else
     {
-      const char *jobserver_error = jobserver_active_p ();
+      jobserver_error = jobserver_active_p ();
       if (jobserver && jobserver_error != NULL)
 	{
-	  warning (0, jobserver_error);
 	  /* Fall back to auto parallelism.  */
 	  jobserver = 0;
 	  auto_parallel = 1;
@@ -1904,6 +1925,20 @@ cont:
       maybe_unlink (ltrans_output_file);
       ltrans_output_file = NULL;
 
+      if (nr > 1)
+	{
+	  if (jobserver_requested && jobserver_error != NULL)
+	    {
+	      warning (0, jobserver_error);
+	      print_lto_docs_link ();
+	    }
+	  else if (parallel == 0)
+	    {
+	      warning (0, "using serial compilation of %d LTRANS jobs", nr);
+	      print_lto_docs_link ();
+	    }
+	}
+
       if (parallel)
 	{
 	  makefile = make_temp_file (".mk");
diff --git a/gcc/opts.c b/gcc/opts.c
index 24bb64198c8..fe6fddbf095 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -3243,9 +3243,13 @@ get_option_html_page (int option_index)
   const cl_option *cl_opt = &cl_options[option_index];
 
   /* Analyzer options are on their own page.  */
-  if (strstr(cl_opt->opt_text, "analyzer-"))
+  if (strstr (cl_opt->opt_text, "analyzer-"))
     return "gcc/Static-Analyzer-Options.html";
 
+  /* Handle -flto= option.  */
+  if (strstr (cl_opt->opt_text, "flto"))
+    return "gcc/Optimize-Options.html";
+
 #ifdef CL_Fortran
   if ((cl_opt->flags & CL_Fortran) != 0
       /* If it is option common to both C/C++ and Fortran, it is documented
-- 
2.31.1


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

* Re: [PATCH] LTO: fallback to -flto=N if -flto=jobserver does not work.
  2021-04-22 14:30           ` Martin Liška
@ 2021-05-12  9:10             ` Martin Liška
  2021-05-12  9:21               ` Richard Biener
  0 siblings, 1 reply; 10+ messages in thread
From: Martin Liška @ 2021-05-12  9:10 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

May I please ping this Richi?

Thanks,
Martin

On 4/22/21 4:30 PM, Martin Liška wrote:
> On 4/22/21 2:47 PM, Richard Biener wrote:
>> On Thu, Apr 22, 2021 at 2:21 PM Martin Liška <mliska@suse.cz> wrote:
>>>
>>> On 4/22/21 1:19 PM, Richard Biener wrote:
>>>> On Thu, Apr 22, 2021 at 11:02 AM Martin Liška <mliska@suse.cz> wrote:
>>>>>
>>>>> On 4/22/21 10:04 AM, Richard Biener wrote:
>>>>>> On Wed, Apr 21, 2021 at 3:08 PM Martin Liška <mliska@suse.cz> wrote:
>>>>>>>
>>>>>>> When -flto=jobserver is used and we cannot detect job server, then we can
>>>>>>> still fallbackto -flto=N mode.
>>>>>>>
>>>>>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>>>>>>
>>>>>>> Ready to be installed?
>>>>>>
>>>>>> I think this behavior needs to be documented - it falls back to a less
>>>>>> conservative (possibly system overloading) mode - which IMHO is
>>>>>> non-obvious and IMHO we shouldn't do.
>>>>>
>>>>> Sure, I'm sending corresponding patch. Note that it's quite common mistake
>>>>> that '+' is missing in Makefile rule. That was motivation for my change.
>>>>
>>>> Sure, but that change won't get this fixed.
>>>
>>> It will as linker command line will contain (-flto=jobserver) and LTO will fallback to -flto=N.
>>>
>>>> IMHO we should eventually
>>>> emit diagnostic like
>>>>
>>>> warning: could not find jobserver, compiling N jobs serially
>>>>
>>>> once N > 1 (or 2?).
>>>
>>> We do that now (for all N):
>>> lto-wrapper: warning: jobserver is not available: ‘MAKEFLAGS’ environment variable is unset
>>>
>>>
>>>> Likewise if people just use -flto and auto-detection
>>>> finds nothing:
>>>
>>> -flto != -flto=auto
>>>
>>> Yes, -flto is a serial linking and we can emit a warning.
>>
>> I'd avoid warning if there's just a single ltrans unit.
> 
> That's doable and I've just done that in the attached patch.
> Two disadvantages:
> - one needs waiting for the warning after WPA
> - source change (>1 LTRANS) can trigger the warning
> 
>>
>>>> warning: using serial compilation of N LTRANS jobs
>>>> note: refer to http://.... for how to use parallel compile
>>>>
>>>> using the URL diagnostics to point to -flto=... documentation.
>>>
>>> What about making that a proper warning (-Wlto)? We have diagnostics infrastructure
>>> that prints URL links.
>>
>> Note that drivers like lto-wrapper do not have fully initialized diagnostic
>> machinery and use a "different" set of overloads (likewise gen* programs).
> 
> I managed printing the warning:
> 
> lto-wrapper: warning: jobserver is not available: ‘MAKEFLAGS’ environment variable is unset
> 
> lto-wrapper: note: see the ‘-flto’ option documentation for more information
> 
> 
> and
> 
> lto-wrapper: warning: using serial compilation of 128 LTRANS jobs
> 
> lto-wrapper: note: see the ‘-flto’ option documentation for more information
> 
> 
>>
>>>>
>>>> That is, teach users rather than second-guessing and eventually
>>>> blowing things up.  IMHO only the jobserver mode is safe to
>>>> automatically use.
>>>
>>> Well, -flto=auto is also fine and document. I think there is no possibility
>>> auto CPU deduction can fail. So -flto=jobserver (with missing make job server)
>>> and -flto (equal to -flto=1) worth emitting a warning.
>>>
>>> What do you think?
>>
>> Yes, that sounds reasonable.  I suspect that people might want to see
>> -flto default to -flto=auto but then I don't think that's good because there's
>> no system wide job scheduler limiting things (systemd-jobserver anyone?)
> 
> Done that.
> 
> Thoughts?
> Martin
> 
>>
>> Richard.
>>
>>> Martin
>>>
>>>>
>>>> Richard.
>>>>
>>>>> Martin
>>>>>
>>>>>>
>>>>>> Richard.
>>>>>>
>>>>>>> Thanks,
>>>>>>> Martin
>>>>>>>
>>>>>>> gcc/ChangeLog:
>>>>>>>
>>>>>>>          * lto-wrapper.c (run_gcc): When -flto=jobserver is used, but the
>>>>>>>          makeserver cannot be detected, then use -flto=N fallback.
>>>>>>> ---
>>>>>>>   gcc/lto-wrapper.c | 3 ++-
>>>>>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
>>>>>>> index 03a5922f8ea..0b626d7c811 100644
>>>>>>> --- a/gcc/lto-wrapper.c
>>>>>>> +++ b/gcc/lto-wrapper.c
>>>>>>> @@ -1585,8 +1585,9 @@ run_gcc (unsigned argc, char *argv[])
>>>>>>>         if (jobserver && jobserver_error != NULL)
>>>>>>>          {
>>>>>>>            warning (0, jobserver_error);
>>>>>>> -         parallel = 0;
>>>>>>> +         /* Fall back to auto parallelism.  */
>>>>>>>            jobserver = 0;
>>>>>>> +         auto_parallel = 1;
>>>>>>>          }
>>>>>>>         else if (!jobserver && jobserver_error == NULL)
>>>>>>>          {
>>>>>>> --
>>>>>>> 2.31.1
>>>>>>>
>>>>>
>>>
> 


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

* Re: [PATCH] LTO: fallback to -flto=N if -flto=jobserver does not work.
  2021-05-12  9:10             ` Martin Liška
@ 2021-05-12  9:21               ` Richard Biener
  0 siblings, 0 replies; 10+ messages in thread
From: Richard Biener @ 2021-05-12  9:21 UTC (permalink / raw)
  To: Martin Liška; +Cc: GCC Patches

On Wed, May 12, 2021 at 11:10 AM Martin Liška <mliska@suse.cz> wrote:
>
> May I please ping this Richi?

OK.

Thanks,
Richard.



> Thanks,
> Martin
>
> On 4/22/21 4:30 PM, Martin Liška wrote:
> > On 4/22/21 2:47 PM, Richard Biener wrote:
> >> On Thu, Apr 22, 2021 at 2:21 PM Martin Liška <mliska@suse.cz> wrote:
> >>>
> >>> On 4/22/21 1:19 PM, Richard Biener wrote:
> >>>> On Thu, Apr 22, 2021 at 11:02 AM Martin Liška <mliska@suse.cz> wrote:
> >>>>>
> >>>>> On 4/22/21 10:04 AM, Richard Biener wrote:
> >>>>>> On Wed, Apr 21, 2021 at 3:08 PM Martin Liška <mliska@suse.cz> wrote:
> >>>>>>>
> >>>>>>> When -flto=jobserver is used and we cannot detect job server, then we can
> >>>>>>> still fallbackto -flto=N mode.
> >>>>>>>
> >>>>>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> >>>>>>>
> >>>>>>> Ready to be installed?
> >>>>>>
> >>>>>> I think this behavior needs to be documented - it falls back to a less
> >>>>>> conservative (possibly system overloading) mode - which IMHO is
> >>>>>> non-obvious and IMHO we shouldn't do.
> >>>>>
> >>>>> Sure, I'm sending corresponding patch. Note that it's quite common mistake
> >>>>> that '+' is missing in Makefile rule. That was motivation for my change.
> >>>>
> >>>> Sure, but that change won't get this fixed.
> >>>
> >>> It will as linker command line will contain (-flto=jobserver) and LTO will fallback to -flto=N.
> >>>
> >>>> IMHO we should eventually
> >>>> emit diagnostic like
> >>>>
> >>>> warning: could not find jobserver, compiling N jobs serially
> >>>>
> >>>> once N > 1 (or 2?).
> >>>
> >>> We do that now (for all N):
> >>> lto-wrapper: warning: jobserver is not available: ‘MAKEFLAGS’ environment variable is unset
> >>>
> >>>
> >>>> Likewise if people just use -flto and auto-detection
> >>>> finds nothing:
> >>>
> >>> -flto != -flto=auto
> >>>
> >>> Yes, -flto is a serial linking and we can emit a warning.
> >>
> >> I'd avoid warning if there's just a single ltrans unit.
> >
> > That's doable and I've just done that in the attached patch.
> > Two disadvantages:
> > - one needs waiting for the warning after WPA
> > - source change (>1 LTRANS) can trigger the warning
> >
> >>
> >>>> warning: using serial compilation of N LTRANS jobs
> >>>> note: refer to http://.... for how to use parallel compile
> >>>>
> >>>> using the URL diagnostics to point to -flto=... documentation.
> >>>
> >>> What about making that a proper warning (-Wlto)? We have diagnostics infrastructure
> >>> that prints URL links.
> >>
> >> Note that drivers like lto-wrapper do not have fully initialized diagnostic
> >> machinery and use a "different" set of overloads (likewise gen* programs).
> >
> > I managed printing the warning:
> >
> > lto-wrapper: warning: jobserver is not available: ‘MAKEFLAGS’ environment variable is unset
> >
> > lto-wrapper: note: see the ‘-flto’ option documentation for more information
> >
> >
> > and
> >
> > lto-wrapper: warning: using serial compilation of 128 LTRANS jobs
> >
> > lto-wrapper: note: see the ‘-flto’ option documentation for more information
> >
> >
> >>
> >>>>
> >>>> That is, teach users rather than second-guessing and eventually
> >>>> blowing things up.  IMHO only the jobserver mode is safe to
> >>>> automatically use.
> >>>
> >>> Well, -flto=auto is also fine and document. I think there is no possibility
> >>> auto CPU deduction can fail. So -flto=jobserver (with missing make job server)
> >>> and -flto (equal to -flto=1) worth emitting a warning.
> >>>
> >>> What do you think?
> >>
> >> Yes, that sounds reasonable.  I suspect that people might want to see
> >> -flto default to -flto=auto but then I don't think that's good because there's
> >> no system wide job scheduler limiting things (systemd-jobserver anyone?)
> >
> > Done that.
> >
> > Thoughts?
> > Martin
> >
> >>
> >> Richard.
> >>
> >>> Martin
> >>>
> >>>>
> >>>> Richard.
> >>>>
> >>>>> Martin
> >>>>>
> >>>>>>
> >>>>>> Richard.
> >>>>>>
> >>>>>>> Thanks,
> >>>>>>> Martin
> >>>>>>>
> >>>>>>> gcc/ChangeLog:
> >>>>>>>
> >>>>>>>          * lto-wrapper.c (run_gcc): When -flto=jobserver is used, but the
> >>>>>>>          makeserver cannot be detected, then use -flto=N fallback.
> >>>>>>> ---
> >>>>>>>   gcc/lto-wrapper.c | 3 ++-
> >>>>>>>   1 file changed, 2 insertions(+), 1 deletion(-)
> >>>>>>>
> >>>>>>> diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
> >>>>>>> index 03a5922f8ea..0b626d7c811 100644
> >>>>>>> --- a/gcc/lto-wrapper.c
> >>>>>>> +++ b/gcc/lto-wrapper.c
> >>>>>>> @@ -1585,8 +1585,9 @@ run_gcc (unsigned argc, char *argv[])
> >>>>>>>         if (jobserver && jobserver_error != NULL)
> >>>>>>>          {
> >>>>>>>            warning (0, jobserver_error);
> >>>>>>> -         parallel = 0;
> >>>>>>> +         /* Fall back to auto parallelism.  */
> >>>>>>>            jobserver = 0;
> >>>>>>> +         auto_parallel = 1;
> >>>>>>>          }
> >>>>>>>         else if (!jobserver && jobserver_error == NULL)
> >>>>>>>          {
> >>>>>>> --
> >>>>>>> 2.31.1
> >>>>>>>
> >>>>>
> >>>
> >
>

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

end of thread, other threads:[~2021-05-12  9:21 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-21 11:50 [PATCH] LTO: fallback to -flto=N if -flto=jobserver does not work Martin Liška
2021-04-21 15:11 ` Jeff Law
2021-04-22  8:04 ` Richard Biener
2021-04-22  9:02   ` Martin Liška
2021-04-22 11:19     ` Richard Biener
2021-04-22 12:21       ` Martin Liška
2021-04-22 12:47         ` Richard Biener
2021-04-22 14:30           ` Martin Liška
2021-05-12  9:10             ` Martin Liška
2021-05-12  9:21               ` Richard Biener

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).