[Bug c/107407] New: Wrong code at -Os on x86_64-linux-gnu

[Bug c/107407] New: Wrong code at -Os on x86_64-linux-gnu

[shaohua.li at inf dot ethz.ch]

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

            Bug ID: 107407
           Summary: Wrong code at -Os on x86_64-linux-gnu
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: shaohua.li at inf dot ethz.ch
  Target Milestone: ---

% gcc-tk -v
Using built-in specs.
COLLECT_GCC=gcc-tk
COLLECT_LTO_WRAPPER=/zdata/shaoli/compilers/ccbuilder-compilers/gcc-97d1ed67fc6a5773c8c00875bfa3616a457cf5f9/libexec/gcc/x86_64-pc-linux-gnu/13.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../configure --disable-multilib --disable-bootstrap
--enable-languages=c,c++
--prefix=/zdata/shaoli/compilers/ccbuilder-compilers/gcc-97d1ed67fc6a5773c8c00875bfa3616a457cf5f9
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 13.0.0 20221024 (experimental) (GCC)
%
% gcc-tk -O0 a.c && ./a.out
3
0
0
0
% gcc-tk -Os a.c && ./a.out
0
0
0
0
% cat a.c
int printf(const char *, ...);
int *a;
static int **b = &a;
int c[4];
int d;

int e(int);

static int f(char, int);

void g() { e((f(0, 0), 0)); }

int e(int h) { *a = 0; }

int f(char k, int j) {
  for (;;) {
    for (; k <= 3; k++) {
      *b = &c[k];
      for (; d <= 1; d++)
        *a = 3;
    }
    break;
  }
}
int main() {
  int i;
  g();
  for (i = 0; i < 4; i++)
    printf("%d\n", c[i]);
}
%

Compiler explorer: https://godbolt.org/z/dTa1dvKfT

[Bug tree-optimization/107407] [12/13 Regression] Wrong code at -Os on x86_64-linux-gnu since r12-383-g32955416d8040b1f

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
          Component|c                           |tree-optimization
   Last reconfirmed|                            |2022-10-26
   Target Milestone|---                         |13.0
     Ever confirmed|0                           |1
                 CC|                            |marxin at gcc dot gnu.org,
                   |                            |rguenth at gcc dot gnu.org
            Summary|Wrong code at -Os on        |[12/13 Regression] Wrong
                   |x86_64-linux-gnu            |code at -Os on
                   |                            |x86_64-linux-gnu since
                   |                            |r12-383-g32955416d8040b1f

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
A bit reduced, started with r12-383-g32955416d8040b1f

int *a;
static int **b = &a;
int c[4];
int d;

int e(int);

static int f(char, int);

void g() { e((f(0, 0), 0)); }

int e(int h) { *a = 0; }

int f(char k, int j) {
  for (;;) {
    for (; k <= 3; k++) {
      *b = &c[k];
      for (; d <= 1; d++)
        *a = 3;
    }
    break;
  }
}
int main() {
  int i;
  g();
  if (c[0] != 3)
    __builtin_abort ();
}

[Bug tree-optimization/107407] [12/13 Regression] Wrong code at -Os on x86_64-linux-gnu since r12-383-g32955416d8040b1f

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1

[Bug tree-optimization/107407] [12/13 Regression] Wrong code at -Os on x86_64-linux-gnu since r12-383-g32955416d8040b1f

[pinskia at gcc dot gnu.org]

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

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

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

[Bug tree-optimization/107407] [12/13 Regression] Wrong code at -Os on x86_64-linux-gnu since r12-383-g32955416d8040b1f

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org
           Priority|P1                          |P2
             Status|NEW                         |ASSIGNED

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
I will have a look.

[Bug tree-optimization/107407] [12/13 Regression] Wrong code at -Os on x86_64-linux-gnu since r12-383-g32955416d8040b1f

[rguenth at gcc dot gnu.org]

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

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
  Deleted dead store: MEM <int[4]> [(int *)&c][_6] = 3;

Further reduced testcase, we mistreat SSA name indexes with must aliases.
We do have

          /* If we visit this PHI by following a backedge then we have to
             make sure ref->ref only refers to SSA names that are invariant
             with respect to the loop represented by this PHI node.  */
          if (dominated_by_p (CDI_DOMINATORS, gimple_bb (stmt),
                              gimple_bb (temp))
              && !for_each_index (ref->ref ? &ref->ref : &ref->base,
                                  check_name, gimple_bb (temp)))
            return DSE_STORE_LIVE;

that is supposed to catch this but somehow it doesn't trigger here.

int *a;
int c[4];
int d;

static int
f(char k, int j)
{
  for (; k <= 3; k++)
    {
      a = &c[k];
      for (; d <= 1; d++)
        *a = 3;
    }
  *a = 0;
}

int main()
{
  int i;
  f(0, 0);
  if (c[0] != 3)
    __builtin_abort ();
}


So what happens is that we elide processing of this check with

          /* In addition to kills we can remove defs whose only use
             is another def in defs.  That can only ever be PHIs of which
             we track two for simplicity reasons, the first and last in
             {first,last}_phi_def (we fail for multiple PHIs anyways).
             We can also ignore defs that feed only into
             already visited PHIs.  */
          else if (single_imm_use (vdef, &use_p, &use_stmt)
                   && (use_stmt == first_phi_def
                       || use_stmt == last_phi_def
                       || (gimple_code (use_stmt) == GIMPLE_PHI
                           && bitmap_bit_p (visited,
                                            SSA_NAME_VERSION
                                              (PHI_RESULT (use_stmt))))))

where we have the last PHI being the outer loop virtual PHI and the first
PHI being the loop exit PHI of the outer loop and we've already processed
the single immediate use of the outer loop PHI, the inner loop PHI.  But
we still have to perform the above check!

It's easiest to perform the check when we visit the PHI node instead of
delaying it to the actual processing loop.

[Bug tree-optimization/107407] [12/13 Regression] Wrong code at -Os on x86_64-linux-gnu since r12-383-g32955416d8040b1f

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:031a400e49d8db156c43f9ec0b21ab0c2aee8c6d

commit r13-3546-g031a400e49d8db156c43f9ec0b21ab0c2aee8c6d
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Oct 28 15:03:49 2022 +0200

    tree-optimization/107407 - wrong code with DSE

    So what happens is that we elide processing of this check with

              /* In addition to kills we can remove defs whose only use
                 is another def in defs.  That can only ever be PHIs of which
                 we track two for simplicity reasons, the first and last in
                 {first,last}_phi_def (we fail for multiple PHIs anyways).
                 We can also ignore defs that feed only into
                 already visited PHIs.  */
              else if (single_imm_use (vdef, &use_p, &use_stmt)
                       && (use_stmt == first_phi_def
                           || use_stmt == last_phi_def
                           || (gimple_code (use_stmt) == GIMPLE_PHI
                               && bitmap_bit_p (visited,
                                                SSA_NAME_VERSION
                                                  (PHI_RESULT (use_stmt))))))

    where we have the last PHI being the outer loop virtual PHI and the first
    PHI being the loop exit PHI of the outer loop and we've already processed
    the single immediate use of the outer loop PHI, the inner loop PHI.  But
    we still have to perform the above check!

    It's easiest to perform the check when we visit the PHI node instead of
    delaying it to the actual processing loop.

            PR tree-optimization/107407
            * tree-ssa-dse.cc (dse_classify_store): Perform backedge
            varying index check when collecting PHI uses rather than
            after optimizing processing of the candidate defs.

            * gcc.dg/torture/pr107407.c: New testcase.

[Bug tree-optimization/107407] [12 Regression] Wrong code at -Os on x86_64-linux-gnu since r12-383-g32955416d8040b1f

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |12.2.0
            Summary|[12/13 Regression] Wrong    |[12 Regression] Wrong code
                   |code at -Os on              |at -Os on x86_64-linux-gnu
                   |x86_64-linux-gnu since      |since
                   |r12-383-g32955416d8040b1f   |r12-383-g32955416d8040b1f
      Known to work|                            |13.0

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed on trunk sofar.

[Bug tree-optimization/107407] [12 Regression] Wrong code at -Os on x86_64-linux-gnu since r12-383-g32955416d8040b1f

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Richard Biener
<rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:e0e37f97e67c99604129eeac2e73ccd3719ccb28

commit r12-8973-ge0e37f97e67c99604129eeac2e73ccd3719ccb28
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Oct 28 15:03:49 2022 +0200

    tree-optimization/107407 - wrong code with DSE

    So what happens is that we elide processing of this check with

              /* In addition to kills we can remove defs whose only use
                 is another def in defs.  That can only ever be PHIs of which
                 we track two for simplicity reasons, the first and last in
                 {first,last}_phi_def (we fail for multiple PHIs anyways).
                 We can also ignore defs that feed only into
                 already visited PHIs.  */
              else if (single_imm_use (vdef, &use_p, &use_stmt)
                       && (use_stmt == first_phi_def
                           || use_stmt == last_phi_def
                           || (gimple_code (use_stmt) == GIMPLE_PHI
                               && bitmap_bit_p (visited,
                                                SSA_NAME_VERSION
                                                  (PHI_RESULT (use_stmt))))))

    where we have the last PHI being the outer loop virtual PHI and the first
    PHI being the loop exit PHI of the outer loop and we've already processed
    the single immediate use of the outer loop PHI, the inner loop PHI.  But
    we still have to perform the above check!

    It's easiest to perform the check when we visit the PHI node instead of
    delaying it to the actual processing loop.

            PR tree-optimization/107407
            * tree-ssa-dse.cc (dse_classify_store): Perform backedge
            varying index check when collecting PHI uses rather than
            after optimizing processing of the candidate defs.

            * gcc.dg/torture/pr107407.c: New testcase.

    (cherry picked from commit 031a400e49d8db156c43f9ec0b21ab0c2aee8c6d)

[Bug tree-optimization/107407] [12 Regression] Wrong code at -Os on x86_64-linux-gnu since r12-383-g32955416d8040b1f

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
      Known to work|                            |12.2.1
             Status|ASSIGNED                    |RESOLVED

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.