From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id 12233389443A for ; Thu, 22 Apr 2021 14:30:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 12233389443A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mliska@suse.cz X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id E4CA0B16C; Thu, 22 Apr 2021 14:30:00 +0000 (UTC) Subject: Re: [PATCH] LTO: fallback to -flto=N if -flto=jobserver does not work. To: Richard Biener Cc: GCC Patches References: <11a45856-44b3-754d-5a42-19a348d0d59d@suse.cz> <8cc12cd9-fe88-f97b-eec2-d47f5342496d@suse.cz> From: =?UTF-8?Q?Martin_Li=c5=a1ka?= Message-ID: <73eaf32c-3776-9345-ec5f-57d43e94ecfe@suse.cz> Date: Thu, 22 Apr 2021 16:30:00 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.9.1 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------F6C02B3CB2B9D6BB8C423A72" Content-Language: en-US X-Spam-Status: No, score=-11.3 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Apr 2021 14:30:04 -0000 This is a multi-part message in MIME format. --------------F6C02B3CB2B9D6BB8C423A72 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit On 4/22/21 2:47 PM, Richard Biener wrote: > On Thu, Apr 22, 2021 at 2:21 PM Martin Liška wrote: >> >> On 4/22/21 1:19 PM, Richard Biener wrote: >>> On Thu, Apr 22, 2021 at 11:02 AM Martin Liška wrote: >>>> >>>> On 4/22/21 10:04 AM, Richard Biener wrote: >>>>> On Wed, Apr 21, 2021 at 3:08 PM 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? >>>>> >>>>> 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 >>>>>> >>>> >> --------------F6C02B3CB2B9D6BB8C423A72 Content-Type: text/x-patch; charset=UTF-8; name="0001-Print-warning-diagnostics-for-flto-issues.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0001-Print-warning-diagnostics-for-flto-issues.patch" >From 7cd89e17c49c027027e2aed1722c0758331ac7ac Mon Sep 17 00:00:00 2001 From: Martin Liska 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 --------------F6C02B3CB2B9D6BB8C423A72--