Bug 24627 - [4.1 Regression] xntp miscompiled
[4.1 Regression] xntp miscompiled
Status: RESOLVED FIXED
Product: gcc
Classification: Unclassified
Component: tree-optimization
4.1.0
: P1 critical
: 4.1.0
Assigned To: Diego Novillo
http://gcc.gnu.org/ml/gcc-patches/200...
: alias, wrong-code
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-11-01 23:04 UTC by Dirk Mueller
Modified: 2005-11-04 19:57 UTC (History)
3 users (show)

See Also:
Host:
Target: i686-gnu-linux
Build:
Known to work:
Known to fail:
Last reconfirmed: 2005-11-01 23:08:47


Attachments
testcase (1.56 KB, text/plain)
2005-11-01 23:06 UTC, Dirk Mueller
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Dirk Mueller 2005-11-01 23:04:32 UTC
xntp is miscompiled since SVN revision 101841.

will attach testcase in a minute
Comment 1 Dirk Mueller 2005-11-01 23:06:04 UTC
Created attachment 10110 [details]
testcase

-O1 or higher exposes the bug. 

-fno-tree-dce or undoing commit 101841 fixes it.
Comment 2 Steven Bosscher 2005-11-01 23:08:47 UTC
-fno-tree-loop-im fixes it too, fwiw.
Comment 3 Steven Bosscher 2005-11-01 23:09:09 UTC
Most likely aliasing related.
Comment 4 Steven Bosscher 2005-11-01 23:09:52 UTC
Mark, this is a new wrong-code bug.
Could you look at it and set a priority please.
Comment 5 Andrew Pinski 2005-11-01 23:12:56 UTC
Note this is union related as if I remove the unions, it works.
Comment 6 Andrew Pinski 2005-11-01 23:22:55 UTC
Here is a little cleaned up testcase without any do {} while(0);:
extern void abort (void) __attribute__ ((noreturn));

typedef unsigned int u_int32;
typedef struct {
  union {u_int32 Xl_ui;} Ul_i;
  union {u_int32 Xl_uf;} Ul_f;
} l_fp;
void dolfptoa(short ndec)
{
  l_fp work, ftmp;
  work.Ul_i.Xl_ui = 0;
  work.Ul_f.Xl_uf = 0x535f3d8;
  while (ndec > 0) {

    ndec--;
    work.Ul_i.Xl_ui = 0;
    work.Ul_i.Xl_ui <<= 1;
    if ((work.Ul_f.Xl_uf) & 0x80000000)
      (work.Ul_i.Xl_ui) |= 0x1; (work.Ul_f.Xl_uf) <<= 1;
  //  printf("%d\t", work.Ul_i.Xl_ui);

    ftmp = work;
    (work.Ul_i.Xl_ui) <<= 1;
    if ((work.Ul_f.Xl_uf) & 0x80000000)
      (work.Ul_i.Xl_ui) |= 0x1; (work.Ul_f.Xl_uf) <<= 1;

    (work.Ul_i.Xl_ui) <<= 1;
    if ((work.Ul_f.Xl_uf) & 0x80000000)
      (work.Ul_i.Xl_ui) |= 0x1; (work.Ul_f.Xl_uf) <<= 1;
  //  printf("%d\t", work.Ul_i.Xl_ui);
    u_int32 lo_tmp;
    u_int32 hi_tmp;
    lo_tmp = ((work.Ul_f.Xl_uf) & 0xffff) + ((ftmp.Ul_f.Xl_uf) & 0xffff);
    hi_tmp = (((work.Ul_f.Xl_uf) >> 16) & 0xffff) + (((ftmp.Ul_f.Xl_uf) >> 16) & 0xffff);

    if (lo_tmp & 0x10000)
      hi_tmp++;

    (work.Ul_f.Xl_uf) = ((hi_tmp & 0xffff) << 16) | (lo_tmp & 0xffff);
    (work.Ul_i.Xl_ui) += (ftmp.Ul_i.Xl_ui);

    if (hi_tmp & 0x10000)
     (work.Ul_i.Xl_ui)++;
   // printf("%d\n", work.Ul_i.Xl_ui);
    if (!(work.Ul_i.Xl_ui < 10))
      abort ();
  }
}
int main()
{
  dolfptoa(6);
  return 0;
}
Comment 7 Andrew Pinski 2005-11-01 23:32:14 UTC
wtf:
-  #   SFT.1_22 = V_MUST_DEF <SFT.1_19>;
+  #   SFT.1_22 = V_MUST_DEF <SFT.1_11>;
   work.Ul_i.Xl_ui = D.1298_21;
...
-  # SFT.1_10 = PHI <SFT.1_12(0), SFT.1_9(11)>;

That is just wrong.  DCE is doing something wrong, very wrong.
Comment 8 Andrew Pinski 2005-11-01 23:34:09 UTC
Eliminating unnecessary statements:
Deleting : SFT.1_10 = PHI <SFT.1_12(0), SFT.1_9(11)>;

Deleting : work.Ul_i.Xl_ui = 0;

Deleting : work.Ul_i.Xl_ui = 0;

Deleting : work.Ul_f.Xl_uf = D.1302_36;

Deleting : work.Ul_f.Xl_uf = D.1302_44;


No SFT.1_10 = PHI <SFT.1_12(0), SFT.1_9(11)>; is not useless
Comment 9 Mark Mitchell 2005-11-02 00:55:06 UTC
Miscompilation of a popular package on a major architecture; showstopper.
Comment 10 Andrew Pinski 2005-11-02 15:47:21 UTC
Here is a further reduced testase:
typedef struct {
  union {unsigned Xl_ui;} Ul_i;
  union {unsigned Xl_uf;} Ul_f;
} l_fp;
void dolfptoa(short ndec)
{
  l_fp work;
  work.Ul_f.Xl_uf = 0x535f3d8;
  while (ndec > 0) {

    ndec--;
    work.Ul_i.Xl_ui = 0;
    work.Ul_i.Xl_ui = 0;
    if ((work.Ul_f.Xl_uf) & 0x80000000)
      (work.Ul_i.Xl_ui) |= 0x1;
    (work.Ul_f.Xl_uf) <<= 1;

    (work.Ul_i.Xl_ui) <<= 1;
    if ((work.Ul_f.Xl_uf) & 0x80000000)
      (work.Ul_i.Xl_ui) |= 0x1;
    (work.Ul_f.Xl_uf) <<= 1;

    (work.Ul_i.Xl_ui) <<= 1;

    if (!(work.Ul_i.Xl_ui < 10))
      abort ();
  }
}
int main()
{
  dolfptoa(6);
  return 0;
}
Comment 11 Andrew Pinski 2005-11-02 15:53:13 UTC
The following fails with -O1 -fno-tree-sra.
typedef struct {
  struct {unsigned Xl_ui;} Ul_i;
  struct {unsigned Xl_uf;} Ul_f;
} l_fp;
void dolfptoa(short ndec)
{
  l_fp work;
  work.Ul_f.Xl_uf = 0x535f3d8;
  while (ndec > 0) {

    ndec--;
    work.Ul_i.Xl_ui = 0;
    work.Ul_i.Xl_ui = 0;
    if ((work.Ul_f.Xl_uf) & 0x80000000)
      (work.Ul_i.Xl_ui) |= 0x1;
    (work.Ul_f.Xl_uf) <<= 1;

    (work.Ul_i.Xl_ui) <<= 1;
    if ((work.Ul_f.Xl_uf) & 0x80000000)
      (work.Ul_i.Xl_ui) |= 0x1;
    (work.Ul_f.Xl_uf) <<= 1;

    (work.Ul_i.Xl_ui) <<= 1;

    if (!(work.Ul_i.Xl_ui < 10))
      abort ();
  }
}
int main()
{
  dolfptoa(6);
  return 0;
}
----

The -fno-tree-sra is need as otherwise SRA actually does it work and renames the PHIs correctly.
Comment 12 Andrew Pinski 2005-11-02 16:03:03 UTC
Here is another more reduced testcase (still at -O1 -fno-tree-sra):
typedef struct {
  unsigned a;
} l_fp;
void dolfptoa(short ndec)
{
  l_fp work;
  unsigned workUl_fXl_uf;
  work.a = 0x535f3d8;
  while (ndec > 0) {

    ndec--;
    work.a = 0;
    work.a = 0;
    if (workUl_fXl_uf & 0x80000000)
      work.a |= 0x1;
    workUl_fXl_uf <<= 1;

    work.a <<= 1;
    if (workUl_fXl_uf & 0x80000000)
      work.a |= 0x1;
    workUl_fXl_uf <<= 1;

    work.a <<= 1;

    if (!(work.a < 10))
      abort ();
  }
}
int main()
{
  dolfptoa(6);
  return 0;
}
Comment 13 Andrew Pinski 2005-11-02 16:12:17 UTC
(In reply to comment #12)
> Here is another more reduced testcase (still at -O1 -fno-tree-sra):
That testcase is invalid, the one which is valid:
ypedef struct {
  unsigned a;
} l_fp;
void dolfptoa(short ndec)
{
  l_fp work;
  unsigned workUl_fXl_uf;
  workUl_fXl_uf = 0x535f3d8;
  while (ndec > 0) {

    ndec--;
    work.a = 0;
    work.a = 0;
    if (workUl_fXl_uf & 0x80000000)
      work.a |= 0x1;
    workUl_fXl_uf <<= 1;

    work.a <<= 1;
    if (workUl_fXl_uf & 0x80000000)
      work.a |= 0x1;
    workUl_fXl_uf <<= 1;

    work.a <<= 1;

    if (!(work.a < 10))
      abort ();
  }
}
int main()
{
  dolfptoa(8);
  return 0;
}
Comment 14 Andrew Pinski 2005-11-02 16:39:53 UTC
This is as far as I can reduce it, -O1:
typedef union {
  unsigned a;
} l_fp;
int main(void)
{
  l_fp work;
  unsigned workUl_fXl_uf = 0xAAAAAAAA;
  int ndec = 10;
  do {
    ndec--;
    work.a = 0;
    work.a = 0;

    if (workUl_fXl_uf & 1)
      work.a ++;

    workUl_fXl_uf <<= 1;

    work.a ++;
    if (workUl_fXl_uf & 1)
      work.a ++;

    work.a ++;

    if (work.a > 4)
      abort ();
  }while(ndec > 0);
  return 0;
}
Comment 15 Andrew Pinski 2005-11-02 17:02:41 UTC
A link to the changes (so I can be a little lazy):
http://gcc.gnu.org/viewcvs?view=rev&rev=101841
Comment 16 Diego Novillo 2005-11-02 19:40:54 UTC
testing patch
Comment 17 Diego Novillo 2005-11-04 19:56:33 UTC
Subject: Bug 24627

Author: dnovillo
Date: Fri Nov  4 19:56:28 2005
New Revision: 106502

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=106502
Log:

	PR 24627
	* tree-ssa-dce.c (pass_dce, pass_dce_loop, pass_cd_dce): Use
	TODO_update_ssa instead of TODO_update_ssa_no_phi.

testsuite/

	PR 24627
	* gcc.dg/tree-ssa/pr24627.c: New test.


Added:
    trunk/gcc/testsuite/gcc.dg/tree-ssa/pr24627.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-dce.c

Comment 18 Diego Novillo 2005-11-04 19:57:50 UTC
Fixed. http://gcc.gnu.org/ml/gcc-patches/2005-11/msg00239.html