[Bug tree-optimization/59747] New: wrong code at -Os and above on x86_64-linux-gnu in 64-bit mode

[Bug tree-optimization/59747] New: wrong code at -Os and above on x86_64-linux-gnu in 64-bit mode

[su at cs dot ucdavis.edu]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59747

            Bug ID: 59747
           Summary: wrong code at -Os and above on x86_64-linux-gnu in
                    64-bit mode
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-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 -Os and
above in 64-bit mode (but not 32-bit). 

This is a regression from 4.8.x. 

$ 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/4.9.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 4.9.0 20140109 (experimental) [trunk revision 206472] (GCC) 
$ 
$ gcc-trunk -m64 -O1 small.c; a.out
0
$ gcc-trunk -m32 -Os small.c; a.out
0
$ gcc-4.8 -m64 -Os small.c; a.out
0
$ 
$ gcc-trunk -m64 -Os small.c; a.out
1
$ 


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


int printf (const char *, ...);

int a[6], b, c = 1, d;
short e;

void
fn1 (int p)
{
  b = a[p];
}

int
main ()
{
  a[0] = 1;
  if (c)
    e--;
  d = e;
  long long f = e;
  fn1 ((f >> 56) & 1);
  printf ("%d\n", b);
  return 0;
}

[Bug tree-optimization/59747] [4.9 Regression] wrong code at -Os and above on x86_64-linux-gnu in 64-bit mode

[mpolacek at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59747

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2014-01-10
                 CC|                            |mpolacek at gcc dot gnu.org
   Target Milestone|---                         |4.9.0
            Summary|wrong code at -Os and above |[4.9 Regression] wrong code
                   |on x86_64-linux-gnu in      |at -Os and above on
                   |64-bit mode                 |x86_64-linux-gnu in 64-bit
                   |                            |mode
     Ever confirmed|0                           |1

--- Comment #1 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Confirmed.

[Bug tree-optimization/59747] [4.9 Regression] wrong code at -Os and above on x86_64-linux-gnu in 64-bit mode

[mpolacek at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59747

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

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

[Bug tree-optimization/59747] [4.9 Regression] wrong code at -Os and above on x86_64-linux-gnu in 64-bit mode

[mpolacek at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59747

--- Comment #2 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
I think the problem is in setting the value of e.  With -O in .gimple we have:
    e.2 = e;
    e.3 = e.2;   
    e.4 = e.3 + -1;
    e = e.4;
while with -O2:
    e.2 = e;
    e.3 = e.2;
    e.4 = (unsigned short) e.3;
    D.1762 = e.4 + 65535;
    e.5 = (short int) D.1762;
    e = e.5;

Wrt the weirdo shift: -1 >> 56 is implementation-defined behavior.

[Bug tree-optimization/59747] [4.9 Regression] wrong code at -Os and above on x86_64-linux-gnu in 64-bit mode

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59747

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

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with r206418.

[Bug tree-optimization/59747] [4.9 Regression] wrong code at -Os and above on x86_64-linux-gnu in 64-bit mode

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59747

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Marek Polacek from comment #2)
> I think the problem is in setting the value of e.  With -O in .gimple we
> have:
>     e.2 = e;
>     e.3 = e.2;   
>     e.4 = e.3 + -1;
>     e = e.4;
> while with -O2:
>     e.2 = e;
>     e.3 = e.2;
>     e.4 = (unsigned short) e.3;
>     D.1762 = e.4 + 65535;
>     e.5 = (short int) D.1762;
>     e = e.5;

That's -fstrict-overflows effect I think, but not the issue.  "Fixed" by
-fno-ree.

[Bug tree-optimization/59747] [4.9 Regression] wrong code at -Os and above on x86_64-linux-gnu in 64-bit mode

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59747

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So, before ree we have:
(insn 13 12 14 4 (set (reg:HI 0 ax [orig:88 D.1782 ] [88])
        (mem/c:HI (symbol_ref:DI ("e")  <var_decl 0x7fbcde42c260 e>) [3 e+0 S2
A16])) pr59747.c:18 91 {*movhi_internal}
     (expr_list:REG_EQUIV (mem/c:HI (symbol_ref:DI ("e")  <var_decl
0x7fbcde42c260 e>) [3 e+0 S2 A16])
        (nil)))
(insn 14 13 15 4 (set (reg:SI 1 dx [orig:97 D.1782 ] [97])
        (sign_extend:SI (reg:HI 0 ax [orig:88 D.1782 ] [88]))) pr59747.c:18 145
{extendhisi2}
     (expr_list:REG_EQUIV (mem/c:SI (symbol_ref:DI ("d")  <var_decl
0x7fbcde42c1c8 d>) [2 d+0 S4 A32])
        (nil)))
(insn 15 14 17 4 (set (mem/c:SI (symbol_ref:DI ("d")  <var_decl 0x7fbcde42c1c8
d>) [2 d+0 S4 A32])
        (reg:SI 1 dx [orig:97 D.1782 ] [97])) pr59747.c:18 90 {*movsi_internal}
     (nil))
(insn 17 15 18 4 (set (reg:DI 0 ax [orig:99 f ] [99])
        (sign_extend:DI (reg:HI 0 ax [orig:88 D.1782 ] [88]))) pr59747.c:19 144
{extendhidi2}
     (nil))

Before Jeff's ree changes, we'd transform it into:
(insn 13 12 14 4 (set (reg:DI 0 ax)
        (sign_extend:DI (mem/c:HI (symbol_ref:DI ("e")  <var_decl
0x7fbcde42c260 e>) [3 e+0 S2 A16]))) pr59747.c:18 144 {extendhidi2}
     (expr_list:REG_EQUIV (mem/c:HI (symbol_ref:DI ("e")  <var_decl
0x7fbcde42c260 e>) [3 e+0 S2 A16])
        (nil)))
(insn 14 13 15 4 (set (reg:SI 1 dx [orig:97 D.1782 ] [97])
        (sign_extend:SI (reg:HI 0 ax [orig:88 D.1782 ] [88]))) pr59747.c:18 145
{extendhisi2}
     (expr_list:REG_EQUIV (mem/c:SI (symbol_ref:DI ("d")  <var_decl
0x7fbcde42c1c8 d>) [2 d+0 S4 A32])
        (nil)))
(insn 15 14 18 4 (set (mem/c:SI (symbol_ref:DI ("d")  <var_decl 0x7fbcde42c1c8
d>) [2 d+0 S4 A32])
        (reg:SI 1 dx [orig:97 D.1782 ] [97])) pr59747.c:18 90 {*movsi_internal}
     (nil))

which is of course suboptimal, now we transform it into:
(insn 13 12 39 4 (set (reg:DI 1 dx)
        (sign_extend:DI (mem/c:HI (symbol_ref:DI ("e")  <var_decl
0x7f76d96ba260 e>) [3 e+0 S2 A16]))) pr59747.c:18 144 {extendhidi2}
     (expr_list:REG_EQUIV (mem/c:HI (symbol_ref:DI ("e")  <var_decl
0x7f76d96ba260 e>) [3 e+0 S2 A16])
        (nil)))
(insn 39 13 15 4 (set (reg:SI 0 ax)
        (reg:SI 1 dx [orig:97 D.1782 ] [97])) pr59747.c:18 -1
     (nil))
(insn 15 39 18 4 (set (mem/c:SI (symbol_ref:DI ("d")  <var_decl 0x7f76d96ba1c8
d>) [2 d+0 S4 A32])
        (reg:SI 1 dx [orig:97 D.1782 ] [97])) pr59747.c:18 90 {*movsi_internal}
     (nil))

which is wrong, insn 39 move would need to be performed in DImode rather than
SImode.  Or alternatively in this case if insn 13 set DImode %rax as it used to
do, it would be sufficient to do a SImode %edx = %eax move in insn 39.

Jeff, can you please have a look?

[Bug tree-optimization/59747] [4.9 Regression] wrong code at -Os and above on x86_64-linux-gnu in 64-bit mode

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59747

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The actual REGNO != REGNO transformation is correct, the problem is that the
second extension is to a wider mode and while handling that we change the
destination mode on the def_insn to an even wider mode.  The code just assumes
that the def insn sets the right REGNO rather than some other.  So, I think it
shouldn't be hard to detect that case.  Then we either need to ensure the
corresponding copy insn from the first extension will be adjusted to the wider
mode, or for this case swap back the registers (revert to the original REGNO
for the def insn and put the swap insn the other way around), or do it the
other way around always (then no special casing would be needed; Jeff, what was
the reason why you haven't handled
(set (reg1) (expression))
(set (reg2) (any_extend (reg1))
as
(set (reg1) (any_extend (expression)))
(set (reg2) (reg1))
but instead
(set (reg2) (any_extend (expression)))
(set (reg1) (reg2))
?).

[Bug tree-optimization/59747] [4.9 Regression] wrong code at -Os and above on x86_64-linux-gnu in 64-bit mode

[law at redhat dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59747

Jeffrey A. Law <law at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |law at redhat dot com
           Assignee|unassigned at gcc dot gnu.org      |law at redhat dot com

[Bug tree-optimization/59747] [4.9 Regression] wrong code at -Os and above on x86_64-linux-gnu in 64-bit mode

[law at redhat dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59747

--- Comment #7 from Jeffrey A. Law <law at redhat dot com> ---
In response to your question in c#6, if you use the most obvious form:

ORIG:
(set (reg1) (expression))
(set (reg2) (any_extend (reg1))
TRANSFORMED:
(set (reg1) (any_extend (expression)))
(set (reg2) (reg1))

Then the copy isn't typically propagated away and we don't actually improve the
code. If we look at the original testcase, the obvious form would look
something like this:

(insn 9 5 24 2 (set (reg:DI 0 ax)
        (sign_extend:DI (mem:HI (plus:DI (mult:DI (reg/v:DI 1 dx [orig:91 rdx ]
[91])
                        (const_int 2 [0x2]))
                    (symbol_ref:DI ("code")  <var_decl 0x7f48935f01c8 code>))
[2 code S2 A16]))) l.c:13 144 {extendhidi2}
     (nil))
(insn 24 9 12 2 (set (reg/v:DI 1 dx [orig:84 rdx ] [84])
        (reg:DI 0 ax)) l.c:13 -1
     (nil))
(note 12 24 13 2 NOTE_INSN_DELETED)
(insn 13 12 14 2 (set (reg:DI 0 ax [orig:95 D.1797 ] [95])
        (zero_extend:DI (reg:QI 0 ax [orig:83 D.1796 ] [83]))) l.c:16 133
{zero_extendqidi2}
     (nil))
(insn 14 13 15 2 (parallel [
            (set (reg:DI 1 dx [orig:96 rdx ] [96])
                (ashiftrt:DI (reg/v:DI 1 dx [orig:84 rdx ] [84])
                    (const_int 8 [0x8])))
            (clobber (reg:CC 17 flags))
        ]) l.c:15 563 {*ashrdi3_1}
     (nil))

To get any improvement we must be able to propagate insn24 away and that's
nontrivial to do in that form.

[Bug tree-optimization/59747] [4.9 Regression] wrong code at -Os and above on x86_64-linux-gnu in 64-bit mode

[law at redhat dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59747

--- Comment #8 from Jeffrey A. Law <law at redhat dot com> ---
This looks pretty easy to fix as we emit the copies.

Basically we had two extensions reached by the same def.  Elimination of the
first extension requires a copy.  Elimination of the second does not.  The
second extension is wider than the first.

All looks good after elimination of the first extension.  Elimination of the
second extension looks good as well -- basically we just widen the extension
that'll be done as part of the def insn.  However, when we do that we need to
widen the copy generated when we eliminated the first extension.

Thankfully we have the defining insn handy when we generate the copy (they're
not generated until after the defining insns are all munged).  So it's just a
matter of generating the copy in the wider mode.

I want to look at a couple things, but right now I expect this'll be wrapped up
shortly.

[Bug tree-optimization/59747] [4.9 Regression] wrong code at -Os and above on x86_64-linux-gnu in 64-bit mode

[trippels at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59747

Markus Trippelsdorf <trippels at gcc dot gnu.org> changed:

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

--- Comment #9 from Markus Trippelsdorf <trippels at gcc dot gnu.org> ---
*** Bug 59824 has been marked as a duplicate of this bug. ***

[Bug tree-optimization/59747] [4.9 Regression] wrong code at -Os and above on x86_64-linux-gnu in 64-bit mode

[law at redhat dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59747

Jeffrey A. Law <law at redhat dot com> changed:

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

--- Comment #11 from Jeffrey A. Law <law at redhat dot com> ---
Fixed on trunk.

[Bug tree-optimization/59747] [4.9 Regression] wrong code at -Os and above on x86_64-linux-gnu in 64-bit mode

[law at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59747

--- Comment #10 from Jeffrey A. Law <law at gcc dot gnu.org> ---
Author: law
Date: Wed Jan 15 18:13:52 2014
New Revision: 206638

URL: http://gcc.gnu.org/viewcvs?rev=206638&root=gcc&view=rev
Log:
    PR tree-optimization/59747
    * ree.c (find_and_remove_re): Properly handle case where a second
    eliminated extension requires widening a copy created for elimination
    of a prior extension.
    (combine_set_extension): Ensure that the number of hard regs needed
    for a destination register does not change when we widen it.

    PR tree-optimization/59747
    * gcc.c-torture/execute/pr59747.c: New test.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr59747.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ree.c
    trunk/gcc/testsuite/ChangeLog

[Bug tree-optimization/59747] [4.9 Regression] wrong code at -Os and above on x86_64-linux-gnu in 64-bit mode

[dominiq at lps dot ens.fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59747

Dominique d'Humieres <dominiq at lps dot ens.fr> changed:

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

--- Comment #12 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
On x86_64-apple-darwin13 at r206658, configured with

 ../p_work/configure --prefix=/opt/gcc/gcc4.9p-206658p1a
--enable-languages=c,c++,lto,fortran,ada,objc,obj-c++ --with-gmp=/opt/mp
--with-system-zlib --enable-checking=release --with-isl=/opt/mp --enable-lto
--enable-plugin --with-arch=corei7 --with-cpu=corei7

I get for -m32 and -m64:

FAIL: gcc.c-torture/execute/pr59747.c execution,  -O0 
FAIL: gcc.c-torture/execute/pr59747.c execution,  -O1 
FAIL: gcc.c-torture/execute/pr59747.c execution,  -O2 
FAIL: gcc.c-torture/execute/pr59747.c execution,  -O3 -fomit-frame-pointer 
FAIL: gcc.c-torture/execute/pr59747.c execution,  -O3 -g 
FAIL: gcc.c-torture/execute/pr59747.c execution,  -Os 
FAIL: gcc.c-torture/execute/pr59747.c execution,  -Og -g 
FAIL: gcc.c-torture/execute/pr59747.c execution,  -O2 -flto
-flto-partition=none 
FAIL: gcc.c-torture/execute/pr59747.c execution,  -O2 -flto 

I'll attach the preprocessed and assembly files compiled without option. Note
that I get the same files when compiling with gcc r206567 and the test still
fails with the changes at r206659.

[Bug tree-optimization/59747] [4.9 Regression] wrong code at -Os and above on x86_64-linux-gnu in 64-bit mode

[dominiq at lps dot ens.fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59747

--- Comment #13 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
Created attachment 31859
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=31859&action=edit
preprocessed file for gcc.c-torture/execute/pr59747.c on darwin13

[Bug tree-optimization/59747] [4.9 Regression] wrong code at -Os and above on x86_64-linux-gnu in 64-bit mode

[dominiq at lps dot ens.fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59747

--- Comment #14 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
Created attachment 31860
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=31860&action=edit
assembly file for gcc.c-torture/execute/pr59747.c on darwin

[Bug tree-optimization/59747] [4.9 Regression] wrong code at -Os and above on x86_64-linux-gnu in 64-bit mode

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59747

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

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

--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
You don't have the r206659 fix in your tree, do you?

[Bug tree-optimization/59747] [4.9 Regression] wrong code at -Os and above on x86_64-linux-gnu in 64-bit mode

[dominiq at lps dot ens.fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59747

--- Comment #16 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
> You don't have the r206659 fix in your tree, do you?

No. I thought to have tested it as said at the end of comment 12, but it seems
that I did not actually do it. Importing r206659 in my working tree made the
test succeeds.

Sorry for the noise.