[Bug rtl-optimization/64756] New: wrong code at -O3 on x86_64-linux-gnu (in 32-bit mode)

[Bug rtl-optimization/64756] New: wrong code at -O3 on x86_64-linux-gnu (in 32-bit mode)

[su at cs dot ucdavis.edu]

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

            Bug ID: 64756
           Summary: wrong code at -O3 on x86_64-linux-gnu (in 32-bit mode)
           Product: gcc
           Version: 5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: su at cs dot ucdavis.edu

The current gcc trunk miscompiles the following code on x86_64-linux at -O3 in
the 32-bit mode (but not in 64-bit mode). 

This is a regression from 4.9.x. 

It is quite strange that the bug seems to be sensitive to the name of the
variable "tmp" --- changing the variable to "b" for example will mask it. 

$ gcc-trunk -v
Using built-in specs.
COLLECT_GCC=gcc-trunk
COLLECT_LTO_WRAPPER=/usr/local/gcc-trunk/libexec/gcc/x86_64-unknown-linux-gnu/5.0.0/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with: ../gcc-trunk/configure --prefix=/usr/local/gcc-trunk
--enable-languages=c,c++ --disable-werror --enable-multilib
Thread model: posix
gcc version 5.0.0 20150122 (experimental) [trunk revision 220011] (GCC) 

$ 
$ gcc-trunk -m32 -O2 small.c; a.out
$ gcc-trunk -m64 -O3 small.c; a.out
$ gcc-4.9 -m32 -O3 small.c; a.out
$ 
$ gcc-trunk -m32 -O3 small.c 
$ ./a.out
Aborted (core dumped)
$ 


---------------------------------



int a, *tmp, **c = &tmp;
volatile int d;
static int *volatile *e = &tmp;
unsigned int f; 

static void
fn1 (int *p)
{
  int g;
  for (; f < 1; f++)
    for (g = 1; g >= 0; g--)
      {
    d || d;
    *c = p;

    if (tmp != &a)
      __builtin_abort (); 

    *e = 0;
      }
}

int
main ()
{
  fn1 (&a);
  return 0;
}

[Bug rtl-optimization/64756] wrong code at -O3 on x86_64-linux-gnu (in 32-bit mode)

[mikpelinux at gmail dot com]

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

Mikael Pettersson <mikpelinux at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mikpelinux at gmail dot com

--- Comment #1 from Mikael Pettersson <mikpelinux at gmail dot com> ---
Started with r210113, aka wide-int merge.  Still occurs with today's trunk.

[Bug rtl-optimization/64756] wrong code at -O3 on x86_64-linux-gnu (in 32-bit mode)

[mikpelinux at gmail dot com]

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

--- Comment #2 from Mikael Pettersson <mikpelinux at gmail dot com> ---
Also fails on m68k, but not on powerpc64, sparc64, or ARMv5.

[Bug rtl-optimization/64756] [5 Regression] wrong code at -O3 on x86_64-linux-gnu (in 32-bit mode)

[hjl.tools at gmail dot com]

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

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1
   Target Milestone|---                         |5.0
            Summary|wrong code at -O3 on        |[5 Regression] wrong code
                   |x86_64-linux-gnu (in 32-bit |at -O3 on x86_64-linux-gnu
                   |mode)                       |(in 32-bit mode)

[Bug rtl-optimization/64756] [5 Regression] wrong code at -O3 on x86_64-linux-gnu (in 32-bit mode)

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-02-02
     Ever confirmed|0                           |1

[Bug rtl-optimization/64756] [5 Regression] wrong code at -O3 on x86_64-linux-gnu (in 32-bit mode)

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
No idea what this has to do with wide_int, I don't see any relation
(unfortunately my r210112 and r210113 copies are without debug info at this
point).
But, it really seems to be a CSE bug to me (during cse2).
The thing is, in insn 29 tmp is stored to using volatile store:
(insn 29 27 30 7 (set (mem/v/f/c:SI (symbol_ref:SI ("tmp")  <var_decl
0x7f0ea7350cf0 tmp>) [1 MEM[(int * volatile *)&tmp]+0 S4 A32])
        (const_int 0 [0])) pr64756.c:19 90 {*movsi_internal}
     (nil))
but when the hash of dest is computed, we actually use:
      if (MEM_P (dest))
        {
#ifdef PUSH_ROUNDING
          /* Stack pushes invalidate the stack pointer.  */
          rtx addr = XEXP (dest, 0);
          if (GET_RTX_CLASS (GET_CODE (addr)) == RTX_AUTOINC
              && XEXP (addr, 0) == stack_pointer_rtx)
            invalidate (stack_pointer_rtx, VOIDmode);
#endif
          dest = fold_rtx (dest, insn);
        }

      /* Compute the hash code of the destination now,
         before the effects of this instruction are recorded,
         since the register values used in the address computation
         are those before this instruction.  */
      sets[i].dest_hash = HASH (dest, mode);
and so dest is folded first into symbol_ref ("a").  That means when computing
hash we do not get do_not_record flag set, which we would otherwise get because
the memory is volatile, nor we do get the hash_arg_in_memory flag set.
Next we recompute the hash, but do not look at do_not_record anymore:
      if (sets[i].rtl != 0 && dest != SET_DEST (sets[i].rtl))
        sets[i].dest_hash = HASH (SET_DEST (sets[i].rtl), mode);
Later on we insert this mem/v/f/c into the hash table and set the in_memory
flag (correctly):
        elt->in_memory = (MEM_P (sets[i].inner_dest)
                          && !MEM_READONLY_P (sets[i].inner_dest));
Later on merge_equiv_classes is called, and that one obviously doesn't count
with the option that do_not_record might be true, and when we set
do_not_record, we don't set hash_arg_in_memory, so we end up with a new elt
that for the MEM
at this time doesn't even have in_memory flag set, so when we later in insn 36
store into memory that aliases with this, we do not even invalidate it.

[Bug rtl-optimization/64756] [5 Regression] wrong code at -O3 on x86_64-linux-gnu (in 32-bit mode)

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 34643
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34643&action=edit
gcc5-pr64756.patch

I'd say we just should never record volatile MEMs into the hash table, this
patch attempts to ensure that.

[Bug rtl-optimization/64756] [5 Regression] wrong code at -O3 on x86_64-linux-gnu (in 32-bit mode)

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Tue Feb  3 20:41:38 2015
New Revision: 220377

URL: https://gcc.gnu.org/viewcvs?rev=220377&root=gcc&view=rev
Log:
    PR rtl-optimization/64756
    * cse.c (invalidate_dest): New function.
    (cse_insn): Use it.  If dest != SET_DEST (sets[i].rtl) and
    HASH (SET_DEST (sets[i].rtl), mode) computation sets do_not_record,
    invalidate and do not record it.

    * gcc.c-torture/execute/pr64756.c: New test.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr64756.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cse.c
    trunk/gcc/testsuite/ChangeLog

[Bug rtl-optimization/64756] [5 Regression] wrong code at -O3 on x86_64-linux-gnu (in 32-bit mode)

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|jakub at gcc dot gnu.org           |
         Resolution|---                         |FIXED
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.