[Bug rtl-optimization/33669] New: [4.3 Regression] Revision 128957 miscompiles 481.wrf

[Bug rtl-optimization/33669] New: [4.3 Regression] Revision 128957 miscompiles 481.wrf

[hjl at lucon dot org]

On Linux/ia32, revision 128957 miscompiles 481.wrf in SPEC CPU 2006:

Compiling Binaries
  Building 481.wrf test base o2 default: (build_base_o2.0000)

Build successes: 481.wrf(base)

Parsing Flags
  Looking at 481.wrf base o2 default: done
Flag Parsing Complete

Setting Up Run Directories
  Setting up 481.wrf test base o2 default: created (run_base_test_o2.0000)
Running Benchmarks
  Running 481.wrf test base o2 default
Error with '/export/gnu/import/rrs/spec/2006/spec/bin/specinvoke -E -d
/export/gnu/import/rrs/spec/2006/spec/benchspec/CPU2006/481.wrf/run/run_base_test_o2.0000
-c 1 -e compare.err -o compare.stdout -f compare.cmd': check file
'/export/gnu/import/rrs/spec/2006/spec/benchspec/CPU2006/481.wrf/run/run_base_test_o2.0000/.err'

****************************************
Contents of wrf.err
****************************************
STOP wrf_abort
 -------------- FATAL CALLED ---------------
  program wrf: error opening wrfinput_d01 for reading ierr=       -1021         
 -------------------------------------------

****************************************
*** Miscompare of rsl.out.0000, see
/export/gnu/import/rrs/spec/2006/spec/benchspec/CPU2006/481.wrf/run/run_base_test_o2.0000/rsl.out.0000.mis
Invalid run; unable to continue.  If you wish to ignore errors please use '-I'
or ignore_errors


-- 
           Summary: [4.3 Regression]  Revision 128957 miscompiles 481.wrf
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: hjl at lucon dot org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33669

[Bug rtl-optimization/33669] [4.3 Regression] Revision 128957 miscompiles 481.wrf

[rguenth at gcc dot gnu dot org]

-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu dot
                   |                            |org
           Keywords|                            |wrong-code
   Target Milestone|---                         |4.3.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33669

[Bug rtl-optimization/33669] [4.3 Regression] Revision 128957 miscompiles 481.wrf

[hjl at lucon dot org]

------- Comment #1 from hjl at lucon dot org  2007-10-05 21:17 -------
netcdf/posixio.c is miscompiled.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33669

[Bug rtl-optimization/33669] [4.3 Regression] Revision 128957 miscompiles 481.wrf

[hjl at lucon dot org]

------- Comment #2 from hjl at lucon dot org  2007-10-05 21:49 -------
px_get in netcdf/posixio.c is miscompiled at -O2.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33669

[Bug rtl-optimization/33669] [4.3 Regression] Revision 128957 miscompiles 481.wrf

[hjl at lucon dot org]

------- Comment #3 from hjl at lucon dot org  2007-10-05 21:52 -------
px_get in netcdf/posixio.c is miscompiled at -O1 and -O0 is OK.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33669

[Bug rtl-optimization/33669] [4.3 Regression] Revision 128957 miscompiles 481.wrf

[hjl at lucon dot org]

------- Comment #4 from hjl at lucon dot org  2007-10-05 23:14 -------
#include <string.h>
#include <stdlib.h>

typedef struct foo_t { 
          size_t blksz;
          size_t  bf_extent;
         size_t  bf_cnt; 
          void    *bf_base;
} foo_t;


#define _RNDUP(x, unit)  ((((x) + (unit) - 1) / (unit)) \
        * (unit))
#define _RNDDOWN(x, unit)  ((x) - ((x)%(unit)))

long long
foo(foo_t *const pxp,  long long offset, size_t extent)
{
        const long long blkoffset = _RNDDOWN(offset,
                                             (long long )pxp->blksz);
        size_t diff = (size_t)(offset - blkoffset);
        size_t blkextent = _RNDUP(diff + extent, pxp->blksz);

        if(pxp->blksz < blkextent)
                return -1;
        {
                void *const middle = pxp->bf_base;
                if(pxp->bf_cnt > pxp->blksz)
                {
                        pxp->bf_extent = pxp->blksz;
                }
                if(pxp->bf_cnt > 0)
                {
                        /* copy lower half into upper half */
                        (void) memcpy(middle, pxp->bf_base, pxp->blksz);
                }
        }

        return  blkoffset;
}

int
main ()
{
  foo_t x;
  char buffer [8192];
  long long xx;

  x.blksz = 8192;
  x.bf_extent = 0;
  x.bf_cnt = 0;
  x.bf_base = buffer;
  xx = foo (&x, 0, 4096);
  if (xx != 0LL)
    abort ();
  return 0;
}
[hjl@gnu-27 pr33669]$ /export/gnu/import/rrs/128957/usr/bin/gcc  foo.c -m32 -O
[hjl@gnu-27 pr33669]$ ./a.out 
Aborted
[hjl@gnu-27 pr33669]$ 


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33669

[Bug rtl-optimization/33669] [4.3 Regression] Revision 128957 miscompiles 481.wrf

[hjl at lucon dot org]

------- Comment #5 from hjl at lucon dot org  2007-10-06 02:07 -------
Kenny, does your patch

http://gcc.gnu.org/ml/gcc-patches/2007-10/msg00124.html

handle cases where number of consecutive hard regs needed to hold some mode > 1
correctly? IA32 needs 2 hard registers to hold long long and your patch
miscompiles the testcase in comment #4.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33669

[Bug rtl-optimization/33669] [4.3 Regression] Revision 128957 miscompiles 481.wrf

[hjl at lucon dot org]

------- Comment #6 from hjl at lucon dot org  2007-10-06 02:31 -------
A smaller testcase:

bash-3.2$ cat x.c
extern void abort (void);

typedef struct foo_t
{ 
  unsigned int blksz;
  unsigned int bf_cnt; 
} foo_t;

#define _RNDUP(x, unit)  ((((x) + (unit) - 1) / (unit)) * (unit))
#define _RNDDOWN(x, unit)  ((x) - ((x)%(unit)))

long long
foo (foo_t *const pxp,  long long offset, unsigned int extent)
{
  long long blkoffset = _RNDDOWN(offset, (long long )pxp->blksz);
  unsigned int diff = (unsigned int)(offset - blkoffset);
  unsigned int blkextent = _RNDUP(diff + extent, pxp->blksz);

  if (pxp->blksz < blkextent)
    return -1LL;

  if (pxp->bf_cnt > pxp->blksz)
    pxp->bf_cnt = pxp->blksz;

  return blkoffset;
}

int
main ()
{
  foo_t x;
  long long xx;

  x.blksz = 8192;
  x.bf_cnt = 0;
  xx = foo (&x, 0, 4096);
  if (xx != 0LL)
    abort ();
  return 0;
}
bash-3.2$ ./xgcc -B./ -m32 -O x.c
bash-3.2$ ./a.out 
Aborted
bash-3.2$ 


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33669

[Bug rtl-optimization/33669] [4.3 Regression] Revision 128957 miscompiles 481.wrf

[zadeck at naturalbridge dot com]

------- Comment #7 from zadeck at naturalbridge dot com  2007-10-06 04:11 -------
Subject: Re:  [4.3 Regression]  Revision 128957
 miscompiles 481.wrf

hjl at lucon dot org wrote:
> ------- Comment #5 from hjl at lucon dot org  2007-10-06 02:07 -------
> Kenny, does your patch
>
> http://gcc.gnu.org/ml/gcc-patches/2007-10/msg00124.html
>
> handle cases where number of consecutive hard regs needed to hold some mode > 1
> correctly? IA32 needs 2 hard registers to hold long long and your patch
> miscompiles the testcase in comment #4.
>
>
>   
I will look into it.  It should do this correctly.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33669

[Bug rtl-optimization/33669] [4.3 Regression] Revision 128957 miscompiles 481.wrf

[hjl at lucon dot org]

------- Comment #8 from hjl at lucon dot org  2007-10-06 16:16 -------
(In reply to comment #7)
> Subject: Re:  [4.3 Regression]  Revision 128957
>  miscompiles 481.wrf
> 
> hjl at lucon dot org wrote:
> > ------- Comment #5 from hjl at lucon dot org  2007-10-06 02:07 -------
> > Kenny, does your patch
> >
> > http://gcc.gnu.org/ml/gcc-patches/2007-10/msg00124.html
> >
> > handle cases where number of consecutive hard regs needed to hold some mode > 1
> > correctly? IA32 needs 2 hard registers to hold long long and your patch
> > miscompiles the testcase in comment #4.
> >
> >
> >   
> I will look into it.  It should do this correctly.
> 

Can you take a look at

                  while (regno < last)
                    {
                      if ((!TEST_HARD_REG_BIT (hard_regs_live, regno))
                          && (!TEST_HARD_REG_BIT (renumbers_live, regno))
                          && ! fixed_regs[regno])
                        {
                          if (dump_file)
                            fprintf (dump_file, "    dying hard reg %d\n",
regno);
                          if (renumbering)
                            SET_HARD_REG_BIT (renumbers_live, regno);
                          else
                            SET_HARD_REG_BIT (hard_regs_live, regno);

                          added = true;
                        }
                      regno++;
                    }

in global_conflicts? Should it also check live_subregs before marking
a hard register is dying?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33669

[Bug rtl-optimization/33669] [4.3 Regression] Revision 128957 miscompiles 481.wrf

[zadeck at naturalbridge dot com]

------- Comment #9 from zadeck at naturalbridge dot com  2007-10-07 03:18 -------
Subject: Re:  [4.3 Regression]  Revision 128957
 miscompiles 481.wrf

hj,

here is a fix.  I will most likely post the patch on monday after i get
it really tested on a bunch of platforms.  The fix is in the third
stanza, the rest is better logging.

The failure only happens if you have a block with 2 or more uses of a
multiword pseudo register that is local to this block and has been
allocated by local_alloc.  The uses must be in a particular form: the
last use was a subreg use that only used some of the hard registers and
a previous non subreg use of the multiword register.

When all of this happens, the code did not properly expand this to a
whole multiregister when the second to last use is encounterd in the
backwards scan.

I.e. a lot of things have to happen to get this to fail.

Thanks for the small test case, that really helped.

Kenny



Index: ra-conflict.c
===================================================================
--- ra-conflict.c    (revision 129036)
+++ ra-conflict.c    (working copy)
@@ -76,7 +76,7 @@ record_one_conflict_between_regnos (enum
                     enum machine_mode mode2, int r2)
 {
   if (dump_file)
-    fprintf (dump_file, "  rocbr adding %d<=>%d\n", r1, r2);
+    fprintf (dump_file, "    rocbr adding %d<=>%d\n", r1, r2);
   if (reg_allocno[r1] >= 0 && reg_allocno[r2] >= 0)
     {
       int tr1 = reg_allocno[r1];
@@ -293,9 +293,6 @@ set_conflicts_for_earlyclobber (rtx insn
                             recog_data.operand[use + 1]);
         }
     }
-
-  if (dump_file)
-    fprintf (dump_file, "  finished early clobber conflicts.\n");
 }


@@ -876,7 +873,7 @@ global_conflicts (void)
                         allocnum, renumber);
             }

-          else if (GET_ALLOCNO_LIVE (allocnos_live, allocnum) == 0)
+          else
             {
               if (dump_file)
             fprintf (dump_file, "    dying pseudo\n");
@@ -963,6 +960,8 @@ global_conflicts (void)
          FIXME: We should consider either adding a new kind of
          clobber, or adding a flag to the clobber distinguish
          these two cases.  */
+          if (dump_file && VEC_length (df_ref_t, clobbers))
+        fprintf (dump_file, "  clobber conflicts\n");
           for (k = VEC_length (df_ref_t, clobbers) - 1; k >= 0; k--)
         {
           struct df_ref *def = VEC_index (df_ref_t, clobbers, k);
@@ -1024,6 +1023,8 @@ global_conflicts (void)
           if (GET_CODE (PATTERN (insn)) == PARALLEL && multiple_sets
(insn))
         {
           int j;
+          if (dump_file)
+            fprintf (dump_file, "  multiple sets\n");
           for (j = VEC_length (df_ref_t, dying_regs) - 1; j >= 0; j--)
             {
               int used_in_output = 0;


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33669

[Bug rtl-optimization/33669] [4.3 Regression] Revision 128957 miscompiles 481.wrf

[steven at gcc dot gnu dot org]

-- 

steven at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2007-10-07 09:41:07
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33669

[Bug rtl-optimization/33669] [4.3 Regression] Revision 128957 miscompiles 481.wrf

[zadeck at naturalbridge dot com]

-- 

zadeck at naturalbridge dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |zadeck at naturalbridge dot
                   |dot org                     |com
             Status|NEW                         |ASSIGNED
 GCC target triplet|                            |linux/ia32
   Last reconfirmed|2007-10-07 09:41:07         |2007-10-07 11:36:14
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33669

[Bug rtl-optimization/33669] [4.3 Regression] Revision 128957 miscompiles 481.wrf

[zadeck at naturalbridge dot com]

------- Comment #10 from zadeck at naturalbridge dot com  2007-10-07 21:57 -------
Subject: Re:  [4.3 Regression]  Revision 128957
 miscompiles 481.wrf

This patch fixes pr33669.

The failure only happens if you have a block with 2 or more uses of a
multiword pseudo register that is local to this block and has been
allocated by local_alloc.  The uses must be in a particular form: the
last use must be a subreg use that only used some of the hard registers and
a previous non subreg use of the multiword register.

When all of this happens, the code did not properly expand this to a
whole multiregister when the second to last use is encountered in the
backwards scan.

I.e. a lot of things have to happen to get this to fail.

I have tested this patch on ia-64, x86-{64,32} and ppc-32.

Ok for commit?

Kenny

2007-10-07  Kenneth Zadeck <zadeck@naturalbridge.com>

    PR middle-end/33669
    * ra-conflict.c (record_one_conflict_between_regnos,
    set_conflicts_for_earlyclobber, global_conflicts): Improved logging.
    (global_conflicts): Removed incorrect check.

2007-10-07  Kenneth Zadeck <zadeck@naturalbridge.com>

    PR middle-end/33669
    * gcc.c-torture/execute/pr33669.c: New.


Index: ra-conflict.c
===================================================================
--- ra-conflict.c       (revision 129053)
+++ ra-conflict.c       (working copy)
@@ -196,7 +196,7 @@ record_one_conflict_between_regnos (enum
   int allocno2 = reg_allocno[r2];

   if (dump_file)
-    fprintf (dump_file, "  rocbr adding %d<=>%d\n", r1, r2);
+    fprintf (dump_file, "    rocbr adding %d<=>%d\n", r1, r2);

   if (allocno1 >= 0 && allocno2 >= 0)
     set_conflict (allocno1, allocno2);
@@ -401,9 +401,6 @@ set_conflicts_for_earlyclobber (rtx insn
                                                    recog_data.operand[use +
1]);
                }
        }
-
-  if (dump_file) 
-    fprintf (dump_file, "  finished early clobber conflicts.\n");
 }


@@ -984,7 +981,7 @@ global_conflicts (void)
                                            allocnum, renumber);
                    }

-                 else if (!sparseset_bit_p (allocnos_live, allocnum))
+                 else
                    {
                      if (dump_file)
                        fprintf (dump_file, "    dying pseudo\n");
@@ -1071,6 +1068,8 @@ global_conflicts (void)
                 FIXME: We should consider either adding a new kind of
                 clobber, or adding a flag to the clobber distinguish
                 these two cases.  */
+             if (dump_file && VEC_length (df_ref_t, clobbers))
+               fprintf (dump_file, "  clobber conflicts\n");
              for (k = VEC_length (df_ref_t, clobbers) - 1; k >= 0; k--)
                {
                  struct df_ref *def = VEC_index (df_ref_t, clobbers, k);
@@ -1132,6 +1131,8 @@ global_conflicts (void)
              if (GET_CODE (PATTERN (insn)) == PARALLEL && multiple_sets
(insn))
                { 
                  int j;
+                 if (dump_file)
+                   fprintf (dump_file, "  multiple sets\n");
                  for (j = VEC_length (df_ref_t, dying_regs) - 1; j >= 0; j--)
                    {
                      int used_in_output = 0;
Index: testsuite/gcc.c-torture/execute/pr33669.c
===================================================================
--- testsuite/gcc.c-torture/execute/pr33669.c   (revision 0)
+++ testsuite/gcc.c-torture/execute/pr33669.c   (revision 0)
@@ -0,0 +1,40 @@
+extern void abort (void);
+
+typedef struct foo_t
+{ 
+  unsigned int blksz;
+  unsigned int bf_cnt; 
+} foo_t;
+
+#define _RNDUP(x, unit)  ((((x) + (unit) - 1) / (unit)) * (unit))
+#define _RNDDOWN(x, unit)  ((x) - ((x)%(unit)))
+
+long long
+foo (foo_t *const pxp,  long long offset, unsigned int extent)
+{
+  long long blkoffset = _RNDDOWN(offset, (long long )pxp->blksz);
+  unsigned int diff = (unsigned int)(offset - blkoffset);
+  unsigned int blkextent = _RNDUP(diff + extent, pxp->blksz);
+
+  if (pxp->blksz < blkextent)
+    return -1LL;
+
+  if (pxp->bf_cnt > pxp->blksz)
+    pxp->bf_cnt = pxp->blksz;
+
+  return blkoffset;
+}
+
+int
+main ()
+{
+  foo_t x;
+  long long xx;
+
+  x.blksz = 8192;
+  x.bf_cnt = 0;
+  xx = foo (&x, 0, 4096);
+  if (xx != 0LL)
+    abort ();
+  return 0;
+}


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33669

[Bug rtl-optimization/33669] [4.3 Regression] Revision 128957 miscompiles 481.wrf

[zadeck at naturalbridge dot com]

------- Comment #11 from zadeck at naturalbridge dot com  2007-10-08 03:53 -------
*** Bug 33662 has been marked as a duplicate of this bug. ***


-- 

zadeck at naturalbridge dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kkojima at gcc dot gnu dot
                   |                            |org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33669

[Bug rtl-optimization/33669] [4.3 Regression] Revision 128957 miscompiles 481.wrf

[seongbae dot park at gmail dot com]

------- Comment #12 from seongbae dot park at gmail dot com  2007-10-08 20:18 -------
Subject: Re:  [4.3 Regression] Revision 128957 miscompiles 481.wrf

Please remove the extra blank line above the line 984 (in new file).
This patch is OK.

Seongbae

On 10/7/07, Kenneth Zadeck <zadeck@naturalbridge.com> wrote:
> This patch fixes pr33669.
>
> The failure only happens if you have a block with 2 or more uses of a
> multiword pseudo register that is local to this block and has been
> allocated by local_alloc.  The uses must be in a particular form: the
> last use must be a subreg use that only used some of the hard registers and
> a previous non subreg use of the multiword register.
>
> When all of this happens, the code did not properly expand this to a
> whole multiregister when the second to last use is encountered in the
> backwards scan.
>
> I.e. a lot of things have to happen to get this to fail.
>
> I have tested this patch on ia-64, x86-{64,32} and ppc-32.
>
> Ok for commit?
>
> Kenny
>
> 2007-10-07  Kenneth Zadeck <zadeck@naturalbridge.com>
>
>     PR middle-end/33669
>     * ra-conflict.c (record_one_conflict_between_regnos,
>     set_conflicts_for_earlyclobber, global_conflicts): Improved logging.
>     (global_conflicts): Removed incorrect check.
>
> 2007-10-07  Kenneth Zadeck <zadeck@naturalbridge.com>
>
>     PR middle-end/33669
>     * gcc.c-torture/execute/pr33669.c: New.
>
>
>
>


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33669

[Bug rtl-optimization/33669] [4.3 Regression] Revision 128957 miscompiles 481.wrf

[hjl at gcc dot gnu dot org]

------- Comment #13 from hjl at gcc dot gnu dot org  2007-10-09 14:00 -------
Subject: Bug 33669

Author: hjl
Date: Tue Oct  9 14:00:11 2007
New Revision: 129166

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

2007-10-09  Kenneth Zadeck <zadeck@naturalbridge.com>

        PR middle-end/33669
        * ra-conflict.c (record_one_conflict_between_regnos,
        set_conflicts_for_earlyclobber, global_conflicts): Improved
        logging.
        (global_conflicts): Removed incorrect check.

gcc/testsuite/

2007-10-09  Kenneth Zadeck <zadeck@naturalbridge.com>

        PR middle-end/33669
        * gcc.c-torture/execute/pr33669.c: New.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr33669.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ra-conflict.c
    trunk/gcc/testsuite/ChangeLog


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33669

[Bug rtl-optimization/33669] [4.3 Regression] Revision 128957 miscompiles 481.wrf

[zadeck at naturalbridge dot com]

------- Comment #14 from zadeck at naturalbridge dot com  2007-10-09 15:32 -------
Subject: Re:  [4.3 Regression]  Revision 128957
 miscompiles 481.wrf

hjl at gcc dot gnu dot org wrote:
> ------- Comment #13 from hjl at gcc dot gnu dot org  2007-10-09 14:00 -------
> Subject: Bug 33669
>
> Author: hjl
> Date: Tue Oct  9 14:00:11 2007
> New Revision: 129166
>
> URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=129166
> Log:
> gcc/
>
> 2007-10-09  Kenneth Zadeck <zadeck@naturalbridge.com>
>
>         PR middle-end/33669
>         * ra-conflict.c (record_one_conflict_between_regnos,
>         set_conflicts_for_earlyclobber, global_conflicts): Improved
>         logging.
>         (global_conflicts): Removed incorrect check.
>
> gcc/testsuite/
>
> 2007-10-09  Kenneth Zadeck <zadeck@naturalbridge.com>
>
>         PR middle-end/33669
>         * gcc.c-torture/execute/pr33669.c: New.
>
> Added:
>     trunk/gcc/testsuite/gcc.c-torture/execute/pr33669.c
> Modified:
>     trunk/gcc/ChangeLog
>     trunk/gcc/ra-conflict.c
>     trunk/gcc/testsuite/ChangeLog
>
>
>   
please back this out.  i have a different patch that i have finished
testing. this one is too conservative.

kenny


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33669

[Bug rtl-optimization/33669] [4.3 Regression] Revision 128957 miscompiles 481.wrf

[zadeck at naturalbridge dot com]

------- Comment #15 from zadeck at naturalbridge dot com  2007-10-09 15:41 -------
Subject: Re:  [4.3 Regression]  Revision 128957
 miscompiles 481.wrf

This patch fixes the problem in a slightly different way.  The other
patch was too conservative in that it ended up setting the added flag
too often what has some downstream quality issues.

I just finished testing this on x86-64, x86-32, ppc-32 and ia-64

kenny


2007-10-07  Kenneth Zadeck <zadeck@naturalbridge.com>

    PR middle-end/33669
    * ra-conflict.c (record_one_conflict_between_regnos,
    set_conflicts_for_earlyclobber, global_conflicts): Improved logging.
    (global_conflicts): Removed incorrect check.

2007-10-07  Kenneth Zadeck <zadeck@naturalbridge.com>

    PR middle-end/33669
    * gcc.c-torture/execute/pr33669.c: New.


Index: ra-conflict.c
===================================================================
--- ra-conflict.c       (revision 129053)
+++ ra-conflict.c       (working copy)
@@ -196,7 +196,7 @@ record_one_conflict_between_regnos (enum
   int allocno2 = reg_allocno[r2];

   if (dump_file)
-    fprintf (dump_file, "  rocbr adding %d<=>%d\n", r1, r2);
+    fprintf (dump_file, "    rocbr adding %d<=>%d\n", r1, r2);

   if (allocno1 >= 0 && allocno2 >= 0)
     set_conflict (allocno1, allocno2);
@@ -401,9 +401,6 @@ set_conflicts_for_earlyclobber (rtx insn
                                                    recog_data.operand[use +
1]);
                }
        }
-
-  if (dump_file) 
-    fprintf (dump_file, "  finished early clobber conflicts.\n");
 }


@@ -983,12 +980,12 @@ global_conflicts (void)
                        set_renumbers_live (&renumbers_live, live_subregs,
live_subregs_used, 
                                            allocnum, renumber);
                    }
-                 
-                 else if (!sparseset_bit_p (allocnos_live, allocnum))
+                 else if (live_subregs_used[allocnum] > 0
+                          || !sparseset_bit_p (allocnos_live, allocnum))
                    {
                      if (dump_file)
-                       fprintf (dump_file, "    dying pseudo\n");
-                     
+                       fprintf (dump_file, "    %sdying pseudo\n", 
+                                (live_subregs_used[allocnum] > 0) ? "partially
": "");
                      /* Resetting the live_subregs_used is
                         effectively saying do not use the subregs
                         because we are reading the whole pseudo.  */
@@ -1071,6 +1068,8 @@ global_conflicts (void)
                 FIXME: We should consider either adding a new kind of
                 clobber, or adding a flag to the clobber distinguish
                 these two cases.  */
+             if (dump_file && VEC_length (df_ref_t, clobbers))
+               fprintf (dump_file, "  clobber conflicts\n");
              for (k = VEC_length (df_ref_t, clobbers) - 1; k >= 0; k--)
                {
                  struct df_ref *def = VEC_index (df_ref_t, clobbers, k);
@@ -1132,6 +1131,8 @@ global_conflicts (void)
              if (GET_CODE (PATTERN (insn)) == PARALLEL && multiple_sets
(insn))
                { 
                  int j;
+                 if (dump_file)
+                   fprintf (dump_file, "  multiple sets\n");
                  for (j = VEC_length (df_ref_t, dying_regs) - 1; j >= 0; j--)
                    {
                      int used_in_output = 0;
@@ -1166,7 +1167,7 @@ global_conflicts (void)
            }
        }

-           /* Add the renumbers live to the hard_regs_live for the next few
+      /* Add the renumbers live to the hard_regs_live for the next few
         calls.  All of this gets recomputed at the top of the loop so
         there is no harm.  */
       IOR_HARD_REG_SET (hard_regs_live, renumbers_live);
Index: testsuite/gcc.c-torture/execute/pr33669.c
===================================================================
--- testsuite/gcc.c-torture/execute/pr33669.c   (revision 0)
+++ testsuite/gcc.c-torture/execute/pr33669.c   (revision 0)
@@ -0,0 +1,40 @@
+extern void abort (void);
+
+typedef struct foo_t
+{ 
+  unsigned int blksz;
+  unsigned int bf_cnt; 
+} foo_t;
+
+#define _RNDUP(x, unit)  ((((x) + (unit) - 1) / (unit)) * (unit))
+#define _RNDDOWN(x, unit)  ((x) - ((x)%(unit)))
+
+long long
+foo (foo_t *const pxp,  long long offset, unsigned int extent)
+{
+  long long blkoffset = _RNDDOWN(offset, (long long )pxp->blksz);
+  unsigned int diff = (unsigned int)(offset - blkoffset);
+  unsigned int blkextent = _RNDUP(diff + extent, pxp->blksz);
+
+  if (pxp->blksz < blkextent)
+    return -1LL;
+
+  if (pxp->bf_cnt > pxp->blksz)
+    pxp->bf_cnt = pxp->blksz;
+
+  return blkoffset;
+}
+
+int
+main ()
+{
+  foo_t x;
+  long long xx;
+
+  x.blksz = 8192;
+  x.bf_cnt = 0;
+  xx = foo (&x, 0, 4096);
+  if (xx != 0LL)
+    abort ();
+  return 0;
+}


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33669

[Bug rtl-optimization/33669] [4.3 Regression] Revision 128957 miscompiles 481.wrf

[hjl at lucon dot org]

------- Comment #16 from hjl at lucon dot org  2007-10-09 16:19 -------
(In reply to comment #14)
> Subject: Re:  [4.3 Regression]  Revision 128957
>  miscompiles 481.wrf
> 
>
> please back this out.  i have a different patch that i have finished
> testing. this one is too conservative.
> 

I backed out the change to ra-conflict.c.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33669

[Bug rtl-optimization/33669] [4.3 Regression] Revision 128957 miscompiles 481.wrf

[zadeck at gcc dot gnu dot org]

------- Comment #17 from zadeck at gcc dot gnu dot org  2007-10-10 03:32 -------
Subject: Bug 33669

Author: zadeck
Date: Wed Oct 10 03:32:43 2007
New Revision: 129193

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=129193
Log:
2007-10-07  Kenneth Zadeck <zadeck@naturalbridge.com>

        PR middle-end/33669
        * ra-conflict.c (record_one_conflict_between_regnos, 
        set_conflicts_for_earlyclobber, global_conflicts): Improved logging.
        (global_conflicts): Enhanced incorrect check.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ra-conflict.c


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33669

[Bug rtl-optimization/33669] [4.3 Regression] Revision 128957 miscompiles 481.wrf

[zadeck at naturalbridge dot com]

------- Comment #18 from zadeck at naturalbridge dot com  2007-10-10 03:39 -------
Subject: Re:  [4.3 Regression]  Revision 128957
 miscompiles 481.wrf

HJ,

Sorry about the committing snafu.  I should have posted the irc log of
seonbae's comments to the log for the bug.  Also I had a meeting in the
city tonight, so there was not time to commit it between when seonbae
gave the final approval and when i had to catch my train.

I have committed the corrected patch as revision 129193.  It looks like
you had left the testcase when you reverted so there is no test case in
this patch.

This patch was tested on ia-64, ppc-32, xa6-{64,32}.

Kenny

> This patch fixes pr33669 <http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33669>.
>
> The failure only happens if you have a block with 2 or more uses of a
> multiword pseudo register that is local to this block and has been
> allocated by local_alloc.  The uses must be in a particular form: the
> last use must be a subreg use that only used some of the hard registers and
> a previous non subreg use of the multiword register.
>
> When all of this happens, the code did not properly expand this to a
> whole multiregister when the second to last use is encountered in the
> backwards scan.
>
> I.e. a lot of things have to happen to get this to fail.
>
> I have tested this patch on ia-64, x86-{64,32} and ppc-32.
>



2007-10-07  Kenneth Zadeck <zadeck@naturalbridge.com>

    PR middle-end/33669
    * ra-conflict.c (record_one_conflict_between_regnos,
    set_conflicts_for_earlyclobber, global_conflicts): Improved logging.
    (global_conflicts): Enhanced incorrect check.


Index: ra-conflict.c
===================================================================
--- ra-conflict.c       (revision 129192)
+++ ra-conflict.c       (working copy)
@@ -196,7 +196,7 @@ record_one_conflict_between_regnos (enum
   int allocno2 = reg_allocno[r2];

   if (dump_file)
-    fprintf (dump_file, "  rocbr adding %d<=>%d\n", r1, r2);
+    fprintf (dump_file, "    rocbr adding %d<=>%d\n", r1, r2);

   if (allocno1 >= 0 && allocno2 >= 0)
     set_conflict (allocno1, allocno2);
@@ -401,9 +401,6 @@ set_conflicts_for_earlyclobber (rtx insn
                                                    recog_data.operand[use +
1]);
                }
        }
-
-  if (dump_file) 
-    fprintf (dump_file, "  finished early clobber conflicts.\n");
 }


@@ -983,12 +980,12 @@ global_conflicts (void)
                        set_renumbers_live (&renumbers_live, live_subregs,
live_subregs_used, 
                                            allocnum, renumber);
                    }
-                 
-                 else if (!sparseset_bit_p (allocnos_live, allocnum))
+                 else if (live_subregs_used[allocnum] > 0
+                          || !sparseset_bit_p (allocnos_live, allocnum))
                    {
                      if (dump_file)
-                       fprintf (dump_file, "    dying pseudo\n");
-                     
+                       fprintf (dump_file, "    %sdying pseudo\n", 
+                                (live_subregs_used[allocnum] > 0) ? "partially
": "");
                      /* Resetting the live_subregs_used is
                         effectively saying do not use the subregs
                         because we are reading the whole pseudo.  */
@@ -1071,6 +1068,8 @@ global_conflicts (void)
                 FIXME: We should consider either adding a new kind of
                 clobber, or adding a flag to the clobber distinguish
                 these two cases.  */
+             if (dump_file && VEC_length (df_ref_t, clobbers))
+               fprintf (dump_file, "  clobber conflicts\n");
              for (k = VEC_length (df_ref_t, clobbers) - 1; k >= 0; k--)
                {
                  struct df_ref *def = VEC_index (df_ref_t, clobbers, k);
@@ -1132,6 +1131,8 @@ global_conflicts (void)
              if (GET_CODE (PATTERN (insn)) == PARALLEL && multiple_sets
(insn))
                { 
                  int j;
+                 if (dump_file)
+                   fprintf (dump_file, "  multiple sets\n");
                  for (j = VEC_length (df_ref_t, dying_regs) - 1; j >= 0; j--)
                    {
                      int used_in_output = 0;
@@ -1166,7 +1167,7 @@ global_conflicts (void)
            }
        }

-           /* Add the renumbers live to the hard_regs_live for the next few
+      /* Add the renumbers live to the hard_regs_live for the next few
         calls.  All of this gets recomputed at the top of the loop so
         there is no harm.  */
       IOR_HARD_REG_SET (hard_regs_live, renumbers_live);


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33669

[Bug rtl-optimization/33669] [4.3 Regression] Revision 128957 miscompiles 481.wrf

[zadeck at naturalbridge dot com]

------- Comment #19 from zadeck at naturalbridge dot com  2007-10-10 03:41 -------
patch committed to fix this.


-- 

zadeck at naturalbridge dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33669

[Bug rtl-optimization/33669] [4.3 Regression] Revision 128957 miscompiles 481.wrf

[hjl at lucon dot org]

------- Comment #20 from hjl at lucon dot org  2007-10-10 03:47 -------
Subject: Re:  [4.3 Regression]  Revision 128957
        miscompiles 481.wrf

On Tue, Oct 09, 2007 at 11:39:48PM -0400, Kenneth Zadeck wrote:
> HJ,
> 
> Sorry about the committing snafu.  I should have posted the irc log of
> seonbae's comments to the log for the bug.  Also I had a meeting in the
> city tonight, so there was not time to commit it between when seonbae
> gave the final approval and when i had to catch my train.
> 
> I have committed the corrected patch as revision 129193.  It looks like
> you had left the testcase when you reverted so there is no test case in
> this patch.
> 
> This patch was tested on ia-64, ppc-32, xa6-{64,32}.
> 

Does it also fix

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33676

which is caused by the same patch, revision 128957?

Thanks.


H.J.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33669