[Bug tree-optimization/102982] New: [12 Regression] Dead Code Elimination Regression at -O3 (trunk vs 11.2.0)

[Bug tree-optimization/102982] New: [12 Regression] Dead Code Elimination Regression at -O3 (trunk vs 11.2.0)

[theodort at inf dot ethz.ch]

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

            Bug ID: 102982
           Summary: [12 Regression] Dead Code Elimination Regression at
                    -O3 (trunk vs 11.2.0)
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: theodort at inf dot ethz.ch
  Target Milestone: ---

cat test.c

void foo(void);

static short a, b, d;
static int c[2][2][2] = {0};

int main() {
  c[0][0][0] = 0;
  if (c[1][1][1]) {
    d = a % b >= 1 ^ 2;
    foo();
  }
}


11.2.0 at -O3 can eliminate the call to foo but trunk at -O3 cannot:

gcc-11 test.c -O3 -S -o /dev/stdout
...
main:
.LFB0:
        .cfi_startproc
        movl    $0, c(%rip)
        movl    c+28(%rip), %eax
        testl   %eax, %eax
        je      .L2
        ud2
        .p2align 4,,10
        .p2align 3
.L2:
        xorl    %eax, %eax
        ret
        .cfi_endproc



gcc-trunk test.c -O3 -S -o /dev/stdout
...
main:
.LFB0:
        .cfi_startproc
        movl    $0, c(%rip)
        movl    c+28(%rip), %ecx
        testl   %ecx, %ecx
        je      .L4
        pushq   %rax
        .cfi_def_cfa_offset 16
        call    foo
        xorl    %eax, %eax
        popq    %rdx
        .cfi_def_cfa_offset 8
        ret
.L4:
        xorl    %eax, %eax
        ret
        .cfi_endproc


gcc-trunk -v
Using built-in specs.
Target: x86_64-pc-linux-gnu
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 12.0.0 20211028 (experimental) (GCC)

Started with
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=b58dc0b803057c0e6032e0d9bd92cd834f72c75c

[Bug tree-optimization/102982] [12 Regression] Dead Code Elimination Regression at -O3 (trunk vs 11.2.0)

[theodort at inf dot ethz.ch]

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

--- Comment #1 from Theodoros Theodoridis <theodort at inf dot ethz.ch> ---
Maybe it's related to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102880 as
they both bisect to the same commit.

[Bug tree-optimization/102982] [12 Regression] Dead Code Elimination Regression at -O3 (trunk vs 11.2.0)

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
DSE is removing undefined behavior code which used to turn into a trap.
That is "a % b" would have caused a trap so it was turned into __builtin_trap
and now it is removed during DSE (well it is also removed after figuring out
that the store to d is not needed).  I don't know if this is a won't fix or
not.

[Bug ipa/102982] [12 Regression] Dead Code Elimination Regression at -O3 (trunk vs 11.2.0)

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
           Keywords|                            |missed-optimization
   Target Milestone|---                         |12.0
     Ever confirmed|0                           |1
                 CC|                            |hubicka at gcc dot gnu.org,
                   |                            |jamborm at gcc dot gnu.org,
                   |                            |marxin at gcc dot gnu.org
   Last reconfirmed|                            |2021-10-28
          Component|tree-optimization           |ipa

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
I'd say that's indeed "unfortunate".  The main missing optimization though is
treating

  c[0][0][0] = 0;

as a "store" when trying to make 'c' constant, not realizing it stores the
same value as the static initializer (after making it readonly we'd have to
elide all such stores though).  That would fix the testcase as to what
is likely the desired trigger of the foo() call removal.

Honza/Martin?

[Bug ipa/102982] [12 Regression] Dead Code Elimination Regression at -O3 (trunk vs 11.2.0)

[hubicka at kam dot mff.cuni.cz]

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

--- Comment #4 from hubicka at kam dot mff.cuni.cz ---
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102982
> 
> Richard Biener <rguenth at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>              Status|UNCONFIRMED                 |NEW
>            Keywords|                            |missed-optimization
>    Target Milestone|---                         |12.0
>      Ever confirmed|0                           |1
>                  CC|                            |hubicka at gcc dot gnu.org,
>                    |                            |jamborm at gcc dot gnu.org,
>                    |                            |marxin at gcc dot gnu.org
>    Last reconfirmed|                            |2021-10-28
>           Component|tree-optimization           |ipa
> 
> --- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
> I'd say that's indeed "unfortunate".  The main missing optimization though is
> treating
> 
>   c[0][0][0] = 0;
> 
> as a "store" when trying to make 'c' constant, not realizing it stores the
> same value as the static initializer (after making it readonly we'd have to
> elide all such stores though).  That would fix the testcase as to what
> is likely the desired trigger of the foo() call removal.
> 
> Honza/Martin?

That is probably not too hard to pattern match.  One needs a flag in
ipa-ref marking a fact that the store is constant 0 and at IPA time
ignore it for read/write analysis.  One catch is that once it becomes
readonly we should optimize out all stores and if we want to play well
with -disable-tree-XYZ we will want specific transform pass for that.

Also if ipa-prop was extended to handle stores/loads to static variables
as "fake arguments" of functions, perhaps we could get this more
systematic.

Honza

[Bug ipa/102982] [12 Regression] Dead Code Elimination Regression at -O3 (trunk vs 11.2.0)

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to hubicka from comment #4)
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102982
> > 
> > Richard Biener <rguenth at gcc dot gnu.org> changed:
> > 
> >            What    |Removed                     |Added
> > ----------------------------------------------------------------------------
> >              Status|UNCONFIRMED                 |NEW
> >            Keywords|                            |missed-optimization
> >    Target Milestone|---                         |12.0
> >      Ever confirmed|0                           |1
> >                  CC|                            |hubicka at gcc dot gnu.org,
> >                    |                            |jamborm at gcc dot gnu.org,
> >                    |                            |marxin at gcc dot gnu.org
> >    Last reconfirmed|                            |2021-10-28
> >           Component|tree-optimization           |ipa
> > 
> > --- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
> > I'd say that's indeed "unfortunate".  The main missing optimization though is
> > treating
> > 
> >   c[0][0][0] = 0;
> > 
> > as a "store" when trying to make 'c' constant, not realizing it stores the
> > same value as the static initializer (after making it readonly we'd have to
> > elide all such stores though).  That would fix the testcase as to what
> > is likely the desired trigger of the foo() call removal.
> > 
> > Honza/Martin?
> 
> That is probably not too hard to pattern match.  One needs a flag in
> ipa-ref marking a fact that the store is constant 0 and at IPA time
> ignore it for read/write analysis.  One catch is that once it becomes
> readonly we should optimize out all stores and if we want to play well
> with -disable-tree-XYZ we will want specific transform pass for that.

fixup_cfg already removes write-only stores so that seems fit for that
purpose.

Btw,

static int x = 1;

int main()
{
  x = 1;
}

should ideally be handled as well as maybe the more common(?)

static int x[128];

int main()
{
  memset (x, 0, 128*4);
}

so we'd like to store a (constant) RHS for the stores in the summaries?
(I suppose we cannot selectively stream in a single stmt ;))

> Also if ipa-prop was extended to handle stores/loads to static variables
> as "fake arguments" of functions, perhaps we could get this more
> systematic.
> 
> Honza

Re: [Bug ipa/102982] [12 Regression] Dead Code Elimination Regression at -O3 (trunk vs 11.2.0)

[Jan Hubicka]

> 
> fixup_cfg already removes write-only stores so that seems fit for that
> purpose.
> 
> Btw,
> 
> static int x = 1;
> 
> int main()
> {
>   x = 1;
> }
> 
> should ideally be handled as well as maybe the more common(?)
> 
> static int x[128];
> 
> int main()
> {
>   memset (x, 0, 128*4);
> }
> 
> so we'd like to store a (constant) RHS for the stores in the summaries?
> (I suppose we cannot selectively stream in a single stmt ;))

Yep, what we want is to have way to attach summaries to particular
IPA_REF_load/store/addr just like we annotate call sites...
It would be nice to extend symbol-summary.h for that.  Martin, would you
be interested to look into it?

Honza

[Bug ipa/102982] [12 Regression] Dead Code Elimination Regression at -O3 (trunk vs 11.2.0)

[hubicka at kam dot mff.cuni.cz]

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

--- Comment #6 from hubicka at kam dot mff.cuni.cz ---
> 
> fixup_cfg already removes write-only stores so that seems fit for that
> purpose.
> 
> Btw,
> 
> static int x = 1;
> 
> int main()
> {
>   x = 1;
> }
> 
> should ideally be handled as well as maybe the more common(?)
> 
> static int x[128];
> 
> int main()
> {
>   memset (x, 0, 128*4);
> }
> 
> so we'd like to store a (constant) RHS for the stores in the summaries?
> (I suppose we cannot selectively stream in a single stmt ;))

Yep, what we want is to have way to attach summaries to particular
IPA_REF_load/store/addr just like we annotate call sites...
It would be nice to extend symbol-summary.h for that.  Martin, would you
be interested to look into it?

Honza

[Bug ipa/102982] [12/13 Regression] Dead Code Elimination Regression at -O3 (trunk vs 11.2.0)

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|12.0                        |12.2

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 12.1 is being released, retargeting bugs to GCC 12.2.

[Bug ipa/102982] [12/13 Regression] Dead Code Elimination Regression at -O3 (trunk vs 11.2.0)

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

[Bug ipa/102982] [12/13/14 Regression] Dead Code Elimination Regression at -O3 (trunk vs 11.2.0)

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|12.3                        |12.4

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 12.3 is being released, retargeting bugs to GCC 12.4.