[Bug tree-optimization/87132] New: Gcc miscompiles at -O2 on valid code

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/87132] New: Gcc miscompiles at -O2 on valid code

marxin at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87132

            Bug ID: 87132
           Summary: Gcc miscompiles at -O2 on valid code
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: helloqirun at gmail dot com
  Target Milestone: ---

It seems to be a recent regression starting from r263875 (need double check).


$ gcc-trunk -v
Using built-in specs.
COLLECT_GCC=gcc-trunk
COLLECT_LTO_WRAPPER=/home/absozero/trunk/root-gcc/libexec/gcc/x86_64-pc-linux-gnu/9.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../gcc/configure --prefix=/home/absozero/trunk/root-gcc
--enable-languages=c,c++ --disable-werror --enable-multilib
Thread model: posix
gcc version 9.0.0 20180828 (experimental) [trunk revision 263917] (GCC)




$ gcc-trunk abc.c ; ./a.out
0
$ gcc-trunk -O2 abc.c ; ./a.out
4


$ cat abc.c
int printf(const char *, ...);
int a, b;
short c;
char d;
int main() {
  int e[] = {4, 4, 4, 4, 4, 4, 4, 4, 4};
  d = 8;
  for (; d; d--) {
    a = 0;
    for (; a <= 8; a++) {
      c = e[1];
      e[d] = b;
    }
  }
  printf("%d\n", c);
}
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/87132] [9 Regression] Gcc miscompiles at -O2 on valid code

marxin at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87132

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
            Version|unknown                     |9.0
           Keywords|                            |wrong-code
   Last reconfirmed|                            |2018-08-29
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org
     Ever confirmed|0                           |1
            Summary|Gcc miscompiles at -O2 on   |[9 Regression] Gcc
                   |valid code                  |miscompiles at -O2 on valid
                   |                            |code
   Target Milestone|---                         |9.0

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Mine.
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/87132] [9 Regression] Gcc miscompiles at -O2 on valid code

marxin at gcc dot gnu.org
In reply to this post by marxin at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87132

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Simplified:

extern void abort (void);
int c, d;
int main()
{
  int e[] = {4, 4, 4, 4, 4, 4, 4, 4, 4};
  d = 8;
  for (; d; d--)
    for (int a = 0; a <= 8; a++)
      {
        c = e[1];
        e[d] = 0;
      }
  if (c != 0)
    abort ();
  return 0;
}

this one is going to be interesting to fix.  The issue is that we either
stop iterating too early or we over-eagerly use values from the previous
iteration when walk_non_aliased_vuses skips over e[d] = 0 when looking
for aliasing defs to e[1].  d still has the previous iteration value of 8
because we didn't yet re-visit the load of it when walking defs across
the backedge.

So this is

static void *
vn_reference_lookup_3 (ao_ref *ref, tree vuse, void *vr_,
                       bool *disambiguate_only)
{
...
  /* First try to disambiguate after value-replacing in the definitions LHS.
*/
  if (is_gimple_assign (def_stmt))
    {
...
      lhs_ops = valueize_refs_1 (lhs_ops, &valueized_anything, true);

as well as the load from e[1] in the first iteration not needing to
visit e[d] because of the not yet executable backedge and thus the
virtual PHI degenerating to the entry value.

A fix could be somehow tracking memory state changes and iterating once more
or avoiding last-iteration values in valueization (or valueization completely
when performing the walk over backedges).  The latter latter is easiest,
testing that for fallout.

Index: gcc/tree-ssa-alias.c
===================================================================
--- gcc/tree-ssa-alias.c        (revision 263944)
+++ gcc/tree-ssa-alias.c        (working copy)
@@ -2722,7 +2722,14 @@ next:;
       if (arg1 == arg0)
        ;
       else if (! maybe_skip_until (phi, arg0, ref, arg1, cnt, visited,
-                                  abort_on_visited, translate, data))
+                                  abort_on_visited,
+                                  /* Do not translate when walking over
+                                     backedges.  */
+                                  dominated_by_p
+                                    (CDI_DOMINATORS,
+                                     gimple_bb (SSA_NAME_DEF_STMT (arg1)),
+                                     phi_bb)
+                                  ? NULL : translate, data))
        return NULL_TREE;
     }
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/87132] [9 Regression] Gcc miscompiles at -O2 on valid code

marxin at gcc dot gnu.org
In reply to this post by marxin at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87132

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
Author: rguenth
Date: Wed Aug 29 14:13:20 2018
New Revision: 263959

URL: https://gcc.gnu.org/viewcvs?rev=263959&root=gcc&view=rev
Log:
2018-08-29  Richard Biener  <[hidden email]>

        PR tree-optimization/87132
        * tree-ssa-alias.c (get_continuation_for_phi): Do not translate
        when skipping defs reachable over backedges.

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

Added:
    trunk/gcc/testsuite/gcc.dg/torture/pr87132.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-alias.c
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/87132] [9 Regression] Gcc miscompiles at -O2 on valid code

marxin at gcc dot gnu.org
In reply to this post by marxin at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87132

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

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/87132] [9 Regression] Gcc miscompiles at -O2 on valid code

marxin at gcc dot gnu.org
In reply to this post by marxin at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87132

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
Author: rguenth
Date: Mon Sep 16 11:58:35 2019
New Revision: 275747

URL: https://gcc.gnu.org/viewcvs?rev=275747&root=gcc&view=rev
Log:
2019-09-16  Richard Biener  <[hidden email]>

        PR tree-optimization/91756
        PR tree-optimization/87132
        * tree-ssa-alias.h (enum translate_flags): New.
        (get_continuation_for_phi): Use it instead of simple bool flag.
        (walk_non_aliased_vuses): Likewise.
        * tree-ssa-alias.c (maybe_skip_until): Adjust.
        (get_continuation_for_phi): When looking across backedges only
        disallow valueization.
        (walk_non_aliased_vuses): Adjust.
        * tree-ssa-sccvn.c (vn_reference_lookup_3): Avoid valueization
        if requested.

        * gcc.dg/tree-ssa/ssa-fre-81.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-81.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-alias.c
    trunk/gcc/tree-ssa-alias.h
    trunk/gcc/tree-ssa-sccvn.c