* [PATCH 1/3] Factor out jobserver_active_p. @ 2022-08-09 12:00 Martin Liška 2022-08-10 6:56 ` Richard Biener 0 siblings, 1 reply; 8+ messages in thread From: Martin Liška @ 2022-08-09 12:00 UTC (permalink / raw) To: gcc-patches Patch can bootstrap on x86_64-linux-gnu and survives regression tests. Ready to be installed? Thanks, Martin gcc/ChangeLog: * gcc.cc (driver::detect_jobserver): Remove and move to jobserver.h. * lto-wrapper.cc (jobserver_active_p): Likewise. (run_gcc): Likewise. * jobserver.h: New file. --- gcc/gcc.cc | 36 +++----------------- gcc/jobserver.h | 85 ++++++++++++++++++++++++++++++++++++++++++++++ gcc/lto-wrapper.cc | 43 +++++------------------ 3 files changed, 97 insertions(+), 67 deletions(-) create mode 100644 gcc/jobserver.h diff --git a/gcc/gcc.cc b/gcc/gcc.cc index 5cbb38560b2..69fbd293eaa 100644 --- a/gcc/gcc.cc +++ b/gcc/gcc.cc @@ -43,6 +43,7 @@ compilation is specified by a string called a "spec". */ #include "opts.h" #include "filenames.h" #include "spellcheck.h" +#include "jobserver.h" \f @@ -9178,38 +9179,9 @@ driver::final_actions () const void driver::detect_jobserver () const { - /* Detect jobserver and drop it if it's not working. */ - const char *makeflags = env.get ("MAKEFLAGS"); - if (makeflags != NULL) - { - const char *needle = "--jobserver-auth="; - const char *n = strstr (makeflags, needle); - if (n != NULL) - { - int rfd = -1; - int wfd = -1; - - bool jobserver - = (sscanf (n + strlen (needle), "%d,%d", &rfd, &wfd) == 2 - && rfd > 0 - && wfd > 0 - && is_valid_fd (rfd) - && is_valid_fd (wfd)); - - /* Drop the jobserver if it's not working now. */ - if (!jobserver) - { - unsigned offset = n - makeflags; - char *dup = xstrdup (makeflags); - dup[offset] = '\0'; - - const char *space = strchr (makeflags + offset, ' '); - if (space != NULL) - strcpy (dup + offset, space); - xputenv (concat ("MAKEFLAGS=", dup, NULL)); - } - } - } + jobserver_info jinfo; + if (!jinfo.is_active && !jinfo.skipped_makeflags.empty ()) + xputenv (jinfo.skipped_makeflags.c_str ()); } /* Determine what the exit code of the driver should be. */ diff --git a/gcc/jobserver.h b/gcc/jobserver.h new file mode 100644 index 00000000000..85453dd3c79 --- /dev/null +++ b/gcc/jobserver.h @@ -0,0 +1,85 @@ +/* GNU make's jobserver related functionality. + Copyright (C) 2022 Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify it under +the terms of the GNU General Public License as published by the Free +Software Foundation; either version 3, or (at your option) any later +version. + +GCC is distributed in the hope that it will be useful, but WITHOUT ANY +WARRANTY; without even the implied warranty of MERCHANTABILITY or +FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +for more details. + +You should have received a copy of the GNU General Public License +along with GCC; see the file COPYING3. If not see +<http://www.gnu.org/licenses/>. + +See dbgcnt.def for usage information. */ + +#ifndef GCC_JOBSERVER_H +#define GCC_JOBSERVER_H + +#include <string> + +using namespace std; + +struct jobserver_info +{ + /* Default constructor. */ + jobserver_info (); + + /* Error message if there is a problem. */ + string error_msg = ""; + /* Skipped MAKEFLAGS where --jobserver-auth is skipped. */ + string skipped_makeflags = ""; + /* File descriptor for reading used for jobserver communication. */ + int rfd = -1; + /* File descriptor for writing used for jobserver communication. */ + int wfd = -1; + /* Return true if jobserver is active. */ + bool is_active = false; +}; + +jobserver_info::jobserver_info () +{ + /* Detect jobserver and drop it if it's not working. */ + string js_needle = "--jobserver-auth="; + + const char *envval = getenv ("MAKEFLAGS"); + if (envval != NULL) + { + string makeflags = envval; + size_t n = makeflags.rfind (js_needle); + if (n != string::npos) + { + if (sscanf (makeflags.c_str () + n + js_needle.size (), + "%d,%d", &rfd, &wfd) == 2 + && rfd > 0 + && wfd > 0 + && is_valid_fd (rfd) + && is_valid_fd (wfd)) + is_active = true; + else + { + string dup = makeflags.substr (0, n); + size_t pos = makeflags.find (' ', n); + if (pos != string::npos) + dup += makeflags.substr (pos); + skipped_makeflags = "MAKEFLAGS=" + dup; + error_msg + = "cannot access %<" + js_needle + "%> file descriptors"; + } + } + error_msg = "%<" + js_needle + "%> is not present in %<MAKEFLAGS%>"; + } + else + error_msg = "%<MAKEFLAGS%> environment variable is unset"; + + if (!error_msg.empty ()) + error_msg = "jobserver is not available: " + error_msg; +} + +#endif /* GCC_JOBSERVER_H */ diff --git a/gcc/lto-wrapper.cc b/gcc/lto-wrapper.cc index 795ab74555c..9279958d055 100644 --- a/gcc/lto-wrapper.cc +++ b/gcc/lto-wrapper.cc @@ -49,6 +49,8 @@ along with GCC; see the file COPYING3. If not see #include "lto-section-names.h" #include "collect-utils.h" #include "opts-diagnostic.h" +#include "opt-suggestions.h" +#include "jobserver.h" /* Environment variable, used for passing the names of offload targets from GCC driver to lto-wrapper. */ @@ -1336,35 +1338,6 @@ init_num_threads (void) #endif } -/* Test and return reason why a jobserver cannot be detected. */ - -static const char * -jobserver_active_p (void) -{ - #define JS_PREFIX "jobserver is not available: " - #define JS_NEEDLE "--jobserver-auth=" - - const char *makeflags = getenv ("MAKEFLAGS"); - if (makeflags == NULL) - return JS_PREFIX "%<MAKEFLAGS%> environment variable is unset"; - - const char *n = strstr (makeflags, JS_NEEDLE); - if (n == NULL) - return JS_PREFIX "%<" JS_NEEDLE "%> is not present in %<MAKEFLAGS%>"; - - int rfd = -1; - int wfd = -1; - - if (sscanf (n + strlen (JS_NEEDLE), "%d,%d", &rfd, &wfd) == 2 - && rfd > 0 - && wfd > 0 - && is_valid_fd (rfd) - && is_valid_fd (wfd)) - return NULL; - else - return JS_PREFIX "cannot access %<" JS_NEEDLE "%> file descriptors"; -} - /* Print link to -flto documentation with a hint message. */ void @@ -1422,7 +1395,6 @@ run_gcc (unsigned argc, char *argv[]) bool jobserver_requested = false; int auto_parallel = 0; bool no_partition = false; - const char *jobserver_error = NULL; bool fdecoded_options_first = true; vec<cl_decoded_option> fdecoded_options; fdecoded_options.create (16); @@ -1653,14 +1625,14 @@ run_gcc (unsigned argc, char *argv[]) } else { - jobserver_error = jobserver_active_p (); - if (jobserver && jobserver_error != NULL) + jobserver_info jinfo; + if (jobserver && !jinfo.is_active) { /* Fall back to auto parallelism. */ jobserver = 0; auto_parallel = 1; } - else if (!jobserver && jobserver_error == NULL) + else if (!jobserver && jinfo.is_active) { parallel = 1; jobserver = 1; @@ -1971,9 +1943,10 @@ cont: if (nr > 1) { - if (jobserver_requested && jobserver_error != NULL) + jobserver_info jinfo; + if (jobserver_requested && !jinfo.is_active) { - warning (0, jobserver_error); + warning (0, jinfo.error_msg.c_str ()); print_lto_docs_link (); } else if (parallel == 0) -- 2.37.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] Factor out jobserver_active_p. 2022-08-09 12:00 [PATCH 1/3] Factor out jobserver_active_p Martin Liška @ 2022-08-10 6:56 ` Richard Biener 2022-08-10 7:17 ` Martin Liška 0 siblings, 1 reply; 8+ messages in thread From: Richard Biener @ 2022-08-10 6:56 UTC (permalink / raw) To: Martin Liška; +Cc: GCC Patches On Tue, Aug 9, 2022 at 2:03 PM Martin Liška <mliska@suse.cz> wrote: > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? > Thanks, > Martin > > gcc/ChangeLog: > > * gcc.cc (driver::detect_jobserver): Remove and move to > jobserver.h. > * lto-wrapper.cc (jobserver_active_p): Likewise. > (run_gcc): Likewise. > * jobserver.h: New file. > --- > gcc/gcc.cc | 36 +++----------------- > gcc/jobserver.h | 85 ++++++++++++++++++++++++++++++++++++++++++++++ > gcc/lto-wrapper.cc | 43 +++++------------------ > 3 files changed, 97 insertions(+), 67 deletions(-) > create mode 100644 gcc/jobserver.h > > diff --git a/gcc/gcc.cc b/gcc/gcc.cc > index 5cbb38560b2..69fbd293eaa 100644 > --- a/gcc/gcc.cc > +++ b/gcc/gcc.cc > @@ -43,6 +43,7 @@ compilation is specified by a string called a "spec". */ > #include "opts.h" > #include "filenames.h" > #include "spellcheck.h" > +#include "jobserver.h" > > > > @@ -9178,38 +9179,9 @@ driver::final_actions () const > void > driver::detect_jobserver () const > { > - /* Detect jobserver and drop it if it's not working. */ > - const char *makeflags = env.get ("MAKEFLAGS"); > - if (makeflags != NULL) > - { > - const char *needle = "--jobserver-auth="; > - const char *n = strstr (makeflags, needle); > - if (n != NULL) > - { > - int rfd = -1; > - int wfd = -1; > - > - bool jobserver > - = (sscanf (n + strlen (needle), "%d,%d", &rfd, &wfd) == 2 > - && rfd > 0 > - && wfd > 0 > - && is_valid_fd (rfd) > - && is_valid_fd (wfd)); > - > - /* Drop the jobserver if it's not working now. */ > - if (!jobserver) > - { > - unsigned offset = n - makeflags; > - char *dup = xstrdup (makeflags); > - dup[offset] = '\0'; > - > - const char *space = strchr (makeflags + offset, ' '); > - if (space != NULL) > - strcpy (dup + offset, space); > - xputenv (concat ("MAKEFLAGS=", dup, NULL)); > - } > - } > - } > + jobserver_info jinfo; > + if (!jinfo.is_active && !jinfo.skipped_makeflags.empty ()) > + xputenv (jinfo.skipped_makeflags.c_str ()); > } > > /* Determine what the exit code of the driver should be. */ > diff --git a/gcc/jobserver.h b/gcc/jobserver.h > new file mode 100644 > index 00000000000..85453dd3c79 > --- /dev/null > +++ b/gcc/jobserver.h > @@ -0,0 +1,85 @@ > +/* GNU make's jobserver related functionality. > + Copyright (C) 2022 Free Software Foundation, Inc. > + > +This file is part of GCC. > + > +GCC is free software; you can redistribute it and/or modify it under > +the terms of the GNU General Public License as published by the Free > +Software Foundation; either version 3, or (at your option) any later > +version. > + > +GCC is distributed in the hope that it will be useful, but WITHOUT ANY > +WARRANTY; without even the implied warranty of MERCHANTABILITY or > +FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License > +for more details. > + > +You should have received a copy of the GNU General Public License > +along with GCC; see the file COPYING3. If not see > +<http://www.gnu.org/licenses/>. > + > +See dbgcnt.def for usage information. */ > + > +#ifndef GCC_JOBSERVER_H > +#define GCC_JOBSERVER_H > + > +#include <string> C++ standard library includes have to go through system.h (#define INCLUDE_STRING). Does the API really have to use std::string? > + > +using namespace std; > + > +struct jobserver_info > +{ > + /* Default constructor. */ > + jobserver_info (); > + > + /* Error message if there is a problem. */ > + string error_msg = ""; > + /* Skipped MAKEFLAGS where --jobserver-auth is skipped. */ > + string skipped_makeflags = ""; > + /* File descriptor for reading used for jobserver communication. */ > + int rfd = -1; > + /* File descriptor for writing used for jobserver communication. */ > + int wfd = -1; > + /* Return true if jobserver is active. */ > + bool is_active = false; > +}; > + > +jobserver_info::jobserver_info () > +{ > + /* Detect jobserver and drop it if it's not working. */ > + string js_needle = "--jobserver-auth="; > + > + const char *envval = getenv ("MAKEFLAGS"); > + if (envval != NULL) > + { > + string makeflags = envval; > + size_t n = makeflags.rfind (js_needle); > + if (n != string::npos) > + { > + if (sscanf (makeflags.c_str () + n + js_needle.size (), > + "%d,%d", &rfd, &wfd) == 2 > + && rfd > 0 > + && wfd > 0 > + && is_valid_fd (rfd) > + && is_valid_fd (wfd)) > + is_active = true; > + else > + { > + string dup = makeflags.substr (0, n); > + size_t pos = makeflags.find (' ', n); > + if (pos != string::npos) > + dup += makeflags.substr (pos); > + skipped_makeflags = "MAKEFLAGS=" + dup; > + error_msg > + = "cannot access %<" + js_needle + "%> file descriptors"; > + } > + } > + error_msg = "%<" + js_needle + "%> is not present in %<MAKEFLAGS%>"; > + } > + else > + error_msg = "%<MAKEFLAGS%> environment variable is unset"; > + > + if (!error_msg.empty ()) > + error_msg = "jobserver is not available: " + error_msg; > +} > + > +#endif /* GCC_JOBSERVER_H */ > diff --git a/gcc/lto-wrapper.cc b/gcc/lto-wrapper.cc > index 795ab74555c..9279958d055 100644 > --- a/gcc/lto-wrapper.cc > +++ b/gcc/lto-wrapper.cc > @@ -49,6 +49,8 @@ along with GCC; see the file COPYING3. If not see > #include "lto-section-names.h" > #include "collect-utils.h" > #include "opts-diagnostic.h" > +#include "opt-suggestions.h" > +#include "jobserver.h" > > /* Environment variable, used for passing the names of offload targets from GCC > driver to lto-wrapper. */ > @@ -1336,35 +1338,6 @@ init_num_threads (void) > #endif > } > > -/* Test and return reason why a jobserver cannot be detected. */ > - > -static const char * > -jobserver_active_p (void) > -{ > - #define JS_PREFIX "jobserver is not available: " > - #define JS_NEEDLE "--jobserver-auth=" > - > - const char *makeflags = getenv ("MAKEFLAGS"); > - if (makeflags == NULL) > - return JS_PREFIX "%<MAKEFLAGS%> environment variable is unset"; > - > - const char *n = strstr (makeflags, JS_NEEDLE); > - if (n == NULL) > - return JS_PREFIX "%<" JS_NEEDLE "%> is not present in %<MAKEFLAGS%>"; > - > - int rfd = -1; > - int wfd = -1; > - > - if (sscanf (n + strlen (JS_NEEDLE), "%d,%d", &rfd, &wfd) == 2 > - && rfd > 0 > - && wfd > 0 > - && is_valid_fd (rfd) > - && is_valid_fd (wfd)) > - return NULL; > - else > - return JS_PREFIX "cannot access %<" JS_NEEDLE "%> file descriptors"; > -} > - > /* Print link to -flto documentation with a hint message. */ > > void > @@ -1422,7 +1395,6 @@ run_gcc (unsigned argc, char *argv[]) > bool jobserver_requested = false; > int auto_parallel = 0; > bool no_partition = false; > - const char *jobserver_error = NULL; > bool fdecoded_options_first = true; > vec<cl_decoded_option> fdecoded_options; > fdecoded_options.create (16); > @@ -1653,14 +1625,14 @@ run_gcc (unsigned argc, char *argv[]) > } > else > { > - jobserver_error = jobserver_active_p (); > - if (jobserver && jobserver_error != NULL) > + jobserver_info jinfo; > + if (jobserver && !jinfo.is_active) > { > /* Fall back to auto parallelism. */ > jobserver = 0; > auto_parallel = 1; > } > - else if (!jobserver && jobserver_error == NULL) > + else if (!jobserver && jinfo.is_active) > { > parallel = 1; > jobserver = 1; > @@ -1971,9 +1943,10 @@ cont: > > if (nr > 1) > { > - if (jobserver_requested && jobserver_error != NULL) > + jobserver_info jinfo; > + if (jobserver_requested && !jinfo.is_active) > { > - warning (0, jobserver_error); > + warning (0, jinfo.error_msg.c_str ()); > print_lto_docs_link (); > } > else if (parallel == 0) > -- > 2.37.1 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] Factor out jobserver_active_p. 2022-08-10 6:56 ` Richard Biener @ 2022-08-10 7:17 ` Martin Liška 2022-08-10 7:47 ` Richard Biener 0 siblings, 1 reply; 8+ messages in thread From: Martin Liška @ 2022-08-10 7:17 UTC (permalink / raw) To: Richard Biener; +Cc: GCC Patches [-- Attachment #1: Type: text/plain, Size: 364 bytes --] On 8/10/22 08:56, Richard Biener wrote: > C++ standard library includes have to go through system.h (#define > INCLUDE_STRING). Oh, yeah. That means I need to rely on the flat header files :/ > > Does the API really have to use std::string? I would like to. My main motivation is std::string::rfind function that has no C equivalent (would be rstrstr). Martin [-- Attachment #2: 0001-Factor-out-jobserver_active_p.patch --] [-- Type: text/x-patch, Size: 7844 bytes --] From 8e1d866e9bf3005c8a8b1afa9df1bdf807b8394c Mon Sep 17 00:00:00 2001 From: Martin Liska <mliska@suse.cz> Date: Tue, 9 Aug 2022 13:59:32 +0200 Subject: [PATCH 1/3] Factor out jobserver_active_p. gcc/ChangeLog: * gcc.cc (driver::detect_jobserver): Remove and move to jobserver.h. * lto-wrapper.cc (jobserver_active_p): Likewise. (run_gcc): Likewise. * jobserver.h: New file. --- gcc/gcc.cc | 37 +++------------------ gcc/jobserver.h | 83 ++++++++++++++++++++++++++++++++++++++++++++++ gcc/lto-wrapper.cc | 44 +++++------------------- 3 files changed, 97 insertions(+), 67 deletions(-) create mode 100644 gcc/jobserver.h diff --git a/gcc/gcc.cc b/gcc/gcc.cc index 5cbb38560b2..ca70dbd3a6d 100644 --- a/gcc/gcc.cc +++ b/gcc/gcc.cc @@ -27,6 +27,7 @@ CC recognizes how to compile each input file by suffixes in the file names. Once it knows which kind of compilation to perform, the procedure for compilation is specified by a string called a "spec". */ +#define INCLUDE_STRING #include "config.h" #include "system.h" #include "coretypes.h" @@ -43,6 +44,7 @@ compilation is specified by a string called a "spec". */ #include "opts.h" #include "filenames.h" #include "spellcheck.h" +#include "jobserver.h" \f @@ -9178,38 +9180,9 @@ driver::final_actions () const void driver::detect_jobserver () const { - /* Detect jobserver and drop it if it's not working. */ - const char *makeflags = env.get ("MAKEFLAGS"); - if (makeflags != NULL) - { - const char *needle = "--jobserver-auth="; - const char *n = strstr (makeflags, needle); - if (n != NULL) - { - int rfd = -1; - int wfd = -1; - - bool jobserver - = (sscanf (n + strlen (needle), "%d,%d", &rfd, &wfd) == 2 - && rfd > 0 - && wfd > 0 - && is_valid_fd (rfd) - && is_valid_fd (wfd)); - - /* Drop the jobserver if it's not working now. */ - if (!jobserver) - { - unsigned offset = n - makeflags; - char *dup = xstrdup (makeflags); - dup[offset] = '\0'; - - const char *space = strchr (makeflags + offset, ' '); - if (space != NULL) - strcpy (dup + offset, space); - xputenv (concat ("MAKEFLAGS=", dup, NULL)); - } - } - } + jobserver_info jinfo; + if (!jinfo.is_active && !jinfo.skipped_makeflags.empty ()) + xputenv (jinfo.skipped_makeflags.c_str ()); } /* Determine what the exit code of the driver should be. */ diff --git a/gcc/jobserver.h b/gcc/jobserver.h new file mode 100644 index 00000000000..d57930ff7af --- /dev/null +++ b/gcc/jobserver.h @@ -0,0 +1,83 @@ +/* GNU make's jobserver related functionality. + Copyright (C) 2022 Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify it under +the terms of the GNU General Public License as published by the Free +Software Foundation; either version 3, or (at your option) any later +version. + +GCC is distributed in the hope that it will be useful, but WITHOUT ANY +WARRANTY; without even the implied warranty of MERCHANTABILITY or +FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +for more details. + +You should have received a copy of the GNU General Public License +along with GCC; see the file COPYING3. If not see +<http://www.gnu.org/licenses/>. + +See dbgcnt.def for usage information. */ + +#ifndef GCC_JOBSERVER_H +#define GCC_JOBSERVER_H + +using namespace std; + +struct jobserver_info +{ + /* Default constructor. */ + jobserver_info (); + + /* Error message if there is a problem. */ + string error_msg = ""; + /* Skipped MAKEFLAGS where --jobserver-auth is skipped. */ + string skipped_makeflags = ""; + /* File descriptor for reading used for jobserver communication. */ + int rfd = -1; + /* File descriptor for writing used for jobserver communication. */ + int wfd = -1; + /* Return true if jobserver is active. */ + bool is_active = false; +}; + +jobserver_info::jobserver_info () +{ + /* Detect jobserver and drop it if it's not working. */ + string js_needle = "--jobserver-auth="; + + const char *envval = getenv ("MAKEFLAGS"); + if (envval != NULL) + { + string makeflags = envval; + size_t n = makeflags.rfind (js_needle); + if (n != string::npos) + { + if (sscanf (makeflags.c_str () + n + js_needle.size (), + "%d,%d", &rfd, &wfd) == 2 + && rfd > 0 + && wfd > 0 + && is_valid_fd (rfd) + && is_valid_fd (wfd)) + is_active = true; + else + { + string dup = makeflags.substr (0, n); + size_t pos = makeflags.find (' ', n); + if (pos != string::npos) + dup += makeflags.substr (pos); + skipped_makeflags = "MAKEFLAGS=" + dup; + error_msg + = "cannot access %<" + js_needle + "%> file descriptors"; + } + } + error_msg = "%<" + js_needle + "%> is not present in %<MAKEFLAGS%>"; + } + else + error_msg = "%<MAKEFLAGS%> environment variable is unset"; + + if (!error_msg.empty ()) + error_msg = "jobserver is not available: " + error_msg; +} + +#endif /* GCC_JOBSERVER_H */ diff --git a/gcc/lto-wrapper.cc b/gcc/lto-wrapper.cc index 795ab74555c..e2dfa30c290 100644 --- a/gcc/lto-wrapper.cc +++ b/gcc/lto-wrapper.cc @@ -37,6 +37,7 @@ along with GCC; see the file COPYING3. If not see ./ccCJuXGv.lto.ltrans.o */ +#define INCLUDE_STRING #include "config.h" #include "system.h" #include "coretypes.h" @@ -49,6 +50,8 @@ along with GCC; see the file COPYING3. If not see #include "lto-section-names.h" #include "collect-utils.h" #include "opts-diagnostic.h" +#include "opt-suggestions.h" +#include "jobserver.h" /* Environment variable, used for passing the names of offload targets from GCC driver to lto-wrapper. */ @@ -1336,35 +1339,6 @@ init_num_threads (void) #endif } -/* Test and return reason why a jobserver cannot be detected. */ - -static const char * -jobserver_active_p (void) -{ - #define JS_PREFIX "jobserver is not available: " - #define JS_NEEDLE "--jobserver-auth=" - - const char *makeflags = getenv ("MAKEFLAGS"); - if (makeflags == NULL) - return JS_PREFIX "%<MAKEFLAGS%> environment variable is unset"; - - const char *n = strstr (makeflags, JS_NEEDLE); - if (n == NULL) - return JS_PREFIX "%<" JS_NEEDLE "%> is not present in %<MAKEFLAGS%>"; - - int rfd = -1; - int wfd = -1; - - if (sscanf (n + strlen (JS_NEEDLE), "%d,%d", &rfd, &wfd) == 2 - && rfd > 0 - && wfd > 0 - && is_valid_fd (rfd) - && is_valid_fd (wfd)) - return NULL; - else - return JS_PREFIX "cannot access %<" JS_NEEDLE "%> file descriptors"; -} - /* Print link to -flto documentation with a hint message. */ void @@ -1422,7 +1396,6 @@ run_gcc (unsigned argc, char *argv[]) bool jobserver_requested = false; int auto_parallel = 0; bool no_partition = false; - const char *jobserver_error = NULL; bool fdecoded_options_first = true; vec<cl_decoded_option> fdecoded_options; fdecoded_options.create (16); @@ -1653,14 +1626,14 @@ run_gcc (unsigned argc, char *argv[]) } else { - jobserver_error = jobserver_active_p (); - if (jobserver && jobserver_error != NULL) + jobserver_info jinfo; + if (jobserver && !jinfo.is_active) { /* Fall back to auto parallelism. */ jobserver = 0; auto_parallel = 1; } - else if (!jobserver && jobserver_error == NULL) + else if (!jobserver && jinfo.is_active) { parallel = 1; jobserver = 1; @@ -1971,9 +1944,10 @@ cont: if (nr > 1) { - if (jobserver_requested && jobserver_error != NULL) + jobserver_info jinfo; + if (jobserver_requested && !jinfo.is_active) { - warning (0, jobserver_error); + warning (0, jinfo.error_msg.c_str ()); print_lto_docs_link (); } else if (parallel == 0) -- 2.37.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] Factor out jobserver_active_p. 2022-08-10 7:17 ` Martin Liška @ 2022-08-10 7:47 ` Richard Biener 2022-08-10 9:30 ` Martin Liška 0 siblings, 1 reply; 8+ messages in thread From: Richard Biener @ 2022-08-10 7:47 UTC (permalink / raw) To: Martin Liška; +Cc: GCC Patches On Wed, Aug 10, 2022 at 9:17 AM Martin Liška <mliska@suse.cz> wrote: > > On 8/10/22 08:56, Richard Biener wrote: > > C++ standard library includes have to go through system.h (#define > > INCLUDE_STRING). > > Oh, yeah. That means I need to rely on the flat header files :/ > > > > > Does the API really have to use std::string? > > I would like to. My main motivation is std::string::rfind function that > has no C equivalent (would be rstrstr). The old code happily uses strstr though, not worrying about finding the last instance of --jobserver-auth? Anyway, I'm not going to insist - I just noticed the actual users use .c_str on the error message and adjusting the environment for a not working jobserver is done inconsistently. Since I'm coming from C I was more expecting sth like bool jobserver_active = probe_jobserver (true /* diagnose */); rather than pulling in a class instance from an all-inline implementation. But hey ;) > > Martin ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] Factor out jobserver_active_p. 2022-08-10 7:47 ` Richard Biener @ 2022-08-10 9:30 ` Martin Liška 2022-08-10 10:47 ` Richard Biener 0 siblings, 1 reply; 8+ messages in thread From: Martin Liška @ 2022-08-10 9:30 UTC (permalink / raw) To: Richard Biener; +Cc: GCC Patches On 8/10/22 09:47, Richard Biener wrote: > On Wed, Aug 10, 2022 at 9:17 AM Martin Liška <mliska@suse.cz> wrote: >> >> On 8/10/22 08:56, Richard Biener wrote: >>> C++ standard library includes have to go through system.h (#define >>> INCLUDE_STRING). >> >> Oh, yeah. That means I need to rely on the flat header files :/ >> >>> >>> Does the API really have to use std::string? >> >> I would like to. My main motivation is std::string::rfind function that >> has no C equivalent (would be rstrstr). > > The old code happily uses strstr though, not worrying about > finding the last instance of --jobserver-auth? Yes, sorry, I forgot to mention that, it's something I was notified by the GNU make developer here: https://savannah.gnu.org/bugs/index.php?57242#comment13 > > Anyway, I'm not going to insist - I just noticed the actual > users use .c_str on the error message and adjusting the > environment for a not working jobserver is done > inconsistently. Since I'm coming from C I was more > expecting sth like > > bool jobserver_active = probe_jobserver (true /* diagnose */); Well, the main problem is that I need to "extra" a bunch of information when parsing the env variable (and each consumer needs something else, so that's why the jobserver_info members). It was very ugly having all these return values being given as params (of pointer type). Martin > > rather than pulling in a class instance from an all-inline > implementation. But hey ;) > > >> >> Martin ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] Factor out jobserver_active_p. 2022-08-10 9:30 ` Martin Liška @ 2022-08-10 10:47 ` Richard Biener 2022-08-10 11:11 ` Martin Liška 0 siblings, 1 reply; 8+ messages in thread From: Richard Biener @ 2022-08-10 10:47 UTC (permalink / raw) To: Martin Liška; +Cc: GCC Patches On Wed, Aug 10, 2022 at 11:30 AM Martin Liška <mliska@suse.cz> wrote: > > On 8/10/22 09:47, Richard Biener wrote: > > On Wed, Aug 10, 2022 at 9:17 AM Martin Liška <mliska@suse.cz> wrote: > >> > >> On 8/10/22 08:56, Richard Biener wrote: > >>> C++ standard library includes have to go through system.h (#define > >>> INCLUDE_STRING). > >> > >> Oh, yeah. That means I need to rely on the flat header files :/ > >> > >>> > >>> Does the API really have to use std::string? > >> > >> I would like to. My main motivation is std::string::rfind function that > >> has no C equivalent (would be rstrstr). > > > > The old code happily uses strstr though, not worrying about > > finding the last instance of --jobserver-auth? > > Yes, sorry, I forgot to mention that, it's something I was notified by the GNU make > developer here: https://savannah.gnu.org/bugs/index.php?57242#comment13 > > > > > Anyway, I'm not going to insist - I just noticed the actual > > users use .c_str on the error message and adjusting the > > environment for a not working jobserver is done > > inconsistently. Since I'm coming from C I was more > > expecting sth like > > > > bool jobserver_active = probe_jobserver (true /* diagnose */); > > Well, the main problem is that I need to "extra" a bunch of information > when parsing the env variable (and each consumer needs something else, > so that's why the jobserver_info members). It was very ugly having all > these return values being given as params (of pointer type). Yeah, fair enough. > Martin > > > > > rather than pulling in a class instance from an all-inline > > implementation. But hey ;) > > > > > >> > >> Martin > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] Factor out jobserver_active_p. 2022-08-10 10:47 ` Richard Biener @ 2022-08-10 11:11 ` Martin Liška 2022-08-10 11:51 ` Richard Biener 0 siblings, 1 reply; 8+ messages in thread From: Martin Liška @ 2022-08-10 11:11 UTC (permalink / raw) To: Richard Biener; +Cc: GCC Patches [-- Attachment #1: Type: text/plain, Size: 186 bytes --] On 8/10/22 12:47, Richard Biener wrote: > Yeah, fair enough. I'm going to install the v3 where I renamed jobserver.h and moved the ctor implementation to opts-common.cc. Cheers, Martin [-- Attachment #2: 0001-Factor-out-jobserver_active_p.patch --] [-- Type: text/x-patch, Size: 8847 bytes --] From 6f3abcb3fa26f6ed719450f6bc70b2b37179973a Mon Sep 17 00:00:00 2001 From: Martin Liska <mliska@suse.cz> Date: Tue, 9 Aug 2022 13:59:32 +0200 Subject: [PATCH 1/3] Factor out jobserver_active_p. gcc/ChangeLog: * gcc.cc (driver::detect_jobserver): Remove and move to jobserver.h. * lto-wrapper.cc (jobserver_active_p): Likewise. (run_gcc): Likewise. * opts-jobserver.h: New file. * opts-common.cc (jobserver_info::jobserver_info): New function. --- gcc/gcc.cc | 37 +++++-------------------------------- gcc/lto-wrapper.cc | 44 +++++++++----------------------------------- gcc/opts-common.cc | 41 +++++++++++++++++++++++++++++++++++++++++ gcc/opts-jobserver.h | 44 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 99 insertions(+), 67 deletions(-) create mode 100644 gcc/opts-jobserver.h diff --git a/gcc/gcc.cc b/gcc/gcc.cc index 5cbb38560b2..cac11c1a117 100644 --- a/gcc/gcc.cc +++ b/gcc/gcc.cc @@ -27,6 +27,7 @@ CC recognizes how to compile each input file by suffixes in the file names. Once it knows which kind of compilation to perform, the procedure for compilation is specified by a string called a "spec". */ +#define INCLUDE_STRING #include "config.h" #include "system.h" #include "coretypes.h" @@ -43,6 +44,7 @@ compilation is specified by a string called a "spec". */ #include "opts.h" #include "filenames.h" #include "spellcheck.h" +#include "opts-jobserver.h" \f @@ -9178,38 +9180,9 @@ driver::final_actions () const void driver::detect_jobserver () const { - /* Detect jobserver and drop it if it's not working. */ - const char *makeflags = env.get ("MAKEFLAGS"); - if (makeflags != NULL) - { - const char *needle = "--jobserver-auth="; - const char *n = strstr (makeflags, needle); - if (n != NULL) - { - int rfd = -1; - int wfd = -1; - - bool jobserver - = (sscanf (n + strlen (needle), "%d,%d", &rfd, &wfd) == 2 - && rfd > 0 - && wfd > 0 - && is_valid_fd (rfd) - && is_valid_fd (wfd)); - - /* Drop the jobserver if it's not working now. */ - if (!jobserver) - { - unsigned offset = n - makeflags; - char *dup = xstrdup (makeflags); - dup[offset] = '\0'; - - const char *space = strchr (makeflags + offset, ' '); - if (space != NULL) - strcpy (dup + offset, space); - xputenv (concat ("MAKEFLAGS=", dup, NULL)); - } - } - } + jobserver_info jinfo; + if (!jinfo.is_active && !jinfo.skipped_makeflags.empty ()) + xputenv (jinfo.skipped_makeflags.c_str ()); } /* Determine what the exit code of the driver should be. */ diff --git a/gcc/lto-wrapper.cc b/gcc/lto-wrapper.cc index 795ab74555c..1e8eba16dfb 100644 --- a/gcc/lto-wrapper.cc +++ b/gcc/lto-wrapper.cc @@ -37,6 +37,7 @@ along with GCC; see the file COPYING3. If not see ./ccCJuXGv.lto.ltrans.o */ +#define INCLUDE_STRING #include "config.h" #include "system.h" #include "coretypes.h" @@ -49,6 +50,8 @@ along with GCC; see the file COPYING3. If not see #include "lto-section-names.h" #include "collect-utils.h" #include "opts-diagnostic.h" +#include "opt-suggestions.h" +#include "opts-jobserver.h" /* Environment variable, used for passing the names of offload targets from GCC driver to lto-wrapper. */ @@ -1336,35 +1339,6 @@ init_num_threads (void) #endif } -/* Test and return reason why a jobserver cannot be detected. */ - -static const char * -jobserver_active_p (void) -{ - #define JS_PREFIX "jobserver is not available: " - #define JS_NEEDLE "--jobserver-auth=" - - const char *makeflags = getenv ("MAKEFLAGS"); - if (makeflags == NULL) - return JS_PREFIX "%<MAKEFLAGS%> environment variable is unset"; - - const char *n = strstr (makeflags, JS_NEEDLE); - if (n == NULL) - return JS_PREFIX "%<" JS_NEEDLE "%> is not present in %<MAKEFLAGS%>"; - - int rfd = -1; - int wfd = -1; - - if (sscanf (n + strlen (JS_NEEDLE), "%d,%d", &rfd, &wfd) == 2 - && rfd > 0 - && wfd > 0 - && is_valid_fd (rfd) - && is_valid_fd (wfd)) - return NULL; - else - return JS_PREFIX "cannot access %<" JS_NEEDLE "%> file descriptors"; -} - /* Print link to -flto documentation with a hint message. */ void @@ -1422,7 +1396,6 @@ run_gcc (unsigned argc, char *argv[]) bool jobserver_requested = false; int auto_parallel = 0; bool no_partition = false; - const char *jobserver_error = NULL; bool fdecoded_options_first = true; vec<cl_decoded_option> fdecoded_options; fdecoded_options.create (16); @@ -1653,14 +1626,14 @@ run_gcc (unsigned argc, char *argv[]) } else { - jobserver_error = jobserver_active_p (); - if (jobserver && jobserver_error != NULL) + jobserver_info jinfo; + if (jobserver && !jinfo.is_active) { /* Fall back to auto parallelism. */ jobserver = 0; auto_parallel = 1; } - else if (!jobserver && jobserver_error == NULL) + else if (!jobserver && jinfo.is_active) { parallel = 1; jobserver = 1; @@ -1971,9 +1944,10 @@ cont: if (nr > 1) { - if (jobserver_requested && jobserver_error != NULL) + jobserver_info jinfo; + if (jobserver_requested && !jinfo.is_active) { - warning (0, jobserver_error); + warning (0, jinfo.error_msg.c_str ()); print_lto_docs_link (); } else if (parallel == 0) diff --git a/gcc/opts-common.cc b/gcc/opts-common.cc index 8097c058c72..4d4f424df13 100644 --- a/gcc/opts-common.cc +++ b/gcc/opts-common.cc @@ -17,6 +17,7 @@ You should have received a copy of the GNU General Public License along with GCC; see the file COPYING3. If not see <http://www.gnu.org/licenses/>. */ +#define INCLUDE_STRING #include "config.h" #include "system.h" #include "intl.h" @@ -25,6 +26,7 @@ along with GCC; see the file COPYING3. If not see #include "options.h" #include "diagnostic.h" #include "spellcheck.h" +#include "opts-jobserver.h" static void prune_options (struct cl_decoded_option **, unsigned int *); @@ -2005,3 +2007,42 @@ void prepend_xassembler_to_collect_as_options (const char *collect_as_options, obstack_1grow (o, '\''); } } + +jobserver_info::jobserver_info () +{ + /* Detect jobserver and drop it if it's not working. */ + string js_needle = "--jobserver-auth="; + + const char *envval = getenv ("MAKEFLAGS"); + if (envval != NULL) + { + string makeflags = envval; + size_t n = makeflags.rfind (js_needle); + if (n != string::npos) + { + if (sscanf (makeflags.c_str () + n + js_needle.size (), + "%d,%d", &rfd, &wfd) == 2 + && rfd > 0 + && wfd > 0 + && is_valid_fd (rfd) + && is_valid_fd (wfd)) + is_active = true; + else + { + string dup = makeflags.substr (0, n); + size_t pos = makeflags.find (' ', n); + if (pos != string::npos) + dup += makeflags.substr (pos); + skipped_makeflags = "MAKEFLAGS=" + dup; + error_msg + = "cannot access %<" + js_needle + "%> file descriptors"; + } + } + error_msg = "%<" + js_needle + "%> is not present in %<MAKEFLAGS%>"; + } + else + error_msg = "%<MAKEFLAGS%> environment variable is unset"; + + if (!error_msg.empty ()) + error_msg = "jobserver is not available: " + error_msg; +} diff --git a/gcc/opts-jobserver.h b/gcc/opts-jobserver.h new file mode 100644 index 00000000000..68ce188b84a --- /dev/null +++ b/gcc/opts-jobserver.h @@ -0,0 +1,44 @@ +/* GNU make's jobserver related functionality. + Copyright (C) 2022 Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify it under +the terms of the GNU General Public License as published by the Free +Software Foundation; either version 3, or (at your option) any later +version. + +GCC is distributed in the hope that it will be useful, but WITHOUT ANY +WARRANTY; without even the implied warranty of MERCHANTABILITY or +FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +for more details. + +You should have received a copy of the GNU General Public License +along with GCC; see the file COPYING3. If not see +<http://www.gnu.org/licenses/>. + +See dbgcnt.def for usage information. */ + +#ifndef GCC_JOBSERVER_H +#define GCC_JOBSERVER_H + +using namespace std; + +struct jobserver_info +{ + /* Default constructor. */ + jobserver_info (); + + /* Error message if there is a problem. */ + string error_msg = ""; + /* Skipped MAKEFLAGS where --jobserver-auth is skipped. */ + string skipped_makeflags = ""; + /* File descriptor for reading used for jobserver communication. */ + int rfd = -1; + /* File descriptor for writing used for jobserver communication. */ + int wfd = -1; + /* Return true if jobserver is active. */ + bool is_active = false; +}; + +#endif /* GCC_JOBSERVER_H */ -- 2.37.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] Factor out jobserver_active_p. 2022-08-10 11:11 ` Martin Liška @ 2022-08-10 11:51 ` Richard Biener 0 siblings, 0 replies; 8+ messages in thread From: Richard Biener @ 2022-08-10 11:51 UTC (permalink / raw) To: Martin Liška; +Cc: GCC Patches On Wed, Aug 10, 2022 at 1:11 PM Martin Liška <mliska@suse.cz> wrote: > > On 8/10/22 12:47, Richard Biener wrote: > > Yeah, fair enough. > > I'm going to install the v3 where I renamed jobserver.h > and moved the ctor implementation to opts-common.cc. For the record, the v3 series is OK > Cheers, > Martin ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-08-10 11:51 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-08-09 12:00 [PATCH 1/3] Factor out jobserver_active_p Martin Liška 2022-08-10 6:56 ` Richard Biener 2022-08-10 7:17 ` Martin Liška 2022-08-10 7:47 ` Richard Biener 2022-08-10 9:30 ` Martin Liška 2022-08-10 10:47 ` Richard Biener 2022-08-10 11:11 ` Martin Liška 2022-08-10 11:51 ` 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).