Bug 19579 - [3.3 regression] -march=i686 generates a bogus program for x86*
Summary: [3.3 regression] -march=i686 generates a bogus program for x86*
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 3.4.3
: P1 critical
Target Milestone: 3.3.6
Assignee: Jakub Jelinek
URL:
Keywords: monitored, patch, wrong-code
Depends on:
Blocks:
 
Reported: 2005-01-23 00:28 UTC by Richard Delorme
Modified: 2005-05-01 13:17 UTC (History)
7 users (show)

See Also:
Host:
Target:
Build:
Known to work: 3.3.3 4.0.0 3.4.4
Known to fail: 3.3.4 3.3.5 3.4.0 3.4.3
Last reconfirmed: 2005-02-26 18:42:28


Attachments
diff .13.loop and .15.cse2 (1.71 KB, text/plain)
2005-01-23 22:18 UTC, Steven Bosscher
Details
diff for .17.combine and .18.ce2 (944 bytes, text/plain)
2005-01-24 00:14 UTC, Steven Bosscher
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Delorme 2005-01-23 00:28:52 UTC
The following program:

/*----------8<-----------------------*/
int printf(const char *, ...);

int f(void)
{
	return 0;	
}

void g(int a[])
{
	int n3, n0;
	int even;
	int diff = a[1] - a[0];

	n3 = 3;
	if ((n0 = f()) > 0) {
		even = diff + 2 * (n3 - n0);
	} else {
		even = diff + 2 * n3 + 1;
		if (even > 0) even++;
		else if (even < 0) even--;
	}

	if (even % 2 == 0) {
		printf("OK! even = %d\n", even);
	} else {
		printf("WRONG! even = %d\n", even);
	}
}

int main(void)
{
	int a[2] = {46, 16};
	g(a);
	return 0;
}
/*----------8<-----------------------*/

when compiled with:
    $ gcc-3.4.3 bug.c -o bug -O2 -march=i686

displays when executed:
    WRONG! even = -23

whereas when compiled with:
    $ gcc-3.4.3 bug.c -o bug -O2

gives the expected output:
    OK! even = -24

The bug is also exhibited when some others -march options are used, like
-march=athlon-xp.

gcc-3.4.3 -v output is:
Reading specs from
/home/richard/programs/gcc-3.4.3/lib/gcc/i686-pc-linux-gnu/3.4.3/specs
Configured with: /home/richard/tmp/gcc/gcc-3.4.3/configure
--prefix=/home/richard/programs/gcc-3.4.3 --program-suffix=-3.4.3
--enable-threads=posix --enable-languages=c,c++
Thread model: posix
gcc version 3.4.3

bug.i:
# 1 "bug.c"
# 1 "<built-in>"
# 1 "<command line>"
# 1 "bug.c"
int printf(const char *, ...);

int f(void)
{
 return 0;
}

void g(int a[])
{
 int n3, n0;
 int even;
 int diff = a[1] - a[0];

 n3 = 3;
 if ((n0 = f()) > 0) {
  even = diff + 2 * (n3 - n0);
 } else {
  even = diff + 2 * n3 + 1;
  if (even > 0) even++;
  else if (even < 0) even--;
 }

 if (even % 2 == 0) {
  printf("OK! even = %d\n", even);
 } else {
  printf("WRONG! even = %d\n", even);
 }
}

int main(void)
{
 int a[2] = {46, 16};
 g(a);
 return 0;
}
Comment 1 Volker Reichelt 2005-01-23 21:14:58 UTC
Confirmed.

Here's a condensed testcase (compile with "-O2 -march=i686" on
i686-pc-linux-gnu):

================================================
int foo(int i, int j)
{
    int k = i+1;

    if (j)
    {
        if (k>0)
            k++;
        else
            if (k<0) k--;
    }

    return k;
}

int main()
{
    return foo(-2,1)+2;
}
================================================

The program should return 0, but returns 1 instead

The bug was introduced in gcc 3.3.4 and affects the 3.3 branch and the
3.4 branch. Mainline does not seem to be affected, but maybe the bug
is latent.

Ian, it looks like your patch for PR 1532 introduced the regression:
http://gcc.gnu.org/ml/gcc-cvs/2004-03/msg00264.html

Could you please have a look?
Comment 2 Serge Belyshev 2005-01-23 21:32:11 UTC
I can confirm this bug on mainline with this testcase:
------------------------------------------------------------------------------
void abort (void);

int f ()
{
  return 0;
}

void __attribute__((noinline)) g (int k, int l)
{
  int j;
  
  if (f ())
    {
      j = k + l;
    }
  else
    {
      j = k + l + 1;
      if (j > 0)
	j++;
      else
	if (j < 0)
	  j--;
    }

  if (j % 2 != 0)
    abort ();
}

int main ()
{
  g (-2, 0);
  return 0;
}
------------------------------------------------------------------------------
Comment 3 Steven Bosscher 2005-01-23 22:06:32 UTC
The tree dump already looks wrong to me on mainline: 
 
g (k, l) 
{ 
  int j.0; 
  int j; 
  _Bool D.1460; 
  int D.1459; 
  int D.1458; 
  int D.1457; 
 
<bb 0>: 
  D.1457 = f (); 
  if (D.1457 != 0) goto <L0>; else goto <L1>; 
 
<L0>:; 
  j = k + l; 
  goto <bb 6> (<L5>); 
 
<L1>:; 
  j = k + l; 
  j.0 = j + 1; 
  if (j.0 > 0) goto <L2>; else goto <L3>; 
 
<L2>:; 
  j = j + 2; 
  goto <bb 6> (<L5>); 
 
<L3>:; 
  if (j.0 < 0) goto <L5>; else goto <L10>; 
 
<L10>:; 
  j = j.0; 
 
<L5>:; 
  if ((j & 1) != 0) goto <L8>; else goto <L9>; 
 
<L8>:; 
  abort (); 
 
<L9>:; 
  return; 
 
} 
 
Note how "j0 = j + 1" is assigned to j, which is "k + l" at that point, 
but the predecrement does not happen. 
 
Comment 4 Steven Bosscher 2005-01-23 22:08:06 UTC
Oh bugger.  The tree dump is good. 
 
Comment 5 Steven Bosscher 2005-01-23 22:13:07 UTC
"-O -frerun-cse-after-loop" is enough to reproduce it on AMD64. 
Comment 6 Andrew Pinski 2005-01-23 22:14:23 UTC
As per GDR (a while back) moving 3.4.x regressions to the target milestone for 3.4.x.
Comment 7 Steven Bosscher 2005-01-23 22:18:34 UTC
Created attachment 8048 [details]
diff .13.loop and .15.cse2
Comment 8 Kazu Hirata 2005-01-23 22:25:50 UTC
On my machine, Athlon XP 2500 running Fedora Core 3,

-O2 -march=i686 produces OK!, but
-O2 -march=athlon-xp produces WRONG!

if the original testcase is compiled with the current mainline.
Comment 9 Steven Bosscher 2005-01-23 23:50:39 UTC
From the dump, I think the cse_condition_code_reg code is OK, but it  
is exposing a bug elsewhere.  
Comment 10 Steven Bosscher 2005-01-24 00:02:21 UTC
This looks more like an if-conversion bug in if-after-combine. 
 
Comment 11 Steven Bosscher 2005-01-24 00:14:50 UTC
Created attachment 8049 [details]
diff for .17.combine and .18.ce2

Notice how in the combine dump (the top half) we have a set of CC in
insn 28 and a conditional jump to block 4 that preserves CC.  Block
4 starts with a conditional move in insn 67, so it depends on insn 28.
There is no REG_DEP note for that (should there be?) but the CC reg
(reg 17) is live on entry at least.

After if-after-combine, we have a new conditional move in insn 73, but
the state of the CC reg is not known there because insn 72 clobbers
that reg.
Comment 12 Steven Bosscher 2005-01-24 00:34:44 UTC
For the RTL illiterate like myself: 
 
BB2: j = k + l 
     j0 = j + 1 
     flags = (j0 <= 0) 
     if (flags) goto BB4 else goto BB3 
     flags are in live_at_end 
 
BB3: flags are not in live_at_start 
     j = j + 2 [ kills the flags ] 
     goto BB5 
     flags are not in live_at_end 
 
BB4: flags are in live_at_start 
     j = (flags) ? j : j0 
     goto BB5 
     flags are not in live_at_end 
 
BB5: just an uninteresting join block 
 
 
Then if-conversion basically merges BB2, BB3, and BB4 to give: 
 
BB2: j = k + l 
     j0 = j + 1 
     r65 = j + 2 [ kills the flags ] 
     r66 = (flags) ? j : j0 
     flags = (j0 <= 0) 
     j = (flags) ? r66 : r65 
     falthru to BB5 
 
BB5: just an uninteresting former join block 
 
Comment 13 Steven Bosscher 2005-01-24 09:40:36 UTC
wrong-code bug, should be P1. 
Comment 14 Jakub Jelinek 2005-01-24 10:30:05 UTC
Regarding the question in #11:
LOG_LINKS only point to instructions in the same basic block:
/* Set up in flow.c; empty before then.
   Holds a chain of INSN_LIST rtx's whose first operands point at
   previous insns with direct data-flow connections to this one.
   That means that those insns set variables whose next use is in this insn.
   They are always in the same basic block as this insn.  */
#define LOG_LINKS(INSN) XEXP(INSN, 7)
Comment 15 Andrew Pinski 2005-01-24 14:13:05 UTC
Patch here: <http://gcc.gnu.org/ml/gcc-patches/2005-01/msg01713.html>.
Comment 16 GCC Commits 2005-01-25 23:09:15 UTC
Subject: Bug 19579

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	jakub@gcc.gnu.org	2005-01-25 23:09:10

Modified files:
	gcc            : ChangeLog ifcvt.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/gcc.c-torture/execute: 20050124-1.c 

Log message:
	PR rtl-optimization/19579
	* ifcvt.c (noce_try_cmove_arith): If emitting instructions to set up
	both A and B, see if they don't clobber registers the other expr uses.
	
	* gcc.c-torture/execute/20050124-1.c: New test.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.7273&r2=2.7274
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ifcvt.c.diff?cvsroot=gcc&r1=1.178&r2=1.179
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.4936&r2=1.4937
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.c-torture/execute/20050124-1.c.diff?cvsroot=gcc&r1=NONE&r2=1.1

Comment 17 Andrew Pinski 2005-01-26 01:19:08 UTC
Fixed at least on the mainline.
Comment 18 GCC Commits 2005-02-10 17:11:48 UTC
Subject: Bug 19579

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_4-branch
Changes by:	jakub@gcc.gnu.org	2005-02-10 17:11:13

Modified files:
	gcc            : ChangeLog ifcvt.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/gcc.c-torture/execute: 20050124-1.c 

Log message:
	PR rtl-optimization/19579
	* ifcvt.c (noce_try_cmove_arith): If emitting instructions to set up
	both A and B, see if they don't clobber registers the other expr uses.
	
	* gcc.c-torture/execute/20050124-1.c: New test.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=2.2326.2.795&r2=2.2326.2.796
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ifcvt.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.136.4.1&r2=1.136.4.2
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.3389.2.358&r2=1.3389.2.359
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.c-torture/execute/20050124-1.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=NONE&r2=1.1.12.1

Comment 19 Andrew Pinski 2005-02-10 17:19:08 UTC
Fixed also in 3.4.4.
Comment 20 Gabriel Dos Reis 2005-04-28 08:34:20 UTC
(In reply to comment #15)
> Patch here: <http://gcc.gnu.org/ml/gcc-patches/2005-01/msg01713.html>.

Roger --

  Are you still confident in this patch (as you said last Jan) for 3.3.6?

-- Gaby
Comment 21 GCC Commits 2005-05-01 07:23:33 UTC
Subject: Bug 19579

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_3-branch
Changes by:	sayle@gcc.gnu.org	2005-05-01 07:23:20

Modified files:
	gcc            : ChangeLog ifcvt.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/gcc.c-torture/execute: 20050124-1.c 

Log message:
	PR rtl-optimization/19579
	Backport from mainline
	2005-01-26  Jakub Jelinek  <jakub@redhat.com>
	* ifcvt.c (noce_try_cmove_arith): If emitting instructions to set up
	both A and B, see if they don't clobber registers the other expr uses.
	
	* gcc.c-torture/execute/20050124-1.c: New test.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.16114.2.1067&r2=1.16114.2.1068
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ifcvt.c.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.105.4.9&r2=1.105.4.10
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.2261.2.399&r2=1.2261.2.400
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.c-torture/execute/20050124-1.c.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=NONE&r2=1.1.32.1

Comment 22 Andrew Pinski 2005-05-01 13:17:40 UTC
Fixed.