Re: [RFC][PATCH] Postpone print of --help=* option.
On 5/3/19 1:33 PM, Szabolcs Nagy wrote: > On 01/04/2019 13:11, Martin Liška wrote: >> Hi. >> >> Last week I was curious which warnings are disabled by default on top >> of -Wall and -Wextra. Thus I used --help=warning and noticed some discrepancy >> in between documentation and output of the --help option. >> >> I created PR89885 where I explained that OPT__help_ option handling happens >> early. That's why LangEnabledBy are not reflected and similarly target >> overrides >> don't take place. >> >> I'm attaching diff for --help=warning for C++ and -Ofast. >> >> Thoughts? > > since this change on arm-* and aarch64-* running RUNTESTFLAGS=help.exp i see > > FAIL: compiler driver --help=params --help=optimizers option(s): "maximum > number of" present in output > FAIL: compiler driver --help=params option(s): "[^.]$" absent from output: "e" > > (indeed previously there were several 'max-*' params > in the output now there are none) > >> >> gcc/ChangeLog: >> >> 2019-04-01 Martin Liska >> >> * gcc.c (process_command): Add dummy file only >> if n_infiles == 0. >> * opts-global.c (decode_options): Pass lang_mask. >> * opts.c (print_help): New function. >> (finish_options): Print --help if help_option_argument >> is set. >> (common_handle_option): Factor out content of OPT__help_ >> into print_help. >> * opts.h (finish_options): Add new argument. >> --- >> gcc/gcc.c | 3 +- >> gcc/opts-global.c | 2 +- >> gcc/opts.c| 267 -- >> gcc/opts.h| 3 +- >> 4 files changed, 146 insertions(+), 129 deletions(-) >> >> > Sure, there's a patch candidate: https://gcc.gnu.org/ml/gcc-patches/2019-05/msg00114.html and PR: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90315 Should be fixed soon. Martin
Re: [RFC][PATCH] Postpone print of --help=* option.
On 01/04/2019 13:11, Martin Liška wrote: > Hi. > > Last week I was curious which warnings are disabled by default on top > of -Wall and -Wextra. Thus I used --help=warning and noticed some discrepancy > in between documentation and output of the --help option. > > I created PR89885 where I explained that OPT__help_ option handling happens > early. That's why LangEnabledBy are not reflected and similarly target > overrides > don't take place. > > I'm attaching diff for --help=warning for C++ and -Ofast. > > Thoughts? since this change on arm-* and aarch64-* running RUNTESTFLAGS=help.exp i see FAIL: compiler driver --help=params --help=optimizers option(s): "maximum number of" present in output FAIL: compiler driver --help=params option(s): "[^.]$" absent from output: "e" (indeed previously there were several 'max-*' params in the output now there are none) > > gcc/ChangeLog: > > 2019-04-01 Martin Liska > > * gcc.c (process_command): Add dummy file only > if n_infiles == 0. > * opts-global.c (decode_options): Pass lang_mask. > * opts.c (print_help): New function. > (finish_options): Print --help if help_option_argument > is set. > (common_handle_option): Factor out content of OPT__help_ > into print_help. > * opts.h (finish_options): Add new argument. > --- > gcc/gcc.c | 3 +- > gcc/opts-global.c | 2 +- > gcc/opts.c| 267 -- > gcc/opts.h| 3 +- > 4 files changed, 146 insertions(+), 129 deletions(-) > >
Re: [PATCH] Fix build with offloading (Re: [RFC][PATCH] Postpone print of --help=* option.)
On Fri, 3 May 2019, Jakub Jelinek wrote: > Hi! > > On Thu, May 02, 2019 at 01:04:07PM +0200, Jakub Jelinek wrote: > > Well, that doesn't answer the question. > > I was wondering why you couldn't: > > > > 2019-05-02 Jakub Jelinek > > > > * opts.h (finish_options): Remove lang_mask argument. > > (print_help, help_option_argument): Declare. > > * opts.c (print_help): Remove forward declaration, no longer static. > > (finish_options): Remove lang_mask argument, don't call print_help > > here. > > * opts-global.c (decode_options): Adjust finish_option caller, call > > print_help here. > > On Thu, May 02, 2019 at 01:13:22PM +0200, Martin Liška wrote: > > On 5/2/19 1:04 PM, Jakub Jelinek wrote: > > > Well, that doesn't answer the question. > > > I was wondering why you couldn't: > > > > Ah sorry, you are right. The patch you suggested > > is obviously nicer than what we have currently in trunk. > > Bootstrapped/regtested successfully now on x86_64-linux and i686-linux, ok > for trunk? OK. Richard.
[PATCH] Fix build with offloading (Re: [RFC][PATCH] Postpone print of --help=* option.)
Hi! On Thu, May 02, 2019 at 01:04:07PM +0200, Jakub Jelinek wrote: > Well, that doesn't answer the question. > I was wondering why you couldn't: > > 2019-05-02 Jakub Jelinek > > * opts.h (finish_options): Remove lang_mask argument. > (print_help, help_option_argument): Declare. > * opts.c (print_help): Remove forward declaration, no longer static. > (finish_options): Remove lang_mask argument, don't call print_help > here. > * opts-global.c (decode_options): Adjust finish_option caller, call > print_help here. On Thu, May 02, 2019 at 01:13:22PM +0200, Martin Liška wrote: > On 5/2/19 1:04 PM, Jakub Jelinek wrote: > > Well, that doesn't answer the question. > > I was wondering why you couldn't: > > Ah sorry, you are right. The patch you suggested > is obviously nicer than what we have currently in trunk. Bootstrapped/regtested successfully now on x86_64-linux and i686-linux, ok for trunk? Jakub
Re: [RFC][PATCH] Postpone print of --help=* option.
On 5/2/19 1:04 PM, Jakub Jelinek wrote: > Well, that doesn't answer the question. > I was wondering why you couldn't: Ah sorry, you are right. The patch you suggested is obviously nicer than what we have currently in trunk. Feel free to install it. Martin
Re: [RFC][PATCH] Postpone print of --help=* option.
On Thu, May 02, 2019 at 12:51:05PM +0200, Martin Liška wrote: > You are right, it's guarded in #ifdef ACCEL_COMPILER > so I haven't seen the compilation error. > > > > > Any reason why you've called print_help from finish_options rather than > > decode_options after it calls finish_options? > > Yes, I need that as I need option propagation, as described in: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89885#c0 Well, that doesn't answer the question. I was wondering why you couldn't: 2019-05-02 Jakub Jelinek * opts.h (finish_options): Remove lang_mask argument. (print_help, help_option_argument): Declare. * opts.c (print_help): Remove forward declaration, no longer static. (finish_options): Remove lang_mask argument, don't call print_help here. * opts-global.c (decode_options): Adjust finish_option caller, call print_help here. --- gcc/opts.h.jj 2019-05-02 12:18:35.558051021 +0200 +++ gcc/opts.h 2019-05-02 13:00:53.488347939 +0200 @@ -418,8 +418,8 @@ extern bool target_handle_option (struct void (*target_option_override_hook) (void)); extern void finish_options (struct gcc_options *opts, struct gcc_options *opts_set, - location_t loc, - unsigned int lang_mask); + location_t loc); +extern void print_help (struct gcc_options *opts, unsigned int lang_mask); extern void default_options_optimization (struct gcc_options *opts, struct gcc_options *opts_set, struct cl_decoded_option *decoded_options, @@ -443,6 +443,8 @@ extern const struct sanitizer_opts_s bool can_recover; } sanitizer_opts[]; +extern const char *help_option_argument; + extern void add_misspelling_candidates (auto_vec *candidates, const struct cl_option *option, const char *base_option); --- gcc/opts.c.jj 2019-05-02 12:18:35.557051038 +0200 +++ gcc/opts.c 2019-05-02 13:00:34.058659611 +0200 @@ -858,15 +858,13 @@ control_options_for_live_patching (struc /* --help option argument if set. */ const char *help_option_argument = NULL; -static void print_help (struct gcc_options *opts, unsigned int lang_mask); - /* After all options at LOC have been read into OPTS and OPTS_SET, finalize settings of those options and diagnose incompatible combinations. */ void finish_options (struct gcc_options *opts, struct gcc_options *opts_set, - location_t loc, unsigned int lang_mask) + location_t loc) { enum unwind_info_type ui_except; @@ -1230,10 +1228,6 @@ finish_options (struct gcc_options *opts opts->x_flag_live_patching, loc); } - - /* Print --help=* if used. */ - if (help_option_argument != NULL) -print_help (opts, lang_mask); } #define LEFT_COLUMN27 @@ -2066,7 +2060,7 @@ check_alignment_argument (location_t loc /* Print help when OPT__help_ is set. */ -static void +void print_help (struct gcc_options *opts, unsigned int lang_mask) { const char *a = help_option_argument; --- gcc/opts-global.c.jj2019-05-02 12:18:36.673033137 +0200 +++ gcc/opts-global.c 2019-05-02 13:01:25.024842046 +0200 @@ -314,7 +314,11 @@ decode_options (struct gcc_options *opts loc, lang_mask, &handlers, dc); - finish_options (opts, opts_set, loc, lang_mask); + finish_options (opts, opts_set, loc); + + /* Print --help=* if used. */ + if (help_option_argument != NULL) +print_help (opts, lang_mask); } /* Hold command-line options associated with stack limitation. */ Jakub
Re: [RFC][PATCH] Postpone print of --help=* option.
On 5/2/19 12:30 PM, Jakub Jelinek wrote: > On Mon, Apr 01, 2019 at 02:11:17PM +0200, Martin Liška wrote: >> 2019-04-01 Martin Liska >> >> * gcc.c (process_command): Add dummy file only >> if n_infiles == 0. >> * opts-global.c (decode_options): Pass lang_mask. >> * opts.c (print_help): New function. >> (finish_options): Print --help if help_option_argument >> is set. >> (common_handle_option): Factor out content of OPT__help_ >> into print_help. >> * opts.h (finish_options): Add new argument. > > As reported by Ulrich Drepper on IRC, this broke > --enable-as-accelerator-for=... build, tree-streamer-in.c calls > finish_options too and that caller has not been adjusted. You are right, it's guarded in #ifdef ACCEL_COMPILER so I haven't seen the compilation error. > > Any reason why you've called print_help from finish_options rather than > decode_options after it calls finish_options? Yes, I need that as I need option propagation, as described in: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89885#c0 > > Or should tree-streamer-in.c pass CL_LTO as the lang_mask, or say CL_DRIVER > so that print_help does nothing in that case? Yes, that would fix it. Can you please provide a patch that he can test? Thanks, Martin > > Jakub >
Re: [RFC][PATCH] Postpone print of --help=* option.
On Mon, Apr 01, 2019 at 02:11:17PM +0200, Martin Liška wrote: > 2019-04-01 Martin Liska > > * gcc.c (process_command): Add dummy file only > if n_infiles == 0. > * opts-global.c (decode_options): Pass lang_mask. > * opts.c (print_help): New function. > (finish_options): Print --help if help_option_argument > is set. > (common_handle_option): Factor out content of OPT__help_ > into print_help. > * opts.h (finish_options): Add new argument. As reported by Ulrich Drepper on IRC, this broke --enable-as-accelerator-for=... build, tree-streamer-in.c calls finish_options too and that caller has not been adjusted. Any reason why you've called print_help from finish_options rather than decode_options after it calls finish_options? Or should tree-streamer-in.c pass CL_LTO as the lang_mask, or say CL_DRIVER so that print_help does nothing in that case? Jakub
Re: [RFC][PATCH] Postpone print of --help=* option.
On 4/30/19 6:18 PM, Jeff Law wrote: > On 4/1/19 6:11 AM, Martin Liška wrote: >> Hi. >> >> Last week I was curious which warnings are disabled by default on top >> of -Wall and -Wextra. Thus I used --help=warning and noticed some discrepancy >> in between documentation and output of the --help option. >> >> I created PR89885 where I explained that OPT__help_ option handling happens >> early. That's why LangEnabledBy are not reflected and similarly target >> overrides >> don't take place. >> >> I'm attaching diff for --help=warning for C++ and -Ofast. >> >> Thoughts? >> >> gcc/ChangeLog: >> >> 2019-04-01 Martin Liska >> >> * gcc.c (process_command): Add dummy file only >> if n_infiles == 0. >> * opts-global.c (decode_options): Pass lang_mask. >> * opts.c (print_help): New function. >> (finish_options): Print --help if help_option_argument >> is set. >> (common_handle_option): Factor out content of OPT__help_ >> into print_help. >> * opts.h (finish_options): Add new argument. >> --- >> gcc/gcc.c | 3 +- >> gcc/opts-global.c | 2 +- >> gcc/opts.c| 267 -- >> gcc/opts.h| 3 +- >> 4 files changed, 146 insertions(+), 129 deletions(-) >> >> >> >> 0001-Postpone-print-of-help-option.patch >> >> diff --git a/gcc/gcc.c b/gcc/gcc.c >> index 4f57765b012..7ce1cae28a7 100644 >> --- a/gcc/gcc.c >> +++ b/gcc/gcc.c >> @@ -4751,7 +4751,8 @@ process_command (unsigned int decoded_options_count, >> } >> >>/* Ensure we only invoke each subprocess once. */ >> - if (print_subprocess_help || print_help_list || print_version) >> + if (n_infiles == 0 >> + && (print_subprocess_help || print_help_list || print_version)) >> { >>n_infiles = 0; > The assignment to n_infiles is redundant after your change. I suspect > the optimizers will catch this, so if you want to keep it for clarity > that's fine with me. > > OK for the trunk. Your call whether or not to remove the redundant > assignment. Thank you for review. I've removed the redundancy assignment before commit. Martin > > jeff >
Re: [RFC][PATCH] Postpone print of --help=* option.
On 4/1/19 6:11 AM, Martin Liška wrote: > Hi. > > Last week I was curious which warnings are disabled by default on top > of -Wall and -Wextra. Thus I used --help=warning and noticed some discrepancy > in between documentation and output of the --help option. > > I created PR89885 where I explained that OPT__help_ option handling happens > early. That's why LangEnabledBy are not reflected and similarly target > overrides > don't take place. > > I'm attaching diff for --help=warning for C++ and -Ofast. > > Thoughts? > > gcc/ChangeLog: > > 2019-04-01 Martin Liska > > * gcc.c (process_command): Add dummy file only > if n_infiles == 0. > * opts-global.c (decode_options): Pass lang_mask. > * opts.c (print_help): New function. > (finish_options): Print --help if help_option_argument > is set. > (common_handle_option): Factor out content of OPT__help_ > into print_help. > * opts.h (finish_options): Add new argument. > --- > gcc/gcc.c | 3 +- > gcc/opts-global.c | 2 +- > gcc/opts.c| 267 -- > gcc/opts.h| 3 +- > 4 files changed, 146 insertions(+), 129 deletions(-) > > > > 0001-Postpone-print-of-help-option.patch > > diff --git a/gcc/gcc.c b/gcc/gcc.c > index 4f57765b012..7ce1cae28a7 100644 > --- a/gcc/gcc.c > +++ b/gcc/gcc.c > @@ -4751,7 +4751,8 @@ process_command (unsigned int decoded_options_count, > } > >/* Ensure we only invoke each subprocess once. */ > - if (print_subprocess_help || print_help_list || print_version) > + if (n_infiles == 0 > + && (print_subprocess_help || print_help_list || print_version)) > { >n_infiles = 0; The assignment to n_infiles is redundant after your change. I suspect the optimizers will catch this, so if you want to keep it for clarity that's fine with me. OK for the trunk. Your call whether or not to remove the redundant assignment. jeff
[RFC][PATCH] Postpone print of --help=* option.
Hi. Last week I was curious which warnings are disabled by default on top of -Wall and -Wextra. Thus I used --help=warning and noticed some discrepancy in between documentation and output of the --help option. I created PR89885 where I explained that OPT__help_ option handling happens early. That's why LangEnabledBy are not reflected and similarly target overrides don't take place. I'm attaching diff for --help=warning for C++ and -Ofast. Thoughts? gcc/ChangeLog: 2019-04-01 Martin Liska * gcc.c (process_command): Add dummy file only if n_infiles == 0. * opts-global.c (decode_options): Pass lang_mask. * opts.c (print_help): New function. (finish_options): Print --help if help_option_argument is set. (common_handle_option): Factor out content of OPT__help_ into print_help. * opts.h (finish_options): Add new argument. --- gcc/gcc.c | 3 +- gcc/opts-global.c | 2 +- gcc/opts.c| 267 -- gcc/opts.h| 3 +- 4 files changed, 146 insertions(+), 129 deletions(-) diff --git a/gcc/gcc.c b/gcc/gcc.c index 4f57765b012..7ce1cae28a7 100644 --- a/gcc/gcc.c +++ b/gcc/gcc.c @@ -4751,7 +4751,8 @@ process_command (unsigned int decoded_options_count, } /* Ensure we only invoke each subprocess once. */ - if (print_subprocess_help || print_help_list || print_version) + if (n_infiles == 0 + && (print_subprocess_help || print_help_list || print_version)) { n_infiles = 0; diff --git a/gcc/opts-global.c b/gcc/opts-global.c index a5e9ef0237a..f110fe1026f 100644 --- a/gcc/opts-global.c +++ b/gcc/opts-global.c @@ -314,7 +314,7 @@ decode_options (struct gcc_options *opts, struct gcc_options *opts_set, loc, lang_mask, &handlers, dc); - finish_options (opts, opts_set, loc); + finish_options (opts, opts_set, loc, lang_mask); } /* Hold command-line options associated with stack limitation. */ diff --git a/gcc/opts.c b/gcc/opts.c index 02f6b4656e1..707e6754294 100644 --- a/gcc/opts.c +++ b/gcc/opts.c @@ -854,12 +854,18 @@ control_options_for_live_patching (struct gcc_options *opts, } } +/* --help option argument if set. */ +const char *help_option_argument = NULL; + +static void print_help (struct gcc_options *opts, unsigned int lang_mask); + + /* After all options at LOC have been read into OPTS and OPTS_SET, finalize settings of those options and diagnose incompatible combinations. */ void finish_options (struct gcc_options *opts, struct gcc_options *opts_set, - location_t loc) + location_t loc, unsigned int lang_mask) { enum unwind_info_type ui_except; @@ -1223,6 +1229,10 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set, opts->x_flag_live_patching, loc); } + + /* Print --help=* if used. */ + if (help_option_argument != NULL) +print_help (opts, lang_mask); } #define LEFT_COLUMN 27 @@ -2052,6 +2062,135 @@ check_alignment_argument (location_t loc, const char *flag, const char *name) parse_and_check_align_values (flag, name, align_result, true, loc); } +/* Print help when OPT__help_ is set. */ + +static void +print_help (struct gcc_options *opts, unsigned int lang_mask) +{ + const char *a = help_option_argument; + unsigned int include_flags = 0; + /* Note - by default we include undocumented options when listing + specific classes. If you only want to see documented options + then add ",^undocumented" to the --help= option. E.g.: + + --help=target,^undocumented */ + unsigned int exclude_flags = 0; + + if (lang_mask == CL_DRIVER) +return; + + /* Walk along the argument string, parsing each word in turn. + The format is: + arg = [^]{word}[,{arg}] + word = {optimizers|target|warnings|undocumented| + params|common|} */ + while (*a != 0) +{ + static const struct + { + const char *string; + unsigned int flag; + } + specifics[] = + { + { "optimizers", CL_OPTIMIZATION }, + { "target", CL_TARGET }, + { "warnings", CL_WARNING }, + { "undocumented", CL_UNDOCUMENTED }, + { "params", CL_PARAMS }, + { "joined", CL_JOINED }, + { "separate", CL_SEPARATE }, + { "common", CL_COMMON }, + { NULL, 0 } + }; + unsigned int *pflags; + const char *comma; + unsigned int lang_flag, specific_flag; + unsigned int len; + unsigned int i; + + if (*a == '^') + { + ++a; + if (*a == '\0') + { + error ("missing argument to %qs", "--help=^"); + break; + } + pflags = &exclude_flags; + } + else + pflags = &include_flags; + + comma = strchr (a, ','); + if (comma == NULL) + len = strlen (a); + else + len = comma - a; + if (len == 0) + { + a = comma + 1; + continue; + } + + /* Check to see if the string matches an option class name. */ + for (i = 0, specific_flag = 0; specifics[i].string != NULL; i++) +