public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug middle-end/104550] New: bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern
@ 2022-02-15 15:49 qinzhao at gcc dot gnu.org
  2022-02-15 15:50 ` [Bug middle-end/104550] " qinzhao at gcc dot gnu.org
                   ` (21 more replies)
  0 siblings, 22 replies; 23+ messages in thread
From: qinzhao at gcc dot gnu.org @ 2022-02-15 15:49 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550

            Bug ID: 104550
           Summary: bogus warning from -Wuninitialized +
                    -ftrivial-auto-var-init=pattern
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: qinzhao at gcc dot gnu.org
  Target Milestone: ---

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Bug middle-end/104550] bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern
  2022-02-15 15:49 [Bug middle-end/104550] New: bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern qinzhao at gcc dot gnu.org
@ 2022-02-15 15:50 ` qinzhao at gcc dot gnu.org
  2022-02-15 15:52 ` qinzhao at gcc dot gnu.org
                   ` (20 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: qinzhao at gcc dot gnu.org @ 2022-02-15 15:50 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550

--- Comment #1 from qinzhao at gcc dot gnu.org ---
Kees reported the following issue with -ftrivial-auto-var-init=pattern. the
testing case was reduced from Kernel building. 

$ cat warns.i
struct vx_audio_level {
 int has_monitor_level : 1;
};

void vx_set_monitor_level() {
 struct vx_audio_level info;
}
$ gcc -Wall -Wno-unused-variable -ftrivial-auto-var-init=zero -c -o /dev/null
warns.i
$ gcc -Wall -Wno-unused-variable -ftrivial-auto-var-init=pattern -c -o
/dev/null warns.i
warns.i: In function 'vx_set_monitor_level':
warns.i:6:25: warning: 'info' is used uninitialized [-Wuninitialized]
   6 |   struct vx_audio_level info;
     |                         ^~~~
warns.i:6:25: note: 'info' declared here
   6 |   struct vx_audio_level info;
     |                         ^~~~

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Bug middle-end/104550] bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern
  2022-02-15 15:49 [Bug middle-end/104550] New: bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern qinzhao at gcc dot gnu.org
  2022-02-15 15:50 ` [Bug middle-end/104550] " qinzhao at gcc dot gnu.org
@ 2022-02-15 15:52 ` qinzhao at gcc dot gnu.org
  2022-02-15 21:33 ` qinzhao at gcc dot gnu.org
                   ` (19 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: qinzhao at gcc dot gnu.org @ 2022-02-15 15:52 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550

qinzhao at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-02-15
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED

--- Comment #2 from qinzhao at gcc dot gnu.org ---
confirmed with the latest gcc12.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Bug middle-end/104550] bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern
  2022-02-15 15:49 [Bug middle-end/104550] New: bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern qinzhao at gcc dot gnu.org
  2022-02-15 15:50 ` [Bug middle-end/104550] " qinzhao at gcc dot gnu.org
  2022-02-15 15:52 ` qinzhao at gcc dot gnu.org
@ 2022-02-15 21:33 ` qinzhao at gcc dot gnu.org
  2022-02-15 21:38 ` pinskia at gcc dot gnu.org
                   ` (18 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: qinzhao at gcc dot gnu.org @ 2022-02-15 21:33 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550

--- Comment #3 from qinzhao at gcc dot gnu.org ---
the root cause for this bug is due to the new call to __builtin_clear_padding 
added by -ftrivial-auto-var-init=pattern, this is treated as a use of the
uninitialized variable during early uninitialized analysis phase, in which
phase DCE hasn't been applied yet, as a result, the early uninit phase reported
this uninitialized usage. 

the following is the details:

1. without -ftrivial-auto-var-init, the IR in gimple stage is:
void vx_set_monitor_level ()
{ 
  struct vx_audio_level info;

  try
    {

    }
  finally
    {
      info = {CLOBBER(eol)};
    }
}

no any usage of the auto var "info", no any warning message for
-Wuninitialized;

2. With -ftrivial-auto-var-init, the IR in gimple stage is:
void vx_set_monitor_level ()
{ 
  struct vx_audio_level info;

  try
    {
      info = .DEFERRED_INIT (4, 1, &"info"[0]);
      __builtin_clear_padding (&info, 1B);
    }
  finally
    {
      info = {CLOBBER(eol)};
    }
}

then after folding of call to "__builtin_clear_padding", the IR at early
uninitialized analysis phase is:

void vx_set_monitor_level ()
{
  struct vx_audio_level info;
  int _1;
  int _2;

  <bb 2> :
  info = .DEFERRED_INIT (4, 1, &"info"[0]);
  _1 = MEM[(struct vx_audio_level *)&info];
  _2 = _1 & 1;
  MEM[(struct vx_audio_level *)&info] = _2;
  info ={v} {CLOBBER(eol)};
  return;

}

The bogus warning is issued for the following fake usage:

 _1 = MEM[(struct vx_audio_level *)&info];

which is unavoidable during the early uninitialized stage based on the current
IR info.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Bug middle-end/104550] bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern
  2022-02-15 15:49 [Bug middle-end/104550] New: bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern qinzhao at gcc dot gnu.org
                   ` (2 preceding siblings ...)
  2022-02-15 21:33 ` qinzhao at gcc dot gnu.org
@ 2022-02-15 21:38 ` pinskia at gcc dot gnu.org
  2022-02-15 22:38 ` qing.zhao at oracle dot com
                   ` (17 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: pinskia at gcc dot gnu.org @ 2022-02-15 21:38 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Maybe __builtin_clear_padding lowering should mark the load "MEM[(struct
vx_audio_level *)&info]" as not needing a warning.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Bug middle-end/104550] bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern
  2022-02-15 15:49 [Bug middle-end/104550] New: bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern qinzhao at gcc dot gnu.org
                   ` (3 preceding siblings ...)
  2022-02-15 21:38 ` pinskia at gcc dot gnu.org
@ 2022-02-15 22:38 ` qing.zhao at oracle dot com
  2022-02-16 10:33 ` rguenther at suse dot de
                   ` (16 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: qing.zhao at oracle dot com @ 2022-02-15 22:38 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550

--- Comment #5 from Qing Zhao <qing.zhao at oracle dot com> ---
> On Feb 15, 2022, at 3:38 PM, pinskia at gcc dot gnu.org <gcc-bugzilla@gcc.gnu.org> wrote:
> Maybe __builtin_clear_padding lowering should mark the load "MEM[(struct
> vx_audio_level *)&info]" as not needing a warning.
> 
This sound like a good idea, I will study a little bit more on this direction.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Bug middle-end/104550] bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern
  2022-02-15 15:49 [Bug middle-end/104550] New: bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern qinzhao at gcc dot gnu.org
                   ` (4 preceding siblings ...)
  2022-02-15 22:38 ` qing.zhao at oracle dot com
@ 2022-02-16 10:33 ` rguenther at suse dot de
  2022-02-16 15:05 ` qinzhao at gcc dot gnu.org
                   ` (15 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: rguenther at suse dot de @ 2022-02-16 10:33 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550

--- Comment #6 from rguenther at suse dot de <rguenther at suse dot de> ---
> Am 15.02.2022 um 22:38 schrieb pinskia at gcc dot gnu.org <gcc-bugzilla@gcc.gnu.org>:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550
> 
> Andrew Pinski <pinskia at gcc dot gnu.org> changed:
> 
>           What    |Removed                     |Added
> ----------------------------------------------------------------------------
>           Keywords|                            |diagnostic
> 
> --- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
> Maybe __builtin_clear_padding lowering should mark the load "MEM[(struct
> vx_audio_level *)&info]" as not needing a warning.

Maybe padding clearing shouldn’t be exposed early but handled by .DEFFEREd_INIT
expansion?

> 
> -- 
> You are receiving this mail because:
> You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Bug middle-end/104550] bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern
  2022-02-15 15:49 [Bug middle-end/104550] New: bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern qinzhao at gcc dot gnu.org
                   ` (5 preceding siblings ...)
  2022-02-16 10:33 ` rguenther at suse dot de
@ 2022-02-16 15:05 ` qinzhao at gcc dot gnu.org
  2022-02-16 15:38 ` jakub at gcc dot gnu.org
                   ` (14 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: qinzhao at gcc dot gnu.org @ 2022-02-16 15:05 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550

--- Comment #7 from qinzhao at gcc dot gnu.org ---
(In reply to rguenther@suse.de from comment #6)
> > --- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
> > Maybe __builtin_clear_padding lowering should mark the load "MEM[(struct
> > vx_audio_level *)&info]" as not needing a warning.
> 
> Maybe padding clearing shouldn’t be exposed early but handled by
> .DEFFEREd_INIT expansion?
> 

If padding clearing is exposed too late till RTL expansion, some tree
optimization will not be able to be applied on the expanded stores? 
the approach to mark the load "MEM" as not needing a warning might be better?

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Bug middle-end/104550] bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern
  2022-02-15 15:49 [Bug middle-end/104550] New: bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern qinzhao at gcc dot gnu.org
                   ` (6 preceding siblings ...)
  2022-02-16 15:05 ` qinzhao at gcc dot gnu.org
@ 2022-02-16 15:38 ` jakub at gcc dot gnu.org
  2022-02-16 16:24 ` qinzhao at gcc dot gnu.org
                   ` (13 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-02-16 15:38 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Well, for the .DEFERRED_INIT case if the var ends up in memory, I really don't
see the point in any clear_padding, .DEFERRED_INIT expansion should just
initialize the whole DECL_RTL MEM_P slot with the pattern it wants, trying to
initialize only the non-padding bits and then only the padding bits next to
each other is a waste of CPU cycles.
Another case are C++ vars with non-trivial ctors, if we for -flifetime-dse=2
emit CLOBBERs at the start of such ctors, then IMNSHO the right thing is to
emit the zero or pattern initialization in those constructors, perhaps through
.DEFERRED_INIT.
This is the start_preparsed_function
  if (!processing_template_decl
      && (flag_lifetime_dse > 1)
      && DECL_CONSTRUCTOR_P (decl1)
      && !DECL_CLONED_FUNCTION_P (decl1)
      /* Clobbering an empty base is harmful if it overlays real data.  */
      && !is_empty_class (current_class_type)
      /* We can't clobber safely for an implicitly-defined default constructor
         because part of the initialization might happen before we enter the
         constructor, via AGGR_INIT_ZERO_FIRST (c++/68006).  */
      && !implicit_default_ctor_p (decl1))
    finish_expr_stmt (build_clobber_this ());
case.  Advantage of doing it in the ctor is that if it isn't inlined, it is
done just once per type, doesn't need to be duplicated in all the spots that
use it.
Of course, if the above conditions aren't met, then it still needs to be
initialized somewhere else like where it is done currently, or for the case of
vars with constructors for which we don't emit it perhaps do
__builtin_clear_padding after the constructor (but can we be sure that the ctor
hasn't e.g. performed placement new and built in itself some other class?).

Anyway, doing __builtin_clear_padding at RTL expansion time might be
non-trivial.  One thing we still haven't decided on what to do with the virtual
inheritance, whether we need a langhook which won't be there at expansion time,
or if we can just use binfo (but doesn't free_lang_data mess up with binfo
too)?
And right now the code has two main possibilities, either emit gimple code to
do the clearing, or set a padding bitmask in memory.  For RTL, either one could
use the latter and turn that into RTL code clearing, or we would need a third
mode in which it would be emitting RTL directly.  Emitting such code early has
the advantage of store-merging and all kinds of other optimizations though...

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Bug middle-end/104550] bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern
  2022-02-15 15:49 [Bug middle-end/104550] New: bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern qinzhao at gcc dot gnu.org
                   ` (7 preceding siblings ...)
  2022-02-16 15:38 ` jakub at gcc dot gnu.org
@ 2022-02-16 16:24 ` qinzhao at gcc dot gnu.org
  2022-02-16 16:44 ` jakub at gcc dot gnu.org
                   ` (12 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: qinzhao at gcc dot gnu.org @ 2022-02-16 16:24 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550

--- Comment #9 from qinzhao at gcc dot gnu.org ---
(In reply to Jakub Jelinek from comment #8)
> Well, for the .DEFERRED_INIT case if the var ends up in memory, I really
> don't see the point in any clear_padding, .DEFERRED_INIT expansion should
> just initialize the whole DECL_RTL MEM_P slot with the pattern it wants,
> trying to initialize only the non-padding bits and then only the padding
> bits next to each other is a waste of CPU cycles.
__builtin_clear_padding is ONLY emitted for pattern init to set the paddings to
zero. for zero init, the .DEFERRED_INIT expansion should block set all the
memory to
zero already. no __builtin_clear_padding is added for zero init. 

that's the reason why this bug is only exposed with
-ftrivial-auto-var-init=pattern

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Bug middle-end/104550] bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern
  2022-02-15 15:49 [Bug middle-end/104550] New: bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern qinzhao at gcc dot gnu.org
                   ` (8 preceding siblings ...)
  2022-02-16 16:24 ` qinzhao at gcc dot gnu.org
@ 2022-02-16 16:44 ` jakub at gcc dot gnu.org
  2022-02-16 16:57 ` qinzhao at gcc dot gnu.org
                   ` (11 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-02-16 16:44 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Even in that case, filling the memory with pattern & mask instead of filling
the memory with pattern + __builtin_clear_padding afterwards seems like a win.
Sure, in some cases combine etc. will be able to merge those back, but in many
cases it won't.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Bug middle-end/104550] bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern
  2022-02-15 15:49 [Bug middle-end/104550] New: bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern qinzhao at gcc dot gnu.org
                   ` (9 preceding siblings ...)
  2022-02-16 16:44 ` jakub at gcc dot gnu.org
@ 2022-02-16 16:57 ` qinzhao at gcc dot gnu.org
  2022-02-17  7:54 ` rguenther at suse dot de
                   ` (10 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: qinzhao at gcc dot gnu.org @ 2022-02-16 16:57 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550

--- Comment #11 from qinzhao at gcc dot gnu.org ---
(In reply to Jakub Jelinek from comment #10)
> Even in that case, filling the memory with pattern & mask instead of filling
> the memory with pattern + __builtin_clear_padding afterwards seems like a
> win.

So, you mean It's better to do the following for pattern init:

1. add call to .DEFERRED_INIT(, pattern, ) during gimplification;
2. during RTL expansion, fill the memory with pattern & mask if the variable is
in memory. 

how to implement "fill the memory with pattern & mask" during RTL expansion?
currently, we use memset to fill the memory when the variable is in memory,
pattern & mask might not fit to a BYTE-repeatable pattern that can be used for
memset?

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Bug middle-end/104550] bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern
  2022-02-15 15:49 [Bug middle-end/104550] New: bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern qinzhao at gcc dot gnu.org
                   ` (10 preceding siblings ...)
  2022-02-16 16:57 ` qinzhao at gcc dot gnu.org
@ 2022-02-17  7:54 ` rguenther at suse dot de
  2022-02-17  8:01 ` rguenther at suse dot de
                   ` (9 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: rguenther at suse dot de @ 2022-02-17  7:54 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550

--- Comment #12 from rguenther at suse dot de <rguenther at suse dot de> ---
On Wed, 16 Feb 2022, qinzhao at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550
> 
> --- Comment #7 from qinzhao at gcc dot gnu.org ---
> (In reply to rguenther@suse.de from comment #6)
> > > --- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
> > > Maybe __builtin_clear_padding lowering should mark the load "MEM[(struct
> > > vx_audio_level *)&info]" as not needing a warning.
> > 
> > Maybe padding clearing shouldn’t be exposed early but handled by
> > .DEFFEREd_INIT expansion?
> > 
> 
> If padding clearing is exposed too late till RTL expansion, some tree
> optimization will not be able to be applied on the expanded stores? 

Doesn't the same argument apply to .DEFERRED_INIT itself?  Dependent
on the .DEFERRED_INIT expansion strathegy the padding clearing might
be unneccessary (for example when using memset())?

> the approach to mark the load "MEM" as not needing a warning might be better?

It's probably a good thing anyway, the 'R' in the RMW cycle isn't really
a use.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Bug middle-end/104550] bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern
  2022-02-15 15:49 [Bug middle-end/104550] New: bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern qinzhao at gcc dot gnu.org
                   ` (11 preceding siblings ...)
  2022-02-17  7:54 ` rguenther at suse dot de
@ 2022-02-17  8:01 ` rguenther at suse dot de
  2022-02-17 16:47 ` qing.zhao at oracle dot com
                   ` (8 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: rguenther at suse dot de @ 2022-02-17  8:01 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550

--- Comment #13 from rguenther at suse dot de <rguenther at suse dot de> ---
On Wed, 16 Feb 2022, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550
> 
> Jakub Jelinek <jakub at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |jakub at gcc dot gnu.org
> 
> --- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> Well, for the .DEFERRED_INIT case if the var ends up in memory, I really don't
> see the point in any clear_padding, .DEFERRED_INIT expansion should just
> initialize the whole DECL_RTL MEM_P slot with the pattern it wants, trying to
> initialize only the non-padding bits and then only the padding bits next to
> each other is a waste of CPU cycles.
> Another case are C++ vars with non-trivial ctors, if we for -flifetime-dse=2
> emit CLOBBERs at the start of such ctors, then IMNSHO the right thing is to
> emit the zero or pattern initialization in those constructors, perhaps through
> .DEFERRED_INIT.
> This is the start_preparsed_function
>   if (!processing_template_decl
>       && (flag_lifetime_dse > 1)
>       && DECL_CONSTRUCTOR_P (decl1)
>       && !DECL_CLONED_FUNCTION_P (decl1)
>       /* Clobbering an empty base is harmful if it overlays real data.  */
>       && !is_empty_class (current_class_type)
>       /* We can't clobber safely for an implicitly-defined default constructor
>          because part of the initialization might happen before we enter the
>          constructor, via AGGR_INIT_ZERO_FIRST (c++/68006).  */
>       && !implicit_default_ctor_p (decl1))
>     finish_expr_stmt (build_clobber_this ());
> case.  Advantage of doing it in the ctor is that if it isn't inlined, it is
> done just once per type, doesn't need to be duplicated in all the spots that
> use it.
> Of course, if the above conditions aren't met, then it still needs to be
> initialized somewhere else like where it is done currently, or for the case of
> vars with constructors for which we don't emit it perhaps do
> __builtin_clear_padding after the constructor (but can we be sure that the ctor
> hasn't e.g. performed placement new and built in itself some other class?).
> 
> Anyway, doing __builtin_clear_padding at RTL expansion time might be
> non-trivial.  One thing we still haven't decided on what to do with the virtual
> inheritance, whether we need a langhook which won't be there at expansion time,
> or if we can just use binfo (but doesn't free_lang_data mess up with binfo
> too)?
> And right now the code has two main possibilities, either emit gimple code to
> do the clearing, or set a padding bitmask in memory.  For RTL, either one could
> use the latter and turn that into RTL code clearing, or we would need a third
> mode in which it would be emitting RTL directly.  Emitting such code early has
> the advantage of store-merging and all kinds of other optimizations though...

There's also the option to do .DEFERRED_INIT expansion on GIMPLE 
somewhere, maybe in ISEL.  But sure, if padding clearing needs a langhook
that's a no-go.  BINFOs are preserved to the extent needed for
devirtualization.  I suppose the mask could also be precomputed and
stuck into the .DEFERRED_INIT call itself somehow ... (encoded into
some sequence of variable number of args? <offset>, <mask>, <offset>, 
<mask>, 0)

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Bug middle-end/104550] bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern
  2022-02-15 15:49 [Bug middle-end/104550] New: bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern qinzhao at gcc dot gnu.org
                   ` (12 preceding siblings ...)
  2022-02-17  8:01 ` rguenther at suse dot de
@ 2022-02-17 16:47 ` qing.zhao at oracle dot com
  2022-02-17 16:53 ` jakub at gcc dot gnu.org
                   ` (7 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: qing.zhao at oracle dot com @ 2022-02-17 16:47 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550

--- Comment #14 from Qing Zhao <qing.zhao at oracle dot com> ---
> On Feb 17, 2022, at 1:54 AM, rguenther at suse dot de <gcc-bugzilla@gcc.gnu.org> wrote:
>> 
>> If padding clearing is exposed too late till RTL expansion, some tree
>> optimization will not be able to be applied on the expanded stores? 
> 
> Doesn't the same argument apply to .DEFERRED_INIT itself?  Dependent
> on the .DEFERRED_INIT expansion strathegy the padding clearing might
> be unneccessary (for example when using memset())?

Yes, because we use memset to initialize auto-var when it’s in memory, we do
not insert a call to
__builtin_clear_padding to zero initialization.  We only insert
__builtin_clear_padding to pattern
Initialization to set the padding to zeroes. That’s why this bug only exposed
with pattern init.
> 
>> the approach to mark the load "MEM" as not needing a warning might be better?
> 
> It's probably a good thing anyway, the 'R' in the RMW cycle isn't really
> a use.
If it’s not a real use, should we exclude this case from emitting the
uninitialized warning directly?

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Bug middle-end/104550] bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern
  2022-02-15 15:49 [Bug middle-end/104550] New: bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern qinzhao at gcc dot gnu.org
                   ` (13 preceding siblings ...)
  2022-02-17 16:47 ` qing.zhao at oracle dot com
@ 2022-02-17 16:53 ` jakub at gcc dot gnu.org
  2022-02-18  6:53 ` rguenther at suse dot de
                   ` (6 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-02-17 16:53 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550

--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to qinzhao from comment #11)
> (In reply to Jakub Jelinek from comment #10)
> > Even in that case, filling the memory with pattern & mask instead of filling
> > the memory with pattern + __builtin_clear_padding afterwards seems like a
> > win.
> 
> So, you mean It's better to do the following for pattern init:
> 
> 1. add call to .DEFERRED_INIT(, pattern, ) during gimplification;
> 2. during RTL expansion, fill the memory with pattern & mask if the variable
> is in memory. 
> 
> how to implement "fill the memory with pattern & mask" during RTL expansion?
> currently, we use memset to fill the memory when the variable is in memory,
> pattern & mask might not fit to a BYTE-repeatable pattern that can be used
> for memset?

You can populate a buffer with the pattern and then clear_type_padding_in_mask.
Then you can create a STRING_CST out from that array and store_expr
(the_STRING_CST, DECL_RTL (var), false, false, false) - at least when DECL_RTL
(var) is a MEM.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Bug middle-end/104550] bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern
  2022-02-15 15:49 [Bug middle-end/104550] New: bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern qinzhao at gcc dot gnu.org
                   ` (14 preceding siblings ...)
  2022-02-17 16:53 ` jakub at gcc dot gnu.org
@ 2022-02-18  6:53 ` rguenther at suse dot de
  2022-02-18 17:16 ` qinzhao at gcc dot gnu.org
                   ` (5 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: rguenther at suse dot de @ 2022-02-18  6:53 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550

--- Comment #16 from rguenther at suse dot de <rguenther at suse dot de> ---
On Thu, 17 Feb 2022, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550
> 
> --- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> (In reply to qinzhao from comment #11)
> > (In reply to Jakub Jelinek from comment #10)
> > > Even in that case, filling the memory with pattern & mask instead of filling
> > > the memory with pattern + __builtin_clear_padding afterwards seems like a
> > > win.
> > 
> > So, you mean It's better to do the following for pattern init:
> > 
> > 1. add call to .DEFERRED_INIT(, pattern, ) during gimplification;
> > 2. during RTL expansion, fill the memory with pattern & mask if the variable
> > is in memory. 
> > 
> > how to implement "fill the memory with pattern & mask" during RTL expansion?
> > currently, we use memset to fill the memory when the variable is in memory,
> > pattern & mask might not fit to a BYTE-repeatable pattern that can be used
> > for memset?
> 
> You can populate a buffer with the pattern and then clear_type_padding_in_mask.
> Then you can create a STRING_CST out from that array and store_expr
> (the_STRING_CST, DECL_RTL (var), false, false, false) - at least when DECL_RTL
> (var) is a MEM.

It would of course be _highly_ desirable to _not_ need to build a
STRING_CST here.  The problematical cases are all when DECL_RTL isn't
a MEM, of course.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Bug middle-end/104550] bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern
  2022-02-15 15:49 [Bug middle-end/104550] New: bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern qinzhao at gcc dot gnu.org
                   ` (15 preceding siblings ...)
  2022-02-18  6:53 ` rguenther at suse dot de
@ 2022-02-18 17:16 ` qinzhao at gcc dot gnu.org
  2022-02-18 20:14 ` qinzhao at gcc dot gnu.org
                   ` (4 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: qinzhao at gcc dot gnu.org @ 2022-02-18 17:16 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550

--- Comment #17 from qinzhao at gcc dot gnu.org ---
So, based on the discussion so far, I'd like to take the following steps:

1. In GCC12, I will take a conservative solution to fix this bug, i.e:

mark the load "MEM" as not needing a warning during __builtin_clear_padding
folding phase;

this should resolve this issue and has lowest risk to introduce more issues.

2. In GCC13, seeking a better way to do padding initialization. right now,
based on the discussion so far, there is no conclusion on which way is better
yet.

let me know if you have other comments or suggestions.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Bug middle-end/104550] bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern
  2022-02-15 15:49 [Bug middle-end/104550] New: bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern qinzhao at gcc dot gnu.org
                   ` (16 preceding siblings ...)
  2022-02-18 17:16 ` qinzhao at gcc dot gnu.org
@ 2022-02-18 20:14 ` qinzhao at gcc dot gnu.org
  2022-02-21  7:56 ` rguenth at gcc dot gnu.org
                   ` (3 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: qinzhao at gcc dot gnu.org @ 2022-02-18 20:14 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550

--- Comment #18 from qinzhao at gcc dot gnu.org ---
One question here, for the following testing case:

[opc@qinzhao-ol7u9 104550]$ cat t1.c
struct vx_audio_level {
 int has_monitor_level : 1;
};

void vx_set_monitor_level() {
 struct vx_audio_level info;
 __builtin_clear_padding (&info);
}
[opc@qinzhao-ol7u9 104550]$ sh t
/home/opc/Install/latest/bin/gcc -O -Wuninitialized -Wall t1.c -S
t1.c: In function ‘vx_set_monitor_level’:
t1.c:7:2: warning: ‘info’ is used uninitialized [-Wuninitialized]
    7 |  __builtin_clear_padding (&info);
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
t1.c:6:24: note: ‘info’ declared here
    6 |  struct vx_audio_level info;
      |                        ^~~~

We can see that the compiler emitted the exactly same warning as with
-ftrivial-auto-var-init=pattern. 

my question is, is the "info" in __builtin_clear_padding(&info) a REAL use of
"info"? is it correct to report the uninitialized use message for it?

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Bug middle-end/104550] bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern
  2022-02-15 15:49 [Bug middle-end/104550] New: bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern qinzhao at gcc dot gnu.org
                   ` (17 preceding siblings ...)
  2022-02-18 20:14 ` qinzhao at gcc dot gnu.org
@ 2022-02-21  7:56 ` rguenth at gcc dot gnu.org
  2022-02-21 15:21 ` qing.zhao at oracle dot com
                   ` (2 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-02-21  7:56 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550

--- Comment #19 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to qinzhao from comment #18)
> One question here, for the following testing case:
> 
> [opc@qinzhao-ol7u9 104550]$ cat t1.c
> struct vx_audio_level {
>  int has_monitor_level : 1;
> };
> 
> void vx_set_monitor_level() {
>  struct vx_audio_level info;
>  __builtin_clear_padding (&info);
> }
> [opc@qinzhao-ol7u9 104550]$ sh t
> /home/opc/Install/latest/bin/gcc -O -Wuninitialized -Wall t1.c -S
> t1.c: In function ‘vx_set_monitor_level’:
> t1.c:7:2: warning: ‘info’ is used uninitialized [-Wuninitialized]
>     7 |  __builtin_clear_padding (&info);
>       |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> t1.c:6:24: note: ‘info’ declared here
>     6 |  struct vx_audio_level info;
>       |                        ^~~~
> 
> We can see that the compiler emitted the exactly same warning as with
> -ftrivial-auto-var-init=pattern. 
> 
> my question is, is the "info" in __builtin_clear_padding(&info) a REAL use
> of "info"? is it correct to report the uninitialized use message for it?

As said I think reads emitted from __builtin_clear_padding lowering should
not be considered for (uninit) diagnostics.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Bug middle-end/104550] bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern
  2022-02-15 15:49 [Bug middle-end/104550] New: bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern qinzhao at gcc dot gnu.org
                   ` (18 preceding siblings ...)
  2022-02-21  7:56 ` rguenth at gcc dot gnu.org
@ 2022-02-21 15:21 ` qing.zhao at oracle dot com
  2022-02-28 15:58 ` cvs-commit at gcc dot gnu.org
  2022-02-28 16:00 ` qinzhao at gcc dot gnu.org
  21 siblings, 0 replies; 23+ messages in thread
From: qing.zhao at oracle dot com @ 2022-02-21 15:21 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550

--- Comment #20 from Qing Zhao <qing.zhao at oracle dot com> ---
> On Feb 21, 2022, at 1:56 AM, rguenth at gcc dot gnu.org <gcc-bugzilla@gcc.gnu.org> wrote:
>> 
>> my question is, is the "info" in __builtin_clear_padding(&info) a REAL use
>> of "info"? is it correct to report the uninitialized use message for it?
> 
> As said I think reads emitted from __builtin_clear_padding lowering should
> not be considered for (uninit) diagnostics.
Okay.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Bug middle-end/104550] bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern
  2022-02-15 15:49 [Bug middle-end/104550] New: bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern qinzhao at gcc dot gnu.org
                   ` (19 preceding siblings ...)
  2022-02-21 15:21 ` qing.zhao at oracle dot com
@ 2022-02-28 15:58 ` cvs-commit at gcc dot gnu.org
  2022-02-28 16:00 ` qinzhao at gcc dot gnu.org
  21 siblings, 0 replies; 23+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-02-28 15:58 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550

--- Comment #21 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Qing Zhao <qinzhao@gcc.gnu.org>:

https://gcc.gnu.org/g:3f3246eb16f554c70c5ce87ad2c785f83adb4625

commit r12-7411-g3f3246eb16f554c70c5ce87ad2c785f83adb4625
Author: Qing Zhao <qing.zhao@oracle.com>
Date:   Mon Feb 28 15:58:43 2022 +0000

    Suppress uninitialized warnings for new created uses from
__builtin_clear_padding folding [PR104550]

    __builtin_clear_padding(&object) will clear all the padding bits of the
object.
    actually, it doesn't involve any use of an user variable. Therefore, users
do
    not expect any uninitialized warning from it. It's reasonable to suppress
    uninitialized warnings for all new created uses from
__builtin_clear_padding
    folding.

            PR middle-end/104550

    gcc/ChangeLog:

            * gimple-fold.cc (clear_padding_flush): Suppress warnings for new
            created uses.

    gcc/testsuite/ChangeLog:

            * gcc.dg/auto-init-pr104550-1.c: New test.
            * gcc.dg/auto-init-pr104550-2.c: New test.
            * gcc.dg/auto-init-pr104550-3.c: New test.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Bug middle-end/104550] bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern
  2022-02-15 15:49 [Bug middle-end/104550] New: bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern qinzhao at gcc dot gnu.org
                   ` (20 preceding siblings ...)
  2022-02-28 15:58 ` cvs-commit at gcc dot gnu.org
@ 2022-02-28 16:00 ` qinzhao at gcc dot gnu.org
  21 siblings, 0 replies; 23+ messages in thread
From: qinzhao at gcc dot gnu.org @ 2022-02-28 16:00 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550

qinzhao at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #22 from qinzhao at gcc dot gnu.org ---
Fixed in gcc12

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2022-02-28 16:00 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-15 15:49 [Bug middle-end/104550] New: bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern qinzhao at gcc dot gnu.org
2022-02-15 15:50 ` [Bug middle-end/104550] " qinzhao at gcc dot gnu.org
2022-02-15 15:52 ` qinzhao at gcc dot gnu.org
2022-02-15 21:33 ` qinzhao at gcc dot gnu.org
2022-02-15 21:38 ` pinskia at gcc dot gnu.org
2022-02-15 22:38 ` qing.zhao at oracle dot com
2022-02-16 10:33 ` rguenther at suse dot de
2022-02-16 15:05 ` qinzhao at gcc dot gnu.org
2022-02-16 15:38 ` jakub at gcc dot gnu.org
2022-02-16 16:24 ` qinzhao at gcc dot gnu.org
2022-02-16 16:44 ` jakub at gcc dot gnu.org
2022-02-16 16:57 ` qinzhao at gcc dot gnu.org
2022-02-17  7:54 ` rguenther at suse dot de
2022-02-17  8:01 ` rguenther at suse dot de
2022-02-17 16:47 ` qing.zhao at oracle dot com
2022-02-17 16:53 ` jakub at gcc dot gnu.org
2022-02-18  6:53 ` rguenther at suse dot de
2022-02-18 17:16 ` qinzhao at gcc dot gnu.org
2022-02-18 20:14 ` qinzhao at gcc dot gnu.org
2022-02-21  7:56 ` rguenth at gcc dot gnu.org
2022-02-21 15:21 ` qing.zhao at oracle dot com
2022-02-28 15:58 ` cvs-commit at gcc dot gnu.org
2022-02-28 16:00 ` qinzhao at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).