Re: [PATCH] options: Fix variable tracking option processing.

[Richard Biener <richard.guenther@gmail.com>]

On Thu, Oct 14, 2021 at 1:10 PM Martin Liška <mliska@suse.cz> wrote:
>
> On 10/13/21 15:29, Richard Biener wrote:
> > On Wed, Oct 13, 2021 at 3:12 PM Martin Liška <mliska@suse.cz> wrote:
> >>
> >> On 10/13/21 14:50, Richard Biener wrote:
> >>> It does, yes.  But that's a ^ with flag_var_tracking_assignments_toggle;)
> >>>
> >>> It's also one of the more weird flags, so it could be applied after the
> >>> otherwise single set of flag_var_tracking_assignments ...
> >>
> >> Well, it's far from being simple.
> >> Can we please make a step and install the patch I sent? I mean the latest
> >> that does the removal of AUTODETECT_VALUE.
> >
> > But parts of the patch are not obvious and you've not explained why you
> > remove all Init(AUTODETECT_VALUE) but for flag_var_tracking you
> > change it to Init(1).  I count 4 assignments to flag_var_tracking in toplev.c
> > and one in nvptx.c and c6x.c each.
>
> All right. So the assignments in these target set flag_var_tracking = 0, which
> is fine.
>
> >
> >    if (flag_var_tracking_uninit == AUTODETECT_VALUE)
> >      flag_var_tracking_uninit = flag_var_tracking;
> >
> > can probably be handled by EnabledBy, but then we also have
> >
> >    if (flag_var_tracking_uninit == 1)
> >      flag_var_tracking = 1;
>
> Yep, I made:
>
> fvar-tracking
> Common Var(flag_var_tracking) PerFunction EnabledBy(fvar-tracking-uninit)
>
> and made fvar-tracking enabled with OPT_LEVELS_1_PLUS.
>
> >
> > which suggests the same the other way around.  I guess
>
> The other way around is problematic as leads to a cycle. I tried adding
> a cycle detection in common_handle_option_auto (using a bitmap). But it breaks
> case OPT_Wunused, which set sets various sub-values :/
>
>
> > positional handling might differ with say
> > -fvar-tracking -fno-var-tracking-uninit vs. -fno-var-tracking-uninit
> > -fvar-tracking
>
> I verified this is fine in debugger.
>
> > when using EnabledBy vs. the "explicit" code.
> >
> > +  else if (!OPTION_SET_P (flag_var_tracking) && flag_var_tracking)
> >       flag_var_tracking = optimize >= 1;
> >
> > I think _this_ should, instead of the Init(1), become an entry in
> > default_options_table with OPT_LEVELS_1_PLUS.
>
> Done.
>
> >
> > As said, besides flag_var_* the posted patch looks OK and is good to
> > commit.
>
> I'm sending an updated version that survives regression tests.
>
> Thoughts?

OK.

Thanks,
Richard.

> Martin
>
> >
> > Richard.
> >
> >>
> >> Martin