Bug 41497 - [4.5 Regression] apparent integer wrong code bug
Summary: [4.5 Regression] apparent integer wrong code bug
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.5.0
: P1 normal
Target Milestone: 4.5.0
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2009-09-29 03:52 UTC by John Regehr
Modified: 2009-10-23 20:57 UTC (History)
5 users (show)

See Also:
Host: i686-pc-linux-gnu
Target: i686-pc-linux-gnu
Build: i686-pc-linux-gnu
Known to work: 4.3.4 4.4.1
Known to fail:
Last reconfirmed: 2009-10-05 19:54:50


Attachments
1706_pr41497.diff (788 bytes, patch)
2009-10-20 21:53 UTC, Sebastian Pop
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Regehr 2009-09-29 03:52:36 UTC
It looks wrong at -Os.  The compiler is valgrind-clean at that level.

regehr@john-home:~/volatile/tmp200$ current-gcc -O small.c -o small
regehr@john-home:~/volatile/tmp200$ ./small
checksum = -13

regehr@john-home:~/volatile/tmp200$ current-gcc -Os small.c -o small
regehr@john-home:~/volatile/tmp200$ ./small
checksum = 65523

regehr@john-home:~/volatile/tmp200$ current-gcc -v

Using built-in specs.
Target: i686-pc-linux-gnu
Configured with: ../configure --prefix=/home/regehr/z/tmp/gcc-r152261-install --program-prefix=r152261- --enable-languages=c,c++
Thread model: posix
gcc version 4.5.0 20090929 (experimental) (GCC) 

regehr@john-home:~/volatile/tmp200$ cat small.c

#include <stdint.h>
#include <stdio.h>

static uint16_t add (uint16_t ui1, uint16_t ui2)
{
  return ui1 + ui2;
}

uint32_t g_108;
uint8_t f3;
uint8_t f0;

void func_1 (void);
void func_1 (void)
{
  for (f3 = 0; f3 <= 0; f3 = 1) {
    for (g_108 = -13; g_108 == 0; g_108 = add (g_108, 0)) {
      f0 = 1;
    }
  }
}

int
main (void)
{
  func_1 ();
  printf ("checksum = %d\n", g_108);
  return 0;
}
Comment 1 Jakub Jelinek 2009-09-29 07:23:12 UTC
Fails on x86_64-linux too, the bug first appears in sccp.
extern void abort (void);

static unsigned short
bar (unsigned short x, unsigned short y)
{
  return x + y;
}

unsigned int a;
unsigned char b, c;

void
foo (void)
{
  for (b = 0; b <= 0; b = 1)
    for (a = -13; a == 0; a = bar (a, 0))
      c = 1;
}

int
main ()
{
  foo ();
  if (a != -13)
    abort ();
  return 0;
}
Comment 2 Richard Biener 2009-09-29 09:51:50 UTC
Thus, confirmed.
Comment 3 Richard Biener 2009-10-11 11:56:19 UTC
The key is IMHO that the inner loop does not have its loop header copied.
This confuses SCEV enough to think that a = (unsigned short)a is always
executed.  LIM is necessary to promote all the memory to SSA names, the
interesting CHREC that gets wrong is

<bb 3>:
..
  a_lsm.6_18 = 0x0fffffff3;
  goto <bb 5>;

<bb 4>:
...
  a.1_2 = a_lsm.6_15;
  D.2006_3 = (short unsigned int) a.1_2;
  a.2_4 = (unsigned int) D.2006_3;
  a_lsm.6_20 = a.2_4;

<bb 5>:
  # a_lsm.6_15 = PHI <a_lsm.6_18(3), a_lsm.6_20(4)>
...
  a.1_1 = a_lsm.6_15;
  if (a.1_1 == 0)
    goto <bb 4>;
  else
    goto <bb 6>;

 <bb 6>:
...
-  # a_lsm.6_27 = PHI <a_lsm.6_15(5)>


 <bb 8>:
-  # a_lsm.6_28 = PHI <a_lsm.6_27(6)>
-  a = a_lsm.6_28;
+  a = 65523;

where interestingly a_lsm.6_15 isn't computed wrong.


Testcase, fails at -Os:

extern void abort (void);

unsigned int a;
int b, c;

void
foo (void)
{
  b = 0;
  do {
    for (a = -13; a == 0; a = (unsigned short)a)
      c = 1;
    b++;
  } while (b == 0);
}

int
main ()
{
  foo ();
  if (a != -13)
    abort ();
  return 0;
}
Comment 4 Richard Biener 2009-10-11 11:56:51 UTC
Sebastian, can you have a look?
Comment 5 H.J. Lu 2009-10-11 16:59:37 UTC
Revision 147714 is OK and revision 147717 is bad.
Comment 6 H.J. Lu 2009-10-11 19:34:41 UTC
It is caused by revision 147716:

http://gcc.gnu.org/ml/gcc-cvs/2009-05/msg00693.html
Comment 7 Eric Botcazou 2009-10-12 17:59:47 UTC
> It is caused by revision 147716:
> 
> http://gcc.gnu.org/ml/gcc-cvs/2009-05/msg00693.html

Yep, but it's Richard's fault. ;-)  The bug is exposed by the change requested in http://gcc.gnu.org/ml/gcc-patches/2009-05/msg01227.html so I won't go beyond suggesting the following partial reversion

Index: tree-scalar-evolution.c
===================================================================
--- tree-scalar-evolution.c	(revision 152647)
+++ tree-scalar-evolution.c	(working copy)
@@ -1159,7 +1159,7 @@ follow_ssa_edge_expr (struct loop *loop,
 
   switch (code)
     {
-    CASE_CONVERT:
+    case NOP_EXPR:
       /* This assignment is under the form "a_1 = (cast) rhs.  */
       res = follow_ssa_edge_expr (loop, at_stmt, TREE_OPERAND (expr, 0),
 				  halting_phi, evolution_of_loop, limit);
@@ -1222,7 +1222,7 @@ follow_ssa_edge_in_rhs (struct loop *loo
 
   switch (code)
     {
-    CASE_CONVERT:
+    case NOP_EXPR:
       /* This assignment is under the form "a_1 = (cast) rhs.  */
       res = follow_ssa_edge_expr (loop, stmt, gimple_assign_rhs1 (stmt),
 				  halting_phi, evolution_of_loop, limit);

But I also think that the real problem is elsewhere.
Comment 8 Richard Biener 2009-10-12 20:23:20 UTC
Eh, indeed if that fixes it the bug is elsewhere.  I suggest to not apply this
partial reversion until we know better.
Comment 9 Richard Biener 2009-10-12 20:24:27 UTC
The problem is that we follow SSA edges into loops that may not be executed.
Sebastian - how are we supposed to deal with this case?
Comment 10 Sebastian Pop 2009-10-20 21:53:06 UTC
Subject: Re:  [4.5 Regression] apparent integer 
	wrong code bug

> The problem is that we follow SSA edges into loops that may not be executed.

It is correct to follow SSA edges in loops that may not execute, as we
compute a symbolic expression of the evolution.

The error is that we are not careful enough in the use of this
symbolic information: in analyze_evolution_in_loop we are given the
loop_phi_node: a_lsm.6_15 = PHI <a_lsm.6_18(3), a_lsm.6_20(4)> and its
initial value init_cond: a_lsm.6_18.

follow_ssa_edge returns an evolution function ev_fn that could be
incompatible with the initial value:

(unsigned int) (short unsigned int) a_lsm.6_18;

The attached patch fixes the problem by returning "don't know" when
init_cond is not equal to ev_fn when no_evolution_in_loop_p manages to
prove that ev_fn is invariant in the loop.

Sebastian
Comment 11 Sebastian Pop 2009-10-20 21:53:07 UTC
Created attachment 18847 [details]
1706_pr41497.diff
Comment 12 Sebastian Pop 2009-10-21 23:05:51 UTC
Subject: Bug 41497

Author: spop
Date: Wed Oct 21 23:05:39 2009
New Revision: 153441

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=153441
Log:
	PR tree-optimization/41497
	* tree-scalar-evolution.c (analyze_evolution_in_loop): Return
	chrec_dont_know if the evolution function returned by follow_ssa_edge
	is constant in the analyzed loop and is not compatible with the
	initial value before the loop.
	* tree-chrec.h (no_evolution_in_loop_p): Call STRIP_NOPS.

	* gcc.dg/tree-ssa/pr41497.c: New.


Added:
    trunk/gcc/testsuite/gcc.dg/tree-ssa/pr41497.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-chrec.h
    trunk/gcc/tree-scalar-evolution.c

Comment 13 Sebastian Pop 2009-10-21 23:10:50 UTC
Fixed.

This bug is potentially latent in older branches.
Should I test it in the 4.4 branch as well?
Comment 14 hjl@gcc.gnu.org 2009-10-30 16:05:31 UTC
Subject: Bug 41497

Author: hjl
Date: Fri Oct 30 16:04:41 2009
New Revision: 153759

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=153759
Log:
2009-10-30  H.J. Lu  <hongjiu.lu@intel.com>

	Backport from mainline:
	2009-10-30  Dodji Seketeli  <dodji@redhat.com>

	PR c++/41863
	* g++.dg/template/sizeof12.C: New test.

	2009-10-29  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/41775
	* g++.dg/torture/pr41775.C: New testcase.

	2009-10-28  Jakub Jelinek  <jakub@redhat.com>

	PR debug/41801
	* g++.dg/ext/sync-3.C: New test.

	2009-10-27  Jakub Jelinek  <jakub@redhat.com>

	PR c++/41020
	* g++.dg/lookup/extern-c-redecl5.C: Fix up regexp.

	2009-10-26  Jakub Jelinek  <jakub@redhat.com>

	PR bootstrap/41345
	* gcc.dg/pr41345.c: New test.

	2009-10-26  Dodji Seketeli  <dodji@redhat.com>

	PR c++/41785
	* g++.dg/cpp0x/variadic96.C: New test.

	2009-10-26  Dodji Seketeli  <dodji@redhat.com>

	PR c++/41020
	* g++.dg/lookup/extern-c-redecl2.C: New test.
	* g++.dg/lookup/extern-c-redecl3.C: Likewise.
	* g++.dg/lookup/extern-c-redecl4.C: Likewise.
	* g++.dg/lookup/extern-c-redecl5.C: Likewise.

	2009-10-23  Joseph Myers  <joseph@codesourcery.com>

	PR c/40033
	* gcc.dg/noncompile/pr40033-1.c: New test.

	2009-10-23  Joseph Myers  <joseph@codesourcery.com>

	PR c/41673
	* gcc.dg/Wstrict-aliasing-bogus-vla-1.c: New test.

	2009-10-21  Sebastian Pop  <sebastian.pop@amd.com>

	PR tree-optimization/41497
	* gcc.dg/tree-ssa/pr41497.c: New.

Added:
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/cpp0x/variadic96.C
      - copied unchanged from r153757, trunk/gcc/testsuite/g++.dg/cpp0x/variadic96.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/ext/sync-3.C
      - copied unchanged from r153757, trunk/gcc/testsuite/g++.dg/ext/sync-3.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/lookup/extern-c-redecl2.C
      - copied unchanged from r153757, trunk/gcc/testsuite/g++.dg/lookup/extern-c-redecl2.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/lookup/extern-c-redecl3.C
      - copied unchanged from r153757, trunk/gcc/testsuite/g++.dg/lookup/extern-c-redecl3.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/lookup/extern-c-redecl4.C
      - copied unchanged from r153757, trunk/gcc/testsuite/g++.dg/lookup/extern-c-redecl4.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/lookup/extern-c-redecl5.C
      - copied unchanged from r153757, trunk/gcc/testsuite/g++.dg/lookup/extern-c-redecl5.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/template/sizeof12.C
      - copied unchanged from r153757, trunk/gcc/testsuite/g++.dg/template/sizeof12.C
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/torture/pr41775.C
      - copied unchanged from r153757, trunk/gcc/testsuite/g++.dg/torture/pr41775.C
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/Wstrict-aliasing-bogus-vla-1.c
      - copied unchanged from r153758, trunk/gcc/testsuite/gcc.dg/Wstrict-aliasing-bogus-vla-1.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/noncompile/pr40033-1.c
      - copied unchanged from r153758, trunk/gcc/testsuite/gcc.dg/noncompile/pr40033-1.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/pr41345.c
      - copied unchanged from r153757, trunk/gcc/testsuite/gcc.dg/pr41345.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/tree-ssa/pr41497.c
      - copied unchanged from r153758, trunk/gcc/testsuite/gcc.dg/tree-ssa/pr41497.c
Modified:
    branches/gcc-4_4-branch/gcc/testsuite/ChangeLog