Message ID | 3f7bc23c-6daf-3d99-7ae7-4575a9d2659a@suse.cz |
---|---|
State | New |
Headers | show |
Series | Fix --help -Q output | expand |
On Mon, Nov 29, 2021 at 4:16 PM Martin Liška <mliska@suse.cz> wrote: > > There are cases where a default option value is -1 and > auto-detection happens in e.g. target. > > Do not print these options. Leads to the following diff: > > - -fdelete-null-pointer-checks [enabled] > + -fdelete-null-pointer-checks > @@ -332 +332 @@ > - -fleading-underscore [enabled] > + -fleading-underscore > @@ -393 +393 @@ > - -fprefetch-loop-arrays [enabled] > + -fprefetch-loop-arrays > @@ -502 +502 @@ > - -fstrict-volatile-bitfields [enabled] > + -fstrict-volatile-bitfields > @@ -533 +533 @@ > - -ftree-loop-if-convert [enabled] > + -ftree-loop-if-convert > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? > Thanks, > Martin > > PR middle-end/103438 > > gcc/ChangeLog: > > * opts-common.c (option_enabled): Return flag_var for BOOLEAN > types (the can contain an unknown value -1). > --- > gcc/opts-common.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/gcc/opts-common.c b/gcc/opts-common.c > index 9d1914ff2ff..c4a19b9a0b6 100644 > --- a/gcc/opts-common.c > +++ b/gcc/opts-common.c > @@ -1586,7 +1586,8 @@ option_flag_var (int opt_index, struct gcc_options *opts) > } > > /* Return 1 if option OPT_IDX is enabled in OPTS, 0 if it is disabled, > - or -1 if it isn't a simple on-off switch. */ > + or -1 if it isn't a simple on-off switch (or if the value is unknown, > + typically set later in target). */ > > int > option_enabled (int opt_idx, unsigned lang_mask, void *opts) > @@ -1608,9 +1609,9 @@ option_enabled (int opt_idx, unsigned lang_mask, void *opts) > { > case CLVC_BOOLEAN: > if (option->cl_host_wide_int) > - return *(HOST_WIDE_INT *) flag_var != 0; > + return *(HOST_WIDE_INT *) flag_var; > else > - return *(int *) flag_var != 0; > + return *(int *) flag_var; So can we assert that only 1, 0 and -1 are returned here? For example I see (random picked) /* [64] = */ { "--param=align-threshold=", "Select fraction of the maximal frequency of executions of basic block in function given basic block get alignment.", NULL, NULL, NULL, NULL, N_OPTS, N_OPTS, 23, /* .neg_idx = */ -1, CL_COMMON | CL_JOINED | CL_OPTIMIZATION | CL_PARAMS, 0, 0, 0, 0, 0, 0, 0, 0, 1 /* UInteger */, 0, 0, 0, offsetof (struct gcc_options, x_param_align_threshold), 0, CLVC_BOOLEAN, 0, 1, 65536 }, or /* [877] = */ { "-faligned-new=", "-faligned-new=<N> Use C++17 over-aligned type allocation for alignments greater than N.", NULL, NULL, NULL, NULL, N_OPTS, N_OPTS, 13, /* .neg_idx = */ -1, CL_CXX | CL_ObjCXX | CL_JOINED, 0, 0, 0, 0, 0, 0, 1 /* RejectNegative */, 0, 1 /* UInteger */, 0, 0, 0, offsetof (struct gcc_options, x_aligned_new_threshold), 0, CLVC_BOOLEAN, 0, -1, -1 }, and the "docs" say /* The switch is enabled when FLAG_VAR is nonzero. */ CLVC_BOOLEAN, so a -1 init contradicts this. I also wonder what determines the CLVC_* kind, it seems it's simply the "default" kind chosen when nothing else matches in var_set(). > > case CLVC_EQUAL: > if (option->cl_host_wide_int) > -- > 2.34.0 >
On 11/30/21 10:33, Richard Biener wrote: > and the "docs" say > > /* The switch is enabled when FLAG_VAR is nonzero. */ > CLVC_BOOLEAN, That's bogus, because real meaning of CLVC_BOOLEAN is that it holds an integer value. It can be just true/false for a simple flag, can be 0,1,2 for things like flag_lifetime_dse, or it can be an arbitrary integer value for things like params. That said, I would suggest removing that.. Thoughts about the patch? Martin
On Thu, Dec 2, 2021 at 3:07 PM Martin Liška <mliska@suse.cz> wrote: > > On 11/30/21 10:33, Richard Biener wrote: > > and the "docs" say > > > > /* The switch is enabled when FLAG_VAR is nonzero. */ > > CLVC_BOOLEAN, > > That's bogus, because real meaning of CLVC_BOOLEAN is that it holds an integer value. > It can be just true/false for a simple flag, can be 0,1,2 for things like flag_lifetime_dse, > or it can be an arbitrary integer value for things like params. > > That said, I would suggest removing that.. > > Thoughts about the patch? + return v > 0 ? (v < 0 ? -1 : 1) : 0; the ?:s look odd to me, the above is equivalent to v > 0 ? 1 : 0, no? Did you mean to make v > 0 v != 0? > Martin
On 12/2/21 15:11, Richard Biener wrote: > the ?:s look odd to me, the above is equivalent to v > 0 ? 1 : 0, no? > Did you mean to make v > 0 v != 0? Yeah, it's typo, should be 'v != 0 ? ...'. Martin
On 12/2/21 15:22, Martin Liška wrote: > On 12/2/21 15:11, Richard Biener wrote: >> the ?:s look odd to me, the above is equivalent to v > 0 ? 1 : 0, no? >> Did you mean to make v > 0 v != 0? > > Yeah, it's typo, should be 'v != 0 ? ...'. > > Martin There's a tested patch can bootstrap on x86_64-linux-gnu and survives regression tests. Ready to be installed? Thanks, Martin
On Thu, Dec 2, 2021 at 4:55 PM Martin Liška <mliska@suse.cz> wrote: > > On 12/2/21 15:22, Martin Liška wrote: > > On 12/2/21 15:11, Richard Biener wrote: > >> the ?:s look odd to me, the above is equivalent to v > 0 ? 1 : 0, no? > >> Did you mean to make v > 0 v != 0? > > > > Yeah, it's typo, should be 'v != 0 ? ...'. > > > > Martin > > There's a tested patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? OK. Thanks, Richard. > Thanks, > Martin
diff --git a/gcc/opts-common.c b/gcc/opts-common.c index 9d1914ff2ff..c4a19b9a0b6 100644 --- a/gcc/opts-common.c +++ b/gcc/opts-common.c @@ -1586,7 +1586,8 @@ option_flag_var (int opt_index, struct gcc_options *opts) } /* Return 1 if option OPT_IDX is enabled in OPTS, 0 if it is disabled, - or -1 if it isn't a simple on-off switch. */ + or -1 if it isn't a simple on-off switch (or if the value is unknown, + typically set later in target). */ int option_enabled (int opt_idx, unsigned lang_mask, void *opts) @@ -1608,9 +1609,9 @@ option_enabled (int opt_idx, unsigned lang_mask, void *opts) { case CLVC_BOOLEAN: if (option->cl_host_wide_int) - return *(HOST_WIDE_INT *) flag_var != 0; + return *(HOST_WIDE_INT *) flag_var; else - return *(int *) flag_var != 0; + return *(int *) flag_var; case CLVC_EQUAL: if (option->cl_host_wide_int)