Hi Segher!
Thanks for the comments!
on 2020/8/7 上午6:04, Segher Boessenkool wrote:
> Hi!
>
> On Thu, Aug 06, 2020 at 08:37:23PM +0800, Kewen.Lin wrote:
>> When I was working to update patch as Richard's review comments
>> here https://gcc.gnu.org/pipermail/gcc-patches/2020-August/551474.html，
>> I noticed that the options "-Q --help=params" don't show the final values
>> after target option overriding, instead it emits the default values in
>> params.opt (without any explicit param settings).
>>
>> I guess it's more meaningful to get it to emit values post-overrided,
>> to avoid possible confusion for users. Does it make sense?
>> Or are there any concerns?
>
> I think this makes a lot of sense.
>
>> btw, not sure whether it's a good idea to move target_option_override_hook
>> call into print_specific_help and use one function local static
>> variable to control it's called once for all kinds of help dumping
>> (possible combination), then can remove the calls in function
>> common_handle_option.
>
> I cannot easily imagine what that will look like... it could easily be
> worse than what you have here (callbacks aren't so nice, but there are
> worse things).
>
I attached opts_alt2.diff to be more specific for this, both alt1 and alt2
follow the existing callback scheme, alt2 aims to avoid possible multiple
times target_option_override_hook calls when we have several --help= or
similar, but I guess alt1 is also fine since the hook should be allowed to
be called more than once.
>> @@ -2145,9 +2146,11 @@ print_help (struct gcc_options *opts, unsigned int
>> lang_mask,
>> if (!(include_flags & CL_PARAMS))
>> exclude_flags |= CL_PARAMS;
>>
>> - if (include_flags)
>> + if (include_flags) {
>> + target_option_override_hook ();
>> print_specific_help (include_flags, exclude_flags, 0, opts,
>> lang_mask);
>> + }
>> }
>
> Indenting should be like
>
> if (include_flags)
> {
> target_option_override_hook ();
> print_specific_help (include_flags, exclude_flags, 0, opts, lang_mask);
> }
>
Thanks for catching! Updated.
BR,
Kewen
diff --git a/gcc/opts-global.c b/gcc/opts-global.c
index b1a8429dc3c..ec960c87c9a 100644
--- a/gcc/opts-global.c
+++ b/gcc/opts-global.c
@@ -328,7 +328,7 @@ decode_options (struct gcc_options *opts, struct
gcc_options *opts_set,
const char *arg;
FOR_EACH_VEC_ELT (help_option_arguments, i, arg)
- print_help (opts, lang_mask, arg);
+ print_help (opts, lang_mask, arg, target_option_override_hook);
}
/* Hold command-line options associated with stack limitation. */
diff --git a/gcc/opts.c b/gcc/opts.c
index 499eb900643..fb4d4b8aa43 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2017,7 +2017,8 @@ check_alignment_argument (location_t loc, const char
*flag, const char *name)
void
print_help (struct gcc_options *opts, unsigned int lang_mask,
- const char *help_option_argument)
+ const char *help_option_argument,
+ void (*target_option_override_hook) (void))
{
const char *a = help_option_argument;
unsigned int include_flags = 0;
@@ -2146,8 +2147,11 @@ print_help (struct gcc_options *opts, unsigned int
lang_mask,
exclude_flags |= CL_PARAMS;
if (include_flags)
- print_specific_help (include_flags, exclude_flags, 0, opts,
- lang_mask);
+ {
+ gcc_assert (target_option_override_hook);
+ target_option_override_hook ();
+ print_specific_help (include_flags, exclude_flags, 0, opts, lang_mask);
+ }
}
/* Handle target- and language-independent options. Return zero to
diff --git a/gcc/opts.h b/gcc/opts.h
index 8f594b46e33..9a837305af1 100644
--- a/gcc/opts.h
+++ b/gcc/opts.h
@@ -419,8 +419,9 @@ extern bool target_handle_option (struct gcc_options *opts,
extern void finish_options (struct gcc_options *opts,
struct gcc_options *opts_set,
location_t loc);
-extern void print_help (struct gcc_options *opts, unsigned int lang_mask, const
- char *help_option_argument);
+extern void print_help (struct gcc_options *opts, unsigned int lang_mask,
+ const char *help_option_argument,
+ void (*target_option_override_hook) (void));
extern void default_options_optimization (struct gcc_options *opts,
struct gcc_options *opts_set,
struct cl_decoded_option
*decoded_options,
diff --git a/gcc/opts-global.c b/gcc/opts-global.c
index b1a8429dc3c..ec960c87c9a 100644
--- a/gcc/opts-global.c
+++ b/gcc/opts-global.c
@@ -328,7 +328,7 @@ decode_options (struct gcc_options *opts, struct
gcc_options *opts_set,
const char *arg;
FOR_EACH_VEC_ELT (help_option_arguments, i, arg)
- print_help (opts, lang_mask, arg);
+ print_help (opts, lang_mask, arg, target_option_override_hook);
}
/* Hold command-line options associated with stack limitation. */
diff --git a/gcc/opts.c b/gcc/opts.c
index 499eb900643..6d07fa6e17e 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -1579,7 +1579,8 @@ print_specific_help (unsigned int include_flags,
unsigned int exclude_flags,
unsigned int any_flags,
struct gcc_options *opts,
- unsigned int lang_mask)
+ unsigned int lang_mask,
+ void (*target_option_override_hook) (void))
{
unsigned int all_langs_mask = (1U << cl_lang_count) - 1;
const char * description = NULL;
@@ -1664,6 +1665,14 @@ print_specific_help (unsigned int include_flags,
}
}
+ static bool call_override_once_p = false;
+ if (!call_override_once_p)
+ {
+ gcc_assert (target_option_override_hook);
+ target_option_override_hook ();
+ call_override_once_p = true;
+ }
+
printf ("%s%s:\n", description, descrip_extra);
print_filtered_help (include_flags, exclude_flags, any_flags,
opts->x_help_columns, opts, lang_mask);
@@ -2017,7 +2026,8 @@ check_alignment_argument (location_t loc, const char
*flag, const char *name)
void
print_help (struct gcc_options *opts, unsigned int lang_mask,
- const char *help_option_argument)
+ const char *help_option_argument,
+ void (*target_option_override_hook) (void))
{
const char *a = help_option_argument;
unsigned int include_flags = 0;
@@ -2146,8 +2156,8 @@ print_help (struct gcc_options *opts, unsigned int
lang_mask,
exclude_flags |= CL_PARAMS;
if (include_flags)
- print_specific_help (include_flags, exclude_flags, 0, opts,
- lang_mask);
+ print_specific_help (include_flags, exclude_flags, 0, opts, lang_mask,
+ target_option_override_hook);
}
/* Handle target- and language-independent options. Return zero to
@@ -2186,18 +2196,19 @@ common_handle_option (struct gcc_options *opts,
undoc_mask = ((opts->x_verbose_flag | opts->x_extra_warnings)
? 0
: CL_UNDOCUMENTED);
- target_option_override_hook ();
/* First display any single language specific options. */
for (i = 0; i < cl_lang_count; i++)
- print_specific_help
- (1U << i, (all_langs_mask & (~ (1U << i))) | undoc_mask, 0, opts,
- lang_mask);
+ print_specific_help (1U << i,
+ (all_langs_mask & (~(1U << i))) | undoc_mask, 0,
+ opts, lang_mask, target_option_override_hook);
/* Next display any multi language specific options. */
- print_specific_help (0, undoc_mask, all_langs_mask, opts, lang_mask);
+ print_specific_help (0, undoc_mask, all_langs_mask, opts, lang_mask,
+ target_option_override_hook);
/* Then display any remaining, non-language options. */
for (i = CL_MIN_OPTION_CLASS; i <= CL_MAX_OPTION_CLASS; i <<= 1)
if (i != CL_DRIVER)
- print_specific_help (i, undoc_mask, 0, opts, lang_mask);
+ print_specific_help (i, undoc_mask, 0, opts, lang_mask,
+ target_option_override_hook);
opts->x_exit_after_options = true;
break;
}
@@ -2206,8 +2217,8 @@ common_handle_option (struct gcc_options *opts,
if (lang_mask == CL_DRIVER)
break;
- target_option_override_hook ();
- print_specific_help (CL_TARGET, CL_UNDOCUMENTED, 0, opts, lang_mask);
+ print_specific_help (CL_TARGET, CL_UNDOCUMENTED, 0, opts, lang_mask,
+ target_option_override_hook);
opts->x_exit_after_options = true;
break;
diff --git a/gcc/opts.h b/gcc/opts.h
index 8f594b46e33..9a837305af1 100644
--- a/gcc/opts.h
+++ b/gcc/opts.h
@@ -419,8 +419,9 @@ extern bool target_handle_option (struct gcc_options *opts,
extern void finish_options (struct gcc_options *opts,
struct gcc_options *opts_set,
location_t loc);
-extern void print_help (struct gcc_options *opts, unsigned int lang_mask, const
- char *help_option_argument);
+extern void print_help (struct gcc_options *opts, unsigned int lang_mask,
+ const char *help_option_argument,
+ void (*target_option_override_hook) (void));
extern void default_options_optimization (struct gcc_options *opts,
struct gcc_options *opts_set,
struct cl_decoded_option
*decoded_options,
