On Thu, Jun 24, 2021 at 5:27 PM Martin Sebor <mse...@gmail.com> wrote:
>
> On 6/24/21 3:32 AM, Richard Biener wrote:
> > On Mon, Jun 21, 2021 at 11:55 PM Martin Sebor via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> >>
> >> Ping: https://gcc.gnu.org/pipermail/gcc-patches/2021-June/571980.html
> >>
> >> Looking for a review of the LTO changes to switch TREE_NO_WARNING to
> >> the suppress_warning() API.
> >
> > Hmm, since the warning suppressions are on location ad-hoc data the
> > appropriate
> > thing is to adjust the location streaming and that part seems to be missing?
> >
> > So what you now stream is simply the "everything" fallback, correct?
> >
> > In particular:
> >
> > else
> > - bp_pack_value (bp, TREE_NO_WARNING (expr), 1);
> > + /* FIXME: pack all warning bits. */
> > + bp_pack_value (bp, warning_suppressed_p (expr), 1);
> >
> > this looks like a wrong comment in that light.
>
> Yes, this is just a fallback. I haven't thought about how to handle
> the FIXME yet but from your comment it sounds like this code might
> stay the same (or maybe even go back to streaming the flag directly)
> and the nowarn_spec_t bitmap should be streamed elsewhere?
Yes, since the bitmap is per location it should be streamed alongside
locations in lto_{input,output}_location.
> >
> > - else
> > - compare_values (TREE_NO_WARNING);
> > + else if (t1->base.nowarning_flag != t2->base.nowarning_flag)
> > + return false;
> >
> > uh. Can you keep sth like TREE_NO_WARNING_RAW or so?
>
> The flag is used directly in fold-const.c and cp/module.cc so
> this would be in keeping with that, but I also don't mind adding
> a macro for it. My only concern is with macro getting used to
> inadvertently bypass the API.
There's precedent with other _RAW accessors, it should be
clear that it bypasses accessors and thus review should easily
spot inadverted uses.
Richard.
> Martin
>
> >
> > Thanks,
> > Richard.
> >
> >> On 6/4/21 3:43 PM, Martin Sebor wrote:
> >>> The attached patch replaces the uses of TREE_NO_WARNING in the LTO
> >>> front end with the new suppress_warning() API. It adds a couple of
> >>> FIXMEs that I plan to take care of in a follow up.
> >>
>
