[Bug middle-end/34852] New: [4.3 Regression] Revision 131576 miscompiled 178.galgel

[Bug middle-end/34852] New: [4.3 Regression] Revision 131576 miscompiled 178.galgel

[hjl dot tools at gmail dot com]

Revision 131576:

http://gcc.gnu.org/ml/gcc-cvs/2008-01/msg00337.html

miscompiled 178.galgel in SPEC CPU 2000 on Linux/ia32 with -O2 -ffast-math.
178.galgel went into a finite loop.


-- 
           Summary: [4.3 Regression]  Revision 131576 miscompiled 178.galgel
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Severity: critical
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: hjl dot tools at gmail dot com
 GCC build triplet: hubicka
GCC target triplet: i686-pc-linux-gnu


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

[Bug middle-end/34852] [4.3 Regression] Revision 131576 miscompiled 178.galgel

[rguenth at gcc dot gnu dot org]

------- Comment #1 from rguenth at gcc dot gnu dot org  2008-01-18 15:48 -------
Confirmed. (happened on haydn)


-- 

rguenth 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         |2008-01-18 15:48:05
               date|                            |


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

[Bug middle-end/34852] [4.3 Regression] Revision 131576 miscompiled 178.galgel

[pinskia at gcc dot gnu dot org]

------- Comment #2 from pinskia at gcc dot gnu dot org  2008-01-18 20:04 -------
Does it happen w/o -ffast-math ?

Also we need a testcase.


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|critical                    |normal
             Status|NEW                         |WAITING
   Target Milestone|---                         |4.3.0


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

[Bug middle-end/34852] [4.3 Regression] Revision 131576 miscompiled 178.galgel

[hubicka at gcc dot gnu dot org]

------- Comment #3 from hubicka at gcc dot gnu dot org  2008-01-20 11:56 -------
Double checking patch, I don't see obvious mistakes.  Since the patch should
only affect register allocation decisions, either we see a latent bug, or
another example of x86 extra precision causing FP program to misbehave.  I used
to have LD_PRELOAD library to set 64bit precision that often helped, I will
check this.

Honza


-- 


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

[Bug middle-end/34852] [4.3 Regression] Revision 131576 miscompiled 178.galgel

[ubizjak at gmail dot com]

------- Comment #4 from ubizjak at gmail dot com  2008-01-20 15:34 -------
(In reply to comment #3)
> Double checking patch, I don't see obvious mistakes.  Since the patch should
> only affect register allocation decisions, either we see a latent bug, or
> another example of x86 extra precision causing FP program to misbehave.  I used
> to have LD_PRELOAD library to set 64bit precision that often helped, I will

You can add -mpc=64 to your compile flags instead of LD_PRELOAD trick.


-- 


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

[Bug middle-end/34852] [4.3 Regression] Revision 131576 miscompiled 178.galgel

[hjl dot tools at gmail dot com]

------- Comment #5 from hjl dot tools at gmail dot com  2008-01-20 16:34 -------
Should we also update REG_FREQ_CALLS_CROSSED whenever REG_N_CALLS_CROSSED
is updated? In regmove.c, there are

              delete_insn (q);
              INC_REG_N_SETS (REGNO (src), -1);
              REG_N_CALLS_CROSSED (REGNO (src)) -= num_calls2;
              REG_LIVE_LENGTH (REGNO (src)) -= s_length2;
              insn_const = 0;
...
  if (src_note)
    {
      /* Move the death note for SRC from INSN to P.  */
      if (! overlap)
        remove_note (insn, src_note);
      XEXP (src_note, 1) = REG_NOTES (p);
      REG_NOTES (p) = src_note;

      REG_N_CALLS_CROSSED (REGNO (src)) += s_num_calls;
    }

  INC_REG_N_SETS (REGNO (src), 1);
  INC_REG_N_SETS (REGNO (dst), -1);

  REG_N_CALLS_CROSSED (REGNO (dst)) -= num_calls;

There is we only update REG_N_CALLS_CROSSED, not REG_FREQ_CALLS_CROSSED.


-- 


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

[Bug middle-end/34852] [4.3 Regression] Revision 131576 miscompiled 178.galgel

[hjl dot tools at gmail dot com]

------- Comment #6 from hjl dot tools at gmail dot com  2008-01-20 16:42 -------
Does this patch make any senses?

--- regmove.c.freq      2008-01-17 07:31:56.000000000 -0800
+++ regmove.c   2008-01-20 08:40:34.000000000 -0800
@@ -1695,7 +1695,7 @@ fixup_match_1 (rtx insn, rtx set, rtx sr
   rtx p;
   rtx post_inc = 0, post_inc_set = 0, search_end = 0;
   int success = 0;
-  int num_calls = 0, s_num_calls = 0;
+  int num_calls = 0, freq_calls = 0, s_num_calls = 0, s_freq_calls = 0;
   enum rtx_code code = NOTE;
   HOST_WIDE_INT insn_const = 0, newconst = 0;
   rtx overlap = 0; /* need to move insn ? */
@@ -1887,10 +1887,13 @@ fixup_match_1 (rtx insn, rtx set, rtx sr
            break;

          num_calls++;
+         freq_calls += REG_FREQ_FROM_BB  (BLOCK_FOR_INSN (p));

          if (src_note)
-           s_num_calls++;
-
+           {
+             s_num_calls++;
+             s_freq_calls += REG_FREQ_FROM_BB  (BLOCK_FOR_INSN (p));
+           }
        }
     }

@@ -1939,7 +1942,7 @@ fixup_match_1 (rtx insn, rtx set, rtx sr
     {
       rtx note = find_reg_note (insn, REG_EQUAL, NULL_RTX);
       rtx q, set2 = NULL_RTX;
-      int num_calls2 = 0, s_length2 = 0;
+      int num_calls2 = 0, s_length2 = 0, freq_calls2 = 0;

       if (note && CONSTANT_P (XEXP (note, 0)))
        {
@@ -1968,7 +1971,10 @@ fixup_match_1 (rtx insn, rtx set, rtx sr
                  break;
                }
              if (CALL_P (p))
-               num_calls2++;
+               {
+                 num_calls2++;
+                 freq_calls2 += REG_FREQ_FROM_BB  (BLOCK_FOR_INSN (p));
+               }
            }
          if (q && set2 && SET_DEST (set2) == src && CONSTANT_P (SET_SRC
(set2))
              && validate_change (insn, &SET_SRC (set), XEXP (note, 0), 0))
@@ -1976,6 +1982,7 @@ fixup_match_1 (rtx insn, rtx set, rtx sr
              delete_insn (q);
              INC_REG_N_SETS (REGNO (src), -1);
              REG_N_CALLS_CROSSED (REGNO (src)) -= num_calls2;
+             REG_FREQ_CALLS_CROSSED (REGNO (src)) -= freq_calls2;
              REG_LIVE_LENGTH (REGNO (src)) -= s_length2;
              insn_const = 0;
            }
@@ -2044,12 +2051,14 @@ fixup_match_1 (rtx insn, rtx set, rtx sr
       REG_NOTES (p) = src_note;

       REG_N_CALLS_CROSSED (REGNO (src)) += s_num_calls;
+      REG_FREQ_CALLS_CROSSED (REGNO (src)) += s_freq_calls;
     }

   INC_REG_N_SETS (REGNO (src), 1);
   INC_REG_N_SETS (REGNO (dst), -1);

   REG_N_CALLS_CROSSED (REGNO (dst)) -= num_calls;
+  REG_FREQ_CALLS_CROSSED (REGNO (dst)) -= num_calls;

   REG_LIVE_LENGTH (REGNO (src)) += s_length;
   if (REG_LIVE_LENGTH (REGNO (dst)) >= 0)


-- 


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

[Bug middle-end/34852] [4.3 Regression] Revision 131576 miscompiled 178.galgel

[hjl dot tools at gmail dot com]

------- Comment #7 from hjl dot tools at gmail dot com  2008-01-20 16:43 -------
Oops. This one

--- regmove.c.freq      2008-01-17 07:31:56.000000000 -0800
+++ regmove.c   2008-01-20 08:42:42.000000000 -0800
@@ -1695,7 +1695,7 @@ fixup_match_1 (rtx insn, rtx set, rtx sr
   rtx p;
   rtx post_inc = 0, post_inc_set = 0, search_end = 0;
   int success = 0;
-  int num_calls = 0, s_num_calls = 0;
+  int num_calls = 0, freq_calls = 0, s_num_calls = 0, s_freq_calls = 0;
   enum rtx_code code = NOTE;
   HOST_WIDE_INT insn_const = 0, newconst = 0;
   rtx overlap = 0; /* need to move insn ? */
@@ -1887,10 +1887,13 @@ fixup_match_1 (rtx insn, rtx set, rtx sr
            break;

          num_calls++;
+         freq_calls += REG_FREQ_FROM_BB  (BLOCK_FOR_INSN (p));

          if (src_note)
-           s_num_calls++;
-
+           {
+             s_num_calls++;
+             s_freq_calls += REG_FREQ_FROM_BB  (BLOCK_FOR_INSN (p));
+           }
        }
     }

@@ -1939,7 +1942,7 @@ fixup_match_1 (rtx insn, rtx set, rtx sr
     {
       rtx note = find_reg_note (insn, REG_EQUAL, NULL_RTX);
       rtx q, set2 = NULL_RTX;
-      int num_calls2 = 0, s_length2 = 0;
+      int num_calls2 = 0, s_length2 = 0, freq_calls2 = 0;

       if (note && CONSTANT_P (XEXP (note, 0)))
        {
@@ -1968,7 +1971,10 @@ fixup_match_1 (rtx insn, rtx set, rtx sr
                  break;
                }
              if (CALL_P (p))
-               num_calls2++;
+               {
+                 num_calls2++;
+                 freq_calls2 += REG_FREQ_FROM_BB  (BLOCK_FOR_INSN (p));
+               }
            }
          if (q && set2 && SET_DEST (set2) == src && CONSTANT_P (SET_SRC
(set2))
              && validate_change (insn, &SET_SRC (set), XEXP (note, 0), 0))
@@ -1976,6 +1982,7 @@ fixup_match_1 (rtx insn, rtx set, rtx sr
              delete_insn (q);
              INC_REG_N_SETS (REGNO (src), -1);
              REG_N_CALLS_CROSSED (REGNO (src)) -= num_calls2;
+             REG_FREQ_CALLS_CROSSED (REGNO (src)) -= freq_calls2;
              REG_LIVE_LENGTH (REGNO (src)) -= s_length2;
              insn_const = 0;
            }
@@ -2044,12 +2051,14 @@ fixup_match_1 (rtx insn, rtx set, rtx sr
       REG_NOTES (p) = src_note;

       REG_N_CALLS_CROSSED (REGNO (src)) += s_num_calls;
+      REG_FREQ_CALLS_CROSSED (REGNO (src)) += s_freq_calls;
     }

   INC_REG_N_SETS (REGNO (src), 1);
   INC_REG_N_SETS (REGNO (dst), -1);

   REG_N_CALLS_CROSSED (REGNO (dst)) -= num_calls;
+  REG_FREQ_CALLS_CROSSED (REGNO (dst)) -= freq_calls;

   REG_LIVE_LENGTH (REGNO (src)) += s_length;
   if (REG_LIVE_LENGTH (REGNO (dst)) >= 0)


-- 


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

[Bug middle-end/34852] [4.3 Regression] Revision 131576 miscompiled 178.galgel

[hjl dot tools at gmail dot com]

------- Comment #8 from hjl dot tools at gmail dot com  2008-01-21 03:09 -------
Add -ffloat-store seems to fix the problem. I will verify it.


-- 


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

[Bug middle-end/34852] [4.3 Regression] Revision 131576 miscompiled 178.galgel

[hubicka at ucw dot cz]

------- Comment #9 from hubicka at ucw dot cz  2008-01-21 09:43 -------
Subject: Re:  [4.3 Regression]  Revision 131576 miscompiled 178.galgel

> 
> 
> ------- Comment #7 from hjl dot tools at gmail dot com  2008-01-20 16:43 -------
> Oops. This one

Yes, it does make sense.  I must've missed it, since I was updating
similar cases all around the file.
It should not be code correcntess issue, just code quality - we still
rely on REG_N_CALLS when asking if register crosses a call, just use
frequency to drive decision on profitability of using caller save
register.

Thanks!  I will also look into the two regressions this afternoon unless
you beat me.

Honza


-- 


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

[Bug middle-end/34852] [4.3 Regression] Revision 131576 miscompiled 178.galgel

[hubicka at ucw dot cz]

------- Comment #10 from hubicka at ucw dot cz  2008-01-21 09:44 -------
Subject: Re:  [4.3 Regression]  Revision 131576 miscompiled 178.galgel

> 
> 
> ------- Comment #8 from hjl dot tools at gmail dot com  2008-01-21 03:09 -------
> Add -ffloat-store seems to fix the problem. I will verify it.

I also found that, but since -ffloat-store changes almost all register
allocation, it is dificult to tell if it made just hid the bug or not :(


-- 


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

[Bug middle-end/34852] [4.3 Regression] Revision 131576 miscompiled 178.galgel

[hubicka at ucw dot cz]

------- Comment #11 from hubicka at ucw dot cz  2008-01-21 09:48 -------
Subject: Re:  [4.3 Regression]  Revision 131576 miscompiled 178.galgel

Hi,
also one extra data point, britten tester that uses -O3
-fomit-frame-pointer -ftree-loop-linear -funroll-all-loops
-fprefetch-loop-arrays -march=k8 -mfpmath=sse -ffast-math is not getting
the faiulre at 32bit runs...

Honza


-- 


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

[Bug middle-end/34852] [4.3 Regression] Revision 131576 miscompiled 178.galgel

[hubicka at ucw dot cz]

------- Comment #12 from hubicka at ucw dot cz  2008-01-21 09:54 -------
Subject: Re:  [4.3 Regression]  Revision 131576 miscompiled 178.galgel

.... and haydn tester using -O3 -funroll-loops -fpeel-loops -ffast-math
-march=native -mtune=native -mfpmath=sse has also started failing at
17th, so this should rule out the extra precision theory :(

Honza


-- 


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

[Bug middle-end/34852] [4.3 Regression] Revision 131576 miscompiled 178.galgel

[hjl dot tools at gmail dot com]

------- Comment #13 from hjl dot tools at gmail dot com  2008-01-21 15:14 -------
I tried -mpc64. It also works.


-- 


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

[Bug middle-end/34852] [4.3 Regression] Revision 131576 miscompiled 178.galgel

[hjl dot tools at gmail dot com]

------- Comment #14 from hjl dot tools at gmail dot com  2008-01-21 15:51 -------
(In reply to comment #12)
> Subject: Re:  [4.3 Regression]  Revision 131576 miscompiled 178.galgel
> 
> .... and haydn tester using -O3 -funroll-loops -fpeel-loops -ffast-math
> -march=native -mtune=native -mfpmath=sse has also started failing at
> 17th, so this should rule out the extra precision theory :(
>

Did you compile input files with the same command line? -march=native
doesn't work with more than one input file. See bug 34904. Only the
first file will use SSE math.


-- 

hjl dot tools at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  BugsThisDependsOn|                            |34904


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

[Bug middle-end/34852] [4.3 Regression] Revision 131576 miscompiled 178.galgel

[hubicka at ucw dot cz]

------- Comment #15 from hubicka at ucw dot cz  2008-01-21 16:42 -------
Subject: Re:  [4.3 Regression]  Revision 131576 miscompiled 178.galgel

> I tried -mpc64. It also works.
I would declare this a proof that it is extra preccision issue and
simply update testers to use -mpc64.  It is what most of other compilers
do anyway and thus we would get more comparable scores. Thanks a lot for
testing it (I've scheduled same test for tonight, but you've beaten me.
I still will try if it works for -mfpmath=sse case too)

Honza


-- 


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

[Bug middle-end/34852] [4.3 Regression] Revision 131576 miscompiled 178.galgel

[hjl dot tools at gmail dot com]

------- Comment #16 from hjl dot tools at gmail dot com  2008-01-21 20:53 -------
(In reply to comment #15)
> Subject: Re:  [4.3 Regression]  Revision 131576 miscompiled 178.galgel
> 
> > I tried -mpc64. It also works.
> I would declare this a proof that it is extra preccision issue and
> simply update testers to use -mpc64.  It is what most of other compilers
> do anyway and thus we would get more comparable scores. Thanks a lot for
> testing it (I've scheduled same test for tonight, but you've beaten me.
> I still will try if it works for -mfpmath=sse case too)
> 

With the -march=native patch:

http://gcc.gnu.org/ml/gcc-patches/2008-01/msg00969.html

-funroll-loops -fpeel-loops -ffast-math -march=native -mtune=native
-mfpmath=sse 
works for me.
> 


-- 


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

[Bug middle-end/34852] [4.3 Regression] Revision 131576 miscompiled 178.galgel

[rguenth at gcc dot gnu dot org]

------- Comment #17 from rguenth at gcc dot gnu dot org  2008-01-22 20:09 -------
So, we indeed think this issue is invalid, right?


-- 


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

[Bug middle-end/34852] [4.3 Regression] Revision 131576 miscompiled 178.galgel

[hubicka at ucw dot cz]

------- Comment #18 from hubicka at ucw dot cz  2008-01-22 20:12 -------
Subject: Re:  [4.3 Regression]  Revision 131576 miscompiled 178.galgel

> So, we indeed think this issue is invalid, right?
I am convinced so. -mpc64 does not change code generation.  I messed up
the change of britten flags to include -mpc64 by default so I am waiting
for tonight runs if failure is consistently gone.

Honza


-- 


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

[Bug middle-end/34852] [4.3 Regression] Revision 131576 miscompiled 178.galgel

[rguenth at gcc dot gnu dot org]

------- Comment #19 from rguenth at gcc dot gnu dot org  2008-01-23 22:21 -------
Run is fine, invalid.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |RESOLVED
         Resolution|                            |INVALID


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