[Bug middle-end/33794] New: [Regression 4.3.0] Wrong code w/ -m32 -ffast-math -march=opteron

[Bug middle-end/33794] New: [Regression 4.3.0] Wrong code w/ -m32 -ffast-math -march=opteron

[burnus at gcc dot gnu dot org]

This is on x86_64-unknown-linux-gnu (AMD Athlon64 X2 4800+)
with     4.3.0 20071016 [trunk revision 129377].
Working: 4.3.0 20071015 [trunk revision 129311].

induct.f90 is part of the polyhedron test suite.

For
   gfortran -m32 -march=opteron -ffast-math -O1 induct.f90
the program ("induct") immediately aborts with:
  "bad mutual inductance between quad 1 and the wand transmit coil, abort."

The program works with "-m64" and with "-m32 -mfpmath=sse".

The Polyhedron test case can be obtained from:
http://www.polyhedron.co.uk/MFL6VW74649


-- 
           Summary: [Regression 4.3.0] Wrong code w/ -m32 -ffast-math -
                    march=opteron
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: burnus at gcc dot gnu dot org


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

[Bug middle-end/33794] Wrong code w/ -m32 -ffast-math -march=opteron

[pinskia at gcc dot gnu dot org]

------- Comment #1 from pinskia at gcc dot gnu dot org  2007-10-16 16:20 -------
Since this is using -ffast-math, i suspect this is not a bug.


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[Regression 4.3.0] Wrong    |Wrong code w/ -m32 -ffast-
                   |code w/ -m32 -ffast-math -  |math -march=opteron
                   |march=opteron               |


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

[Bug middle-end/33794] Wrong code w/ -m32 -ffast-math -march=opteron

[ubizjak at gmail dot com]

------- Comment #2 from ubizjak at gmail dot com  2007-10-17 09:10 -------
(In reply to comment #1)
> Since this is using -ffast-math, i suspect this is not a bug.

By adding a print before the condition (line 6372), it looks that something
goes seriously wrong:

gfortran -O2:

cpu time to define wand geometries/inductances =   0.003
cpu time to define res-q coil geometry/inductances =   0.004
 inductance =  -4.96596104015761125E-003
 inductance =  -5.80817784327351903E-003
 ...

gfortran -O2 -ffast-math:

cpu time to define wand geometries/inductances =   0.003
cpu time to define res-q coil geometry/inductances =   0.003
 inductance =   -3.2565300518352496     
 bad mutual inductance between quad            1  and the wand transmit coil,
abort.
 computed mutual inductance =   -3.2565300518352496     

The difference is 3 orders of magnitude!

I will run a bisection to isolate the change that introduced this failure.


-- 

ubizjak at gmail dot com changed:

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


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

[Bug middle-end/33794] [4.3 regression] Wrong code w/ -ffast-math

[ubizjak at gmail dot com]

------- Comment #3 from ubizjak at gmail dot com  2007-10-17 09:11 -------
Confirmed with plain -O2 -ffast-math on i686-pc-linux-gnu.


-- 

ubizjak at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Wrong code w/ -m32 -ffast-  |[4.3 regression] Wrong code
                   |math -march=opteron         |w/ -ffast-math


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

[Bug middle-end/33794] [4.3 regression] Wrong code w/ -ffast-math

[ubizjak at gmail dot com]

------- Comment #4 from ubizjak at gmail dot com  2007-10-17 10:23 -------
Bisection points to r129350:
http://gcc.gnu.org/viewcvs?view=rev&revision=129350

        PR tree-optimization/33619
        * tree-ssa-ter.c (is_replaceable_p): Return false for all
        calls.

        * gcc.dg/pr33619.c: New test.


-- 

ubizjak at gmail dot com changed:

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


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

[Bug middle-end/33794] [4.3 regression] Wrong code w/ -ffast-math

[pinskia at gcc dot gnu dot org]

------- Comment #5 from pinskia at gcc dot gnu dot org  2007-10-17 10:39 -------
And really that patch should not change anything really since it just means TER
does not cross function call boundaries now (and not just accross over non
pure/const function calls).

Can you at least give the final_clean for the file which is missed compiled for
both before and after the patch?  Since I doubt TER is doing something wrong.


-- 

pinskia at gcc dot gnu dot org changed:

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


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

[Bug middle-end/33794] [4.3 regression] Wrong code w/ -ffast-math

[pinskia at gcc dot gnu dot org]

------- Comment #6 from pinskia at gcc dot gnu dot org  2007-10-17 11:02 -------
Also what happens if you use -ffloat-store ?

Even though it might be 3 orders of magnitude, this could be still the same
issue as PR 323 (errors multiply in some cases).  Also note -ffloat-store
disables TER for floating point types.


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |WAITING


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

[Bug middle-end/33794] [4.3 regression] Wrong code w/ -ffast-math

[ubizjak at gmail dot com]

------- Comment #7 from ubizjak at gmail dot com  2007-10-17 11:38 -------
(In reply to comment #5)

There are (many!) changes of type:

-  D.6241 = __builtin_sqrt (receive_coil_1.self_ind * transmit_coil.self_ind);
-  coil_coil_mutuals[0] = coil_coil_mutuals[0] / D.6241;
+  coil_coil_mutuals[0] = coil_coil_mutuals[0] / __builtin_sqrt (receive_coil_1

where (-) represents failed run and (+) successful run, produced with Jakub's
patch removed.


-- 


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

[Bug middle-end/33794] [4.3 regression] Wrong code w/ -ffast-math

[ubizjak at gmail dot com]

------- Comment #8 from ubizjak at gmail dot com  2007-10-17 11:40 -------
(In reply to comment #6)
> Also what happens if you use -ffloat-store ?

-O2 -ffast-math -ffloat-store works OK...


-- 


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

[Bug middle-end/33794] [4.3 regression] Wrong code w/ -ffast-math

[pinskia at gcc dot gnu dot org]

------- Comment #9 from pinskia at gcc dot gnu dot org  2007-10-17 11:46 -------
I think what is happening is that "coil_coil_mutuals[0] / D.6241" is being
expanded as coil_coil_mutuals[0] * (1/D.6241) which is not bad, just
inconstaint.  I think we need to look into this more but I doubt there is
anything you can fix here.  This is just the way stuff works with -ffast-math
and x87.


-- 


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

[Bug middle-end/33794] [4.3 regression] Wrong code w/ -ffast-math

[ubizjak at gmail dot com]

------- Comment #10 from ubizjak at gmail dot com  2007-10-17 12:28 -------
(In reply to comment #9)

I don't want to claim excess precision problems here. I was trying to debug
self_ind_cir_coil procedure, as it looks that the problem is in calculation of
self_r output.

The input to this procedure is always the same:

  2.99999999999999989E-002
  3.00000000000000006E-003
  166666.66666666666
  1.25663706143591729E-006

but for -O2 -ffast-math -ffloat-store, self_l is returned as
 self_l  3.66008420600437631E-002

and for -O2 -ffast-math, self_l is returned as (wrong):
 self_l  8.51113565610957021E-008

I don't see why the code in between would produce such wildly different result.


-- 


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

[Bug middle-end/33794] [4.3 regression] Wrong code w/ -ffast-math

[ubizjak at gmail dot com]

------- Comment #11 from ubizjak at gmail dot com  2007-10-17 12:39 -------
The problem is somewhere around line 212 in induct.f90. Adding some debug code
at the end of the function:

...
!        evaluate self-inductance
!
      self_l = (mu * turns**2 * l**2 * 2.0_longreal * r)/3.0_longreal *        
            &
               (((tan(alpha)**2-1.0_longreal)*elliptice+elliptick)/sin(alpha) -
            &
               tan(alpha)**2)
!
      print *,"input", mu, turns, l, r, alpha, elliptice, elliptick
      print *,"result", self_l
      end subroutine self_ind_cir_coil

we obtain:

-O2 -ffast-math

 input  1.25663706143591729E-006   166666.66666666666      
3.00000000000000006E-003  2.99999999999999989E-002   1.5208379310729538       
1.0048458281970889        4.3853871539549072     
 result  8.51113565610956888E-008

-O2 -ffast-math -ffloat-store:

 input  1.25663706143591729E-006   166666.66666666666      
3.00000000000000006E-003  2.99999999999999989E-002   1.5208379310729538       
1.0048458281970889        4.3853871539548939     
 result  2.13170362012935939E-002

So, for the same input parameters, the equation above returns the result that
differs in 3 orders of magnitude.


-- 


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

[Bug target/33794] [4.3 regression] Wrong code w/ -ffast-math

[ubizjak at gmail dot com]

------- Comment #12 from ubizjak at gmail dot com  2007-10-17 13:57 -------
Well, the problem here is that swap insn is not emitted before x87 fptan.
It is actually my fault (UNSPEC_TAN handling is a bit wrong), the problem is
only exposed by Jakub's patch.

So, mine.


-- 

ubizjak at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |ubizjak at gmail dot com
                   |dot org                     |
             Status|WAITING                     |ASSIGNED
          Component|middle-end                  |target
 GCC target triplet|                            |i686-pc-linux-gnu
   Last reconfirmed|2007-10-17 09:10:16         |2007-10-17 13:57:25
               date|                            |


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

[Bug target/33794] [4.3 regression] Wrong code w/ -ffast-math

[ubizjak at gmail dot com]

------- Comment #13 from ubizjak at gmail dot com  2007-10-17 14:31 -------
Proposed patch in testing:

Index: reg-stack.c
===================================================================
--- reg-stack.c (revision 129402)
+++ reg-stack.c (working copy)
@@ -1085,11 +1085,13 @@
         special case with i387 UNSPEC_TAN, where destination is live
         (an argument to fptan) but inherent load of 1.0 is modelled
         as a load from a constant.  */
-      if (! (GET_CODE (pat) == PARALLEL
-            && XVECLEN (pat, 0) == 2
-            && GET_CODE (XVECEXP (pat, 0, 1)) == SET
-            && GET_CODE (SET_SRC (XVECEXP (pat, 0, 1))) == UNSPEC
-            && XINT (SET_SRC (XVECEXP (pat, 0, 1)), 1) == UNSPEC_TAN))
+      if (GET_CODE (pat) == PARALLEL
+         && XVECLEN (pat, 0) == 2
+         && GET_CODE (XVECEXP (pat, 0, 1)) == SET
+         && GET_CODE (SET_SRC (XVECEXP (pat, 0, 1))) == UNSPEC
+         && XINT (SET_SRC (XVECEXP (pat, 0, 1)), 1) == UNSPEC_TAN)
+       emit_swap_insn (insn, regstack, dest);
+      else
        gcc_assert (get_hard_regnum (regstack, dest) < FIRST_STACK_REG);

       gcc_assert (regstack->top < REG_STACK_SIZE);


-- 


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

[Bug target/33794] [4.3 regression] Wrong code w/ -ffast-math

[uros at gcc dot gnu dot org]

------- Comment #14 from uros at gcc dot gnu dot org  2007-10-17 15:22 -------
Subject: Bug 33794

Author: uros
Date: Wed Oct 17 15:22:23 2007
New Revision: 129406

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=129406
Log:
        PR middle-end/33794
        * reg-stack.c (move_for_stack_reg): Swap input argument of
        UNSPEC_TAN insn to the top of the stack.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/reg-stack.c


-- 


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

[Bug target/33794] [4.3 regression] Wrong code w/ -ffast-math

[uros at gcc dot gnu dot org]

------- Comment #15 from uros at gcc dot gnu dot org  2007-10-17 17:10 -------
Subject: Bug 33794

Author: uros
Date: Wed Oct 17 17:09:58 2007
New Revision: 129410

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=129410
Log:
        PR middle-end/33794
        * gfortran.dg/pr33794.f90: New testcase.


Added:
    trunk/gcc/testsuite/gfortran.dg/pr33794.f90
Modified:
    trunk/gcc/testsuite/ChangeLog


-- 


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

[Bug middle-end/33794] [4.3 regression] Wrong code w/ -ffast-math

[ubizjak at gmail dot com]

------- Comment #16 from ubizjak at gmail dot com  2007-10-17 17:11 -------
Fixed.


-- 

ubizjak at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
          Component|target                      |middle-end
         Resolution|                            |FIXED
   Target Milestone|---                         |4.3.0


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