Bug 44858 - [4.5/4.6 Regression] likely integer wrong code bug
Summary: [4.5/4.6 Regression] likely integer wrong code bug
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 4.5.0
: P2 normal
Target Milestone: 4.5.2
Assignee: Jakub Jelinek
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-07-07 14:12 UTC by John Regehr
Modified: 2010-08-25 21:27 UTC (History)
4 users (show)

See Also:
Host: i686-pc-linux-gnu
Target: i686-pc-linux-gnu
Build: i686-pc-linux-gnu
Known to work:
Known to fail:
Last reconfirmed: 2010-07-08 08:14:08


Attachments
gcc46-vrp.patch (1.05 KB, patch)
2010-07-08 14:44 UTC, Jakub Jelinek
Details | Diff
gcc46-pr44858.patch (822 bytes, patch)
2010-08-25 13:25 UTC, Jakub Jelinek
Details | Diff
gcc46-pr44858-2.patch (1023 bytes, patch)
2010-08-25 14:01 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Regehr 2010-07-07 14:12:06 UTC
regehr@john-home:~$ current-gcc -O0 small.c -o small
regehr@john-home:~$ ./small
checksum g_610 = 1
regehr@john-home:~$ current-gcc -O1 small.c -o small
regehr@john-home:~$ ./small
checksum g_610 = 0
regehr@john-home:~$ current-gcc -v
Using built-in specs.
COLLECT_GCC=current-gcc
COLLECT_LTO_WRAPPER=/home/regehr/z/compiler-install/gcc-r161813-install/libexec/gcc/i686-pc-linux-gnu/4.6.0/lto-wrapper
Target: i686-pc-linux-gnu
Configured with: ../configure --with-libelf=/usr/local --enable-lto --prefix=/home/regehr/z/compiler-install/gcc-r161813-install --program-prefix=r161813- --enable-languages=c,c++
Thread model: posix
gcc version 4.6.0 20100704 (experimental) (GCC) 
regehr@john-home:~$ cat small.c
extern int printf (__const char *__restrict __format, ...);

int g_33 = 3;
int g_610 = 1;

long long foo(int i1, int i2)
{
    return (i1 / i2);
}

int main(void)
{
    int l_2 = 2;
    l_2 &= foo(1, g_610) > g_610;
    g_610 = (g_33 != 0) | l_2;
    printf("checksum g_610 = %d\n", g_610);
    return l_2;
}
Comment 1 Jakub Jelinek 2010-07-08 08:37:35 UTC
Things go wrong in the combiner:

(insn 21 42 22 6 pr44858.c:16 (parallel [
            (set (reg/v:SI 62 [ c ])
                (and:SI (reg:SI 68)
                    (const_int 2 [0x2])))
            (clobber (reg:CC 17 flags))
        ]) 375 {*andsi_1} (expr_list:REG_DEAD (reg:SI 68)
        (expr_list:REG_UNUSED (reg:CC 17 flags)
            (nil))))

(insn 22 21 23 6 pr44858.c:17 (set (reg:CCZ 17 flags)
        (compare:CCZ (mem/c/i:SI (symbol_ref:SI ("a") [flags 0x2]  <var_decl 0x7f8d06f05000 a>) [0 a+0 S4 A32])
            (const_int 0 [0]))) 2 {*cmpsi_ccno_1} (nil))

(insn 23 22 24 6 pr44858.c:17 (set (reg:QI 70)
        (ne:QI (reg:CCZ 17 flags)
            (const_int 0 [0]))) 570 {*setcc_qi} (expr_list:REG_DEAD (reg:CCZ 17 flags)
        (nil)))

(insn 24 23 25 6 pr44858.c:17 (parallel [
            (set (reg:SI 69)
                (zero_extend:SI (reg:QI 70)))
            (clobber (reg:CC 17 flags))
        ]) 119 {*zero_extendqisi2_movzbl_and} (expr_list:REG_DEAD (reg:QI 70)
        (expr_list:REG_UNUSED (reg:CC 17 flags)
            (nil))))

(insn 25 24 26 6 pr44858.c:17 (parallel [
            (set (reg:SI 65 [ b.2 ])
                (ior:SI (reg/v:SI 62 [ c ])
                    (reg:SI 69)))
            (clobber (reg:CC 17 flags))
        ]) 394 {*iorsi_1} (expr_list:REG_DEAD (reg:SI 69)
        (expr_list:REG_UNUSED (reg:CC 17 flags)
            (nil))))

changes into:

(note 21 42 22 6 NOTE_INSN_DELETED)
        
(insn 22 21 23 6 pr44858.c:17 (set (reg:CCZ 17 flags)
        (compare:CCZ (mem/c/i:SI (symbol_ref:SI ("a") [flags 0x2]  <var_decl 0x7f8d06f05000 a>) [0 a+0 S4 A32])
            (const_int 0 [0]))) 2 {*cmpsi_ccno_1} (nil))

(note 23 22 24 6 NOTE_INSN_DELETED)

(insn 24 23 25 6 pr44858.c:17 (parallel [
            (set (reg/v:SI 62 [ c ])
                (and:SI (reg:SI 68)
                    (const_int 2 [0x2])))
            (clobber (reg:CC 17 flags))
        ]) 375 {*andsi_1} (expr_list:REG_UNUSED (reg:CC 17 flags)
        (expr_list:REG_DEAD (reg:SI 68)
            (nil))))

(insn 25 24 26 6 pr44858.c:17 (set (reg:SI 65 [ b.2 ])
        (ne:SI (reg:CCZ 17 flags)
            (const_int 0 [0]))) 569 {*setcc_si_1_movzbl} (expr_list:REG_DEAD (reg:CC 17 flags)
        (nil)))
Comment 2 Jakub Jelinek 2010-07-08 09:58:49 UTC
Caused by http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=152642
Comment 3 Jakub Jelinek 2010-07-08 10:01:34 UTC
Updated testcase:

extern void abort (void);
int a = 3;
int b = 1;

__attribute__((noinline)) long long
foo (int x, int y)
{
  return x / y;
}

__attribute__((noinline)) int
bar (void)
{
  int c = 2;
  c &= foo (1, b) > b;
  b = (a != 0) | c;
  return c;
}

int
main (void)
{
  if (bar () != 0 || b != 1)
    abort ();
  return 0;
}
Comment 4 Jakub Jelinek 2010-07-08 14:44:43 UTC
Created attachment 21147 [details]
gcc46-vrp.patch

So, combiner seems to be the first to notice that c must be actually zero
(2 & (x > y)).
This patch just tries to notice it earlier, during VRP, which of course isn't a fix for this bug (which is somewhere in the combiner where it doesn't notice that when it moves the and insn it clobbers cc which is needed), just makes it latent for -O2 (still present at -O1 or -O2 -fno-tree-vrp).
Comment 5 Jakub Jelinek 2010-07-22 09:36:43 UTC
The VRP changes have been committed, so on the trunk this will be now reproduceable only with -O1 or -O2 -fno-tree-vrp.
Comment 6 Richard Biener 2010-07-31 09:29:58 UTC
GCC 4.5.1 is being released, adjusting target milestone.
Comment 7 Jakub Jelinek 2010-08-25 13:25:49 UTC
Created attachment 21560 [details]
gcc46-pr44858.patch

The combiner bug seems to be similar to PR20322, attached untested patch should fix it.
Comment 8 Jakub Jelinek 2010-08-25 14:01:20 UTC
Created attachment 21561 [details]
gcc46-pr44858-2.patch

Alternate patch, which doesn't give up so easily (usually both sets can go in any order, so if added clobbers prevent one order, this patch tries the other order too and only gives up if both orders aren't going to work).
Comment 9 Jakub Jelinek 2010-08-25 17:51:16 UTC
Subject: Bug 44858

Author: jakub
Date: Wed Aug 25 17:50:59 2010
New Revision: 163552

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=163552
Log:
	PR rtl-optimization/44858
	* combine.c (try_combine): If recog_for_combine added CLOBBERs to
	newi2pat, make sure they don't affect newpat.

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

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr44858.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/combine.c
    trunk/gcc/testsuite/ChangeLog

Comment 10 Jakub Jelinek 2010-08-25 21:25:36 UTC
Subject: Bug 44858

Author: jakub
Date: Wed Aug 25 21:25:18 2010
New Revision: 163555

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=163555
Log:
	PR rtl-optimization/44858
	* combine.c (try_combine): If recog_for_combine added CLOBBERs to
	newi2pat, make sure they don't affect newpat.

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

Added:
    branches/gcc-4_5-branch/gcc/testsuite/gcc.c-torture/execute/pr44858.c
Modified:
    branches/gcc-4_5-branch/gcc/ChangeLog
    branches/gcc-4_5-branch/gcc/combine.c
    branches/gcc-4_5-branch/gcc/testsuite/ChangeLog

Comment 11 Jakub Jelinek 2010-08-25 21:27:22 UTC
Fixed.