Re: [PATCH] LTO: fallback to -flto=N if -flto=jobserver does not work.

["Martin Liška" <mliska@suse.cz>]

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