Bug 68990 - [6 Regression] wrong code at -O3 on x86_64-pc-linux-gnu in 32-bit mode.
Summary: [6 Regression] wrong code at -O3 on x86_64-pc-linux-gnu in 32-bit mode.
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 6.0
: P1 normal
Target Milestone: 6.0
Assignee: Not yet assigned to anyone
URL:
Keywords: ra, wrong-code
Depends on:
Blocks:
 
Reported: 2015-12-18 22:57 UTC by Chengnian Sun
Modified: 2016-01-21 17:29 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2015-12-21 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Chengnian Sun 2015-12-18 22:57:41 UTC
The following code is miscompiled by the gcc trunk at -O3 in 32-bit mode on 
x86_64-pc-linux-gnu.

$: gcc-trunk -v 
Using built-in specs.
COLLECT_GCC=gcc-trunk
COLLECT_LTO_WRAPPER=/usr/local/gcc-trunk/libexec/gcc/x86_64-pc-linux-gnu/6.0.0/lto-wrapper
Target: x86_64-pc-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 6.0.0 20151218 (experimental) [trunk revision 231807] (GCC) 
$:
$: gcc-trunk -m32 small.c -w -O3 ; ./a.out
$: gcc-trunk -m32 small.c -w -O2 ; ./a.out
g_95=0
$: gcc-trunk -m32 small.c -w -O0 ; ./a.out
g_95=0
$: 
$: cat small.c
short a;
int b = 1, f;
char c, e = 1;
long long d;
static short fn1() {
    unsigned g, h = 0;
    int i = 0 || d * (b | e);
    char j = a << i, l = a;
    short k;
    int m = -b;
    if (m < b) {
        k = m = b;
        g = (k || l) / (b / e);
        if (b)
            printf("g_95=%lld\n", (long long)a);
    }
    a = b = m;
    if (j || e) {
        h = g;
        i = m;
        g = j * k / (i - d);
        if (m)
            b = j && b;
        e = b * (h & d) || g;
    }
    b = i;
    char n = e || h | d;
    e = i < d & k / n;
    return f;
}

int main() {
    if (fn1())
        if (c)
lbl_2043:
            goto lbl_2043;
    return 0;
}
Comment 1 Marek Polacek 2015-12-21 11:46:41 UTC
Confirmed, let me bisect.
Comment 2 Marek Polacek 2015-12-21 12:16:31 UTC
Note you may need -march=x86-64 to reproduce.

Started with r228231:

commit 3ef16338b7f4970607d1d0b9c8228bfe3aeeca94
Author: ienkovich <ienkovich@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Tue Sep 29 09:32:40 2015 +0000

    gcc/
    
        PR target/65105
        * config/i386/i386.c: Include dbgcnt.h.
...
Comment 3 Jakub Jelinek 2015-12-21 14:42:57 UTC
Looks like a RA issue to me.
At least, on -m32 -O3 -march=x86-64 compiled:
short a;
int b = 1, f;
char c, e = 1;
long long d;

static short
foo ()
{
  unsigned g, h = 0;
  int i = 0 || d * (b | e);
  char j = a << i, l = a;
  short k;
  int m = -b;
  if (m < b)
    {
      k = m = b;
      g = (k || l) / (b / e);
      if (b)
	__builtin_printf ("foo=%lld\n", (long long) a);
    }
  a = b = m;
  if (j || e)
    {
      h = g;
      i = m;
      g = j * k / (i - d);
      if (m)
	b = j && b;
      e = b * (h & d) || g;
    }
  b = i;
  char n = e || h | d;
  e = i < d & k / n;
  return f;
}

int
main ()
{
  if (foo ())
    if (c)
     lab:
      goto lab;
  return 0;
}

we in *.ira still have correct:
(insn 30 28 31 2 (parallel [
            (set (reg/v:SI 103 [ m ])
                (neg:SI (reg:SI 91 [ b.2_7 ])))
            (clobber (reg:CC 17 flags))
        ]) pr68990-4.c:13 463 {*negsi2_1}
     (expr_list:REG_UNUSED (reg:CC 17 flags)
        (nil)))
(insn 31 30 32 2 (set (reg:CCGC 17 flags)
        (compare:CCGC (reg:SI 91 [ b.2_7 ])
            (reg/v:SI 103 [ m ]))) pr68990-4.c:14 7 {*cmpsi_1}
     (nil))
(jump_insn 32 31 33 2 (set (pc)
        (if_then_else (le (reg:CCGC 17 flags)
                (const_int 0 [0]))
            (label_ref 56)
            (pc))) pr68990-4.c:14 631 {*jcc_1}
     (expr_list:REG_DEAD (reg:CCGC 17 flags)
        (int_list:REG_BR_PROB 5000 (nil)))
and
31:r91  l0   mem
13:r103 l0     3
but LRA turns that into:
(insn 256 207 208 2 (set (reg:SI 1 dx [orig:91 b.2_7 ] [91])
        (mem/c:SI (plus:SI (reg/f:SI 6 bp)
                (const_int -28 [0xffffffffffffffe4])) [5 %sfp+-4 S4 A32])) pr68990-4.c:13 86 {*movsi_internal}
     (nil))
(insn 208 256 30 2 (set (reg/v:SI 3 bx [orig:103 m ] [103])
        (reg:SI 1 dx [orig:91 b.2_7 ] [91])) pr68990-4.c:13 86 {*movsi_internal}
     (nil))
(insn 30 208 286 2 (parallel [
            (set (reg/v:SI 3 bx [orig:103 m ] [103])
                (neg:SI (reg/v:SI 3 bx [orig:103 m ] [103])))
            (clobber (reg:CC 17 flags))
        ]) pr68990-4.c:13 463 {*negsi2_1}
     (nil))
(insn 286 30 210 2 (set (mem/c:SI (plus:SI (reg/f:SI 6 bp)
                (const_int -28 [0xffffffffffffffe4])) [5 %sfp+-4 S4 A32])
        (reg:SI 3 bx [orig:91 b.2_7 ] [91])) pr68990-4.c:13 86 {*movsi_internal}
     (nil))
(insn 210 286 31 2 (set (reg:SI 3 bx [orig:91 b.2_7 ] [91])
        (reg:SI 1 dx [orig:91 b.2_7 ] [91])) pr68990-4.c:14 86 {*movsi_internal}
     (nil))
(insn 31 210 32 2 (set (reg:CCGC 17 flags)
        (compare:CCGC (reg:SI 3 bx [orig:91 b.2_7 ] [91])
            (reg:SI 3 bx [orig:91 b.2_7 ] [91]))) pr68990-4.c:14 7 {*cmpsi_1}
     (nil))
(jump_insn 32 31 33 2 (set (pc)
        (if_then_else (le (reg:CCGC 17 flags)
                (const_int 0 [0]))
            (label_ref 56)
            (pc))) pr68990-4.c:14 631 {*jcc_1}
     (int_list:REG_BR_PROB 5000 (nil))
 -> 56)
which is just wrong, instead of comparing m < b it suddenly compares b < b.
Comment 4 Jakub Jelinek 2015-12-21 14:58:44 UTC
I think the problem is:
********** Pseudos coalescing #1: **********

  Coalescing move 5:r91(91)-r103(103) (freq=1)
         Removing move 5 (freq=1)
deleting insn with uid = 5.
         Make unique value for inheritance r256
         Make unique value for inheritance r257
         Make unique value for inheritance r258
         Make unique value for inheritance r259
         Make unique value for inheritance r260
         Make unique value for inheritance r262
         Make unique value for inheritance r264
Coalesced Moves = 1

in the other spot where insn 5 is it is perhaps possible to coalesce the two pseudos, but not at the
  int m = -b;
  if (m < b)
comparison, where it breaks it.
Comment 5 Jakub Jelinek 2015-12-21 15:03:49 UTC
BTW, with -mno-stv -O3 the testcase passes, and while the *.stv pass actually doesn't do anything at all on this testcase, that switch already changes expansion in a couple of places so no wonder that the RA makes different decisions.

And, I haven't managed to create a testcase that would abort vs. return 0 from main for good vs. bad, even other fail defining a stdarg function like printf didn't help.  Do, unless somebody comes up with one, guess we'll need a dg-output directive and try to match what is printed (without the newline in there).
Comment 6 Vladimir Makarov 2016-01-19 19:08:55 UTC
(In reply to Jakub Jelinek from comment #4)
> I think the problem is:
> ********** Pseudos coalescing #1: **********
> 
>   Coalescing move 5:r91(91)-r103(103) (freq=1)
>          Removing move 5 (freq=1)
> deleting insn with uid = 5.
>          Make unique value for inheritance r256
>          Make unique value for inheritance r257
>          Make unique value for inheritance r258
>          Make unique value for inheritance r259
>          Make unique value for inheritance r260
>          Make unique value for inheritance r262
>          Make unique value for inheritance r264
> Coalesced Moves = 1
> 
> in the other spot where insn 5 is it is perhaps possible to coalesce the two
> pseudos, but not at the
>   int m = -b;
>   if (m < b)
> comparison, where it breaks it.

I've reproduced the bug and checked the coalescing.   The coalescing seems the right transformation as p91 becomes dead right before insn#30 and actually its value before insn#30 through an inheritance pseudo is used in insn #31.  The problem actually occurs later in pseudo assignment sub-pass.

I suspect that something is wrong with pseudo values.  Changes affecting pseudo values much affect the performance much.  So when I have the patch, I need to check the performance impact too.  I think if it is everything ok, the patch will be ready at the end of week.
Comment 7 Vladimir Makarov 2016-01-21 16:01:54 UTC
Author: vmakarov
Date: Thu Jan 21 16:01:22 2016
New Revision: 232679

URL: https://gcc.gnu.org/viewcvs?rev=232679&root=gcc&view=rev
Log:
2016-01-21  Vladimir Makarov  <vmakarov@redhat.com>

	PR rtl-optimization/68990
	* lra-coalesce.c (lra_coalesce): Invalidate value for the result
	pseudo instead of inheritance ones.

2016-01-21  Vladimir Makarov  <vmakarov@redhat.com>

	PR rtl-optimization/68990
	* gcc.target/i386/pr68990: New.



Added:
    trunk/gcc/testsuite/gcc.target/i386/pr68990.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/lra-coalesce.c
    trunk/gcc/testsuite/ChangeLog
Comment 8 Jeffrey A. Law 2016-01-21 17:29:56 UTC
Fixed by Vlad's commit on the trunk.