On Thu, 24 Feb 2022, Qing Zhao wrote:
> I briefly checked all the usages of suppress_warning within the current gcc,
> and see that most of them are not guarded by any condition.
>
> So, the current change should be fine without introducing new issues. -:)
>
> Another thing is, if we use “warning_enable_at” to guard, I just checked,
> this routine is not used by any routine right now, so it might be possible
> that this
> routine has some bug itself. And now it’s the stage 4, we might need to be
> conservative.
>
> Based on this, I think that it might be better to put the change in as it
> right now.
> If we think that all the “suppress_warning” need to be guarded by a specific
> condition, we can do it in gcc13 earlier stage.
>
> What’s your opinion?
I would agree here. Maybe a compromise would be to simply
set gimple_set_no_warning () on the actual stmt. That shouldn't
pollute hashtables or locations in any way and the loads are
artificial so I'm not sure what we should warn about there - in
the end there's always the store that can be diagnosed for out-of-bound
accesses and the like.
Richard.
> Qing
>
>
> > On Feb 24, 2022, at 9:13 AM, Jakub Jelinek <ja...@redhat.com> wrote:
> >
> > On Thu, Feb 24, 2022 at 04:00:33PM +0100, Richard Biener wrote:
> >>>> --- a/gcc/gimple-fold.cc
> >>>> +++ b/gcc/gimple-fold.cc
> >>>> @@ -4379,7 +4379,12 @@ clear_padding_flush (clear_padding_struct *buf,
> >>>> bool full)
> >>>> else
> >>>> {
> >>>> src = make_ssa_name (type);
> >>>> - g = gimple_build_assign (src, unshare_expr (dst));
> >>>> + tree tmp_dst = unshare_expr (dst);
> >>>> + /* The folding introduces a read from the tmp_dst, we
> >>>> should
> >>>> + prevent uninitialized warning analysis from
> >>>> issuing warning
> >>>> + for such fake read. */
> >>>> + suppress_warning (tmp_dst, OPT_Wuninitialized);
> >>>
> >>> I wonder if we shouldn't guard the suppress_warning call on
> >>> if (warn_uninitialized || warn_maybe_uninitialized)
> >>> because those warnings aren't on by default and the suppress_warning
> >>> stuff,
> >>> especially when it could be done for many loads from the builtin means
> >>> populating hash tables with those.
> >>
> >> Maybe that's something suppress_warning should do then? OTOH you
> >
> > Well, OPT_Wuninitialized is an argument why it can't. The suppression
> > is using a single OPT_W*, but there are multiple different warnings
> > that care about that suppression, and suppress_warning can't know about it.
> >
> >> don't know whether you're suppressing a warning in a region with
> >> -Wno-uninitialized but that's inlined into a -Wuninitialized
> >> function where then the false diagnostic pops up if we didn't
> >> suppress the warning ...
> >
> > I think both -Wuninitialized and -Wmaybe-uninitialized aren't
> > Optimization or PerFunction, so they are global options.
> > On the other side, they can be locally changed through pragmas.
> >
> > Maybe we could use
> > if (warning_enabled_at (buf->loc, OPT_Wuninitialized)
> > || warning_enabled_at (buf->loc, OPT_Wmaybe_uninitialized))
> > if uninit pass uses the gimple_location of the read, that shouldn't
> > be really changing...
> >
> > Jakub
> >
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)
