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?
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
>
