[Bug fortran/35946] New: wrong result with array constructor as argument to ATAN2

[Bug fortran/35946] New: wrong result with array constructor as argument to ATAN2

[dick dot hendrickson at gmail dot com]

The following program gives the wrong answers when an array
constructor with a "complicated" implied do is used as an
argument to REAL which is then used as an argument to
ATAN2.

Dick Hendrickson 

      program try_fa6013

! fails on Windows XP
! gcc version 4.4.0 20080312 (experimental) [trunk revision 133139]

      call fa6013(10,1,-1)
      end program


      subroutine  FA6013 (nf10,nf1,mf1)

      integer, parameter :: kv=4    !fails
!     integer, parameter :: kv=8    !fails

      REAL(KV) DDA1(10)
      REAL(KV) DDA2(10)
      REAL(KV) DDA(10), dval

      dda = (/ 1,2,3,4,5,6,7,8,9,10/)

      print '(10f5.1)',   REAL((/(J1,J1=nf10,nf1,mf1)/),KV) !works 


       DDA1 = ATAN2 (  (/(REAL(J1,KV),J1=1,10)/) ,
     $                 REAL((/(J1,J1=nf10,nf1,mf1)/),KV))   !fails

!    $                 REAL((/(J1,J1=  10,  1, -1)/),KV))   !works

       DDA2 = ATAN2 ( DDA, DDA(10:1:-1) )

      DVAL = .09967_kv     !atan2(1.0,10.0)
      print '(i3,3f10.5)', 1, dda1(1),dval, dval-dda1(1)

      DO J1 = 2,10
      DVAL = DDA2(J1)
      print '(i3,3f10.5)', j1, dda1(j1),dval, dval-dda1(j1)
  100 ENDDO

      END

c:\gfortran:gfortran fa6013.f

c:\gfortran:a
 10.0  9.0  8.0  7.0  6.0  5.0  4.0  3.0  2.0  1.0
  1   0.11066   0.09967  -0.01099
  2   0.24498   0.21867  -0.02631
  3   0.40489   0.35877  -0.04612
  4   0.58800   0.51915  -0.06886
  5   0.78540   0.69474  -0.09066
  6   0.98279   0.87606  -0.10674
  7   1.16590   1.05165  -0.11425
  8   1.32582   1.21203  -0.11379
  9   1.46014   1.35213  -0.10801
 10   0.00000   1.47113   1.47113


-- 
           Summary: wrong result with array constructor as argument to ATAN2
           Product: gcc
           Version: 4.4.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: dick dot hendrickson at gmail dot com


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

[Bug fortran/35946] wrong result with array constructor as argument to ATAN2

[tkoenig at gcc dot gnu dot org]

------- Comment #1 from tkoenig at gcc dot gnu dot org  2008-04-16 18:36 -------
Confirmed, a regression vs. 4.2.3:

$ gfortran fa.f 
$ ./a.out
 10.0  9.0  8.0  7.0  6.0  5.0  4.0  3.0  2.0  1.0
  1   0.11066   0.09967  -0.01099
  2   0.24498   0.21867  -0.02631
  3   0.40489   0.35877  -0.04612
  4   0.58800   0.51915  -0.06886
  5   0.78540   0.69474  -0.09066
  6   0.98279   0.87606  -0.10674
  7   1.16590   1.05165  -0.11425
  8   1.32582   1.21203  -0.11379
  9   1.46014   1.35213  -0.10801
 10   3.14159   1.47113  -1.67047
$ ifort fa.f 
$ ./a.out
 10.0  9.0  8.0  7.0  6.0  5.0  4.0  3.0  2.0  1.0
  1   0.09967   0.09967   0.00000
  2   0.21867   0.21867   0.00000
  3   0.35877   0.35877   0.00000
  4   0.51915   0.51915   0.00000
  5   0.69474   0.69474   0.00000
  6   0.87606   0.87606   0.00000
  7   1.05165   1.05165   0.00000
  8   1.21203   1.21203   0.00000
  9   1.35213   1.35213   0.00000
 10   1.47113   1.47113   0.00000
$ gfortran-4.2 fa.f 
$ ./a.out
 10.0  9.0  8.0  7.0  6.0  5.0  4.0  3.0  2.0  1.0
  1   0.09967   0.09967   0.00000
  2   0.21867   0.21867   0.00000
  3   0.35877   0.35877   0.00000
  4   0.51915   0.51915   0.00000
  5   0.69474   0.69474   0.00000
  6   0.87606   0.87606   0.00000
  7   1.05165   1.05165   0.00000
  8   1.21203   1.21203   0.00000
  9   1.35213   1.35213   0.00000
 10   1.47113   1.47113   0.00000
$ gfortran-4.3 fa.f 
$ ./a.out
 10.0  9.0  8.0  7.0  6.0  5.0  4.0  3.0  2.0  1.0
  1   0.11066   0.09967  -0.01099
  2   0.24498   0.00000  -0.24498
  3   0.40489   0.00000  -0.40489
  4   0.58800   0.00000  -0.58800
  5   0.78540   0.00000  -0.78540
  6   0.98279   0.87606  -0.10674
  7   1.16590   1.05165  -0.11425
  8   1.32582   1.21203  -0.11379
  9   1.46014   1.35213  -0.10801
 10   3.14159   1.47113  -1.67047


-- 

tkoenig at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
      Known to fail|                            |4.3.1 4.4.0
      Known to work|                            |4.2.3
   Last reconfirmed|0000-00-00 00:00:00         |2008-04-16 18:36:40
               date|                            |
   Target Milestone|---                         |4.3.1


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

[Bug fortran/35946] [4.3, 4.4 regression] wrong result with array constructor as argument to ATAN2

[ubizjak at gmail dot com]

------- Comment #2 from ubizjak at gmail dot com  2008-04-17 08:21 -------
Marked as a regression.


-- 

ubizjak at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|wrong result with array     |[4.3, 4.4 regression] wrong
                   |constructor as argument to  |result with array
                   |ATAN2                       |constructor as argument to
                   |                            |ATAN2


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

[Bug fortran/35946] [4.3, 4.4 regression] wrong result with array constructor as argument to ATAN2

[ubizjak at gmail dot com]

------- Comment #3 from ubizjak at gmail dot com  2008-04-17 08:56 -------
The second argument, passed to ATAN2 is off by one, i.e. instead of
atan2(1,10), atan2 (1,9) is passed.

atan2 (1,10) = 0.09967 (* expected)
atan2 (1,9) =  0.11066


-- 


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

[Bug fortran/35946] [4.3, 4.4 regression] wrong result with array constructor as argument to ATAN2

[ubizjak at gmail dot com]

------- Comment #4 from ubizjak at gmail dot com  2008-04-17 09:15 -------
The difference between non-working (---) and working (+++) asm is:

--- pr35946.s_  2008-04-17 11:07:19.000000000 +0200
+++ pr35946.s   2008-04-17 11:07:28.000000000 +0200
@@ -188,7 +188,7 @@
        .p2align 3
 .L16:
        movl    A.7.603(%ebx), %eax
-       fildl   4(%edi,%ebx)
+       fildl   (%edi,%ebx)
        fstps   4(%esp)
        movl    %eax, (%esp)
        call    atan2f

The problem can be seen in .003.t.orignal:

        S.12 = 1;
        while (1)
          {
            if (S.12 > 10) goto L.4;
            dda1[S.12 + -1] = __builtin_atan2f (A.7[S.12 + -1], (real(kind=4))
(*(integer(kind=4)[10] *) atmp.8.data)[S.12]);
            S.12 = S.12 + 1;
          }

Please note off-by one in the last index.


-- 


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

[Bug fortran/35946] [4.3, 4.4 regression] wrong result with array constructor as argument to ATAN2

[jakub at gcc dot gnu dot org]

-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P4


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

[Bug fortran/35946] [4.3, 4.4 regression] wrong result with array constructor as argument to ATAN2

[pault at gcc dot gnu dot org]

------- Comment #5 from pault at gcc dot gnu dot org  2008-04-18 21:02 -------
(In reply to comment #4)

> 
> Please note off-by one in the last index.
>

Uros,

Indeed:)  I'm snooping round trying to understand why the scalarizer has taken
to doing that.  Not only is the offset wrong, but the size of the temporary
array is one more than needed.

*sigh*

Paul


-- 


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

[Bug fortran/35946] [4.3, 4.4 regression] wrong result with array constructor as argument to ATAN2

[pault at gcc dot gnu dot org]

------- Comment #6 from pault at gcc dot gnu dot org  2008-04-19 08:45 -------
This fixes it, albeit with one regression (scalarize_parameter_array_1.f90):

Index: gcc/fortran/trans-array.c
===================================================================
*** gcc/fortran/trans-array.c   (revision 134362)
--- gcc/fortran/trans-array.c   (working copy)
*************** gfc_trans_array_constructor (gfc_loopinf
*** 1679,1684 ****
--- 1679,1685 ----
    tree offsetvar;
    tree desc;
    tree type;
+   tree loopfrom;
    bool dynamic;

    if (flag_bounds_check && ss->expr->ts.type == BT_CHARACTER)
*************** gfc_trans_array_constructor (gfc_loopinf
*** 1757,1765 ****
--- 1758,1783 ----
        }
      }

+   if (!integer_zerop (loop->from[0]))
+     {
+       loopfrom = loop->from[0];
+       loop->from[0] = gfc_index_zero_node;
+       loop->to[0] = fold_build2 (MINUS_EXPR, gfc_array_index_type,
+                                loop->to[0], loopfrom);
+     }
+   else
+     loopfrom = NULL_TREE;
+ 
    gfc_trans_create_temp_array (&loop->pre, &loop->post, loop, &ss->data.info,
                               type, dynamic, true, false);

+   if (loopfrom != NULL_TREE)
+     {
+       loop->from[0] = loopfrom;
+       loop->to[0] = fold_build2 (PLUS_EXPR, gfc_array_index_type,
+                                loop->to[0], loopfrom);
+     }
+ 
    desc = ss->data.info.descriptor;
    offset = gfc_index_zero_node;
    offsetvar = gfc_create_var_np (gfc_array_index_type, "offset");
*************** gfc_trans_array_constructor (gfc_loopinf
*** 1768,1773 ****
--- 1786,1794 ----
    gfc_trans_array_constructor_value (&loop->pre, type, desc, c,
                                     &offset, &offsetvar, dynamic);

+   ss->data.info.offset = fold_build1 (NEGATE_EXPR, gfc_array_index_type,
+                                     loop->from[0]);
+ 
    /* If the array grows dynamically, the upper bound of the loop variable
       is determined by the array's final upper bound.  */
    if (dynamic)

Somewhere along the line, the scalarizer has undergone a change in the way in
which the loop ranges are chosen.  In 4.2 and earlier, the loop->from[0] is
zero and this gives a temporary with the right offset and lbound.

I'll figure out why the regression occurs and see if there is a cleaner way of
effecting the same modification.

Paul


-- 

pault at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |pault at gcc dot gnu dot org
                   |dot org                     |
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2008-04-16 18:36:40         |2008-04-19 08:45:44
               date|                            |


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

[Bug fortran/35946] [4.3, 4.4 regression] wrong result with array constructor as argument to ATAN2

[pault at gcc dot gnu dot org]

------- Comment #7 from pault at gcc dot gnu dot org  2008-04-19 21:56 -------
Subject: Bug 35946

Author: pault
Date: Sat Apr 19 21:55:24 2008
New Revision: 134472

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=134472
Log:
2008-04-19  Paul Thomas  <pault@gcc.gnu.org>

        PR fortran/35944
        PR fortran/35946
        PR fortran/35947
        * trans_array.c (gfc_trans_array_constructor): Temporarily
        realign loop, if loop->from is not zero, before creating
        the temporary array and provide an offset.

        PR fortran/35959
        * trans-decl.c (gfc_init_default_dt): Add gfc_ prefix to name
        and allow for NULL body.  Change all references from
        init_default_dt to gfc_init_default_dt.
        * trans.h : Add prototype for gfc_init_default_dt.
        * trans-array.c (gfc_trans_deferred_vars): After nullification
        call gfc_init_default_dt for derived types with allocatable
        components.

2008-04-19  Paul Thomas  <pault@gcc.gnu.org>

        PR fortran/35944
        PR fortran/35946
        PR fortran/35947
        * gfortran.dg/array_constructor_23.f: New test.

        PR fortran/35959
        * gfortran.dg/alloc_comp_default_init_2.f90: New test.
        * gfortran.dg/alloc_comp_basics_1.f90: Change occurrences of
        "builtin_free" to 27.
        * gfortran.dg/alloc_comp_constructor_1.f90: Change occurrences
        of "builtin_free" to 21.

Added:
    trunk/gcc/testsuite/gfortran.dg/alloc_comp_default_init_2.f90
    trunk/gcc/testsuite/gfortran.dg/array_constructor_23.f
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-array.c
    trunk/gcc/fortran/trans-decl.c
    trunk/gcc/fortran/trans.h
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/alloc_comp_basics_1.f90
    trunk/gcc/testsuite/gfortran.dg/alloc_comp_constructor_1.f90


-- 


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

[Bug fortran/35946] [4.3, 4.4 regression] wrong result with array constructor as argument to ATAN2

[pault at gcc dot gnu dot org]

------- Comment #8 from pault at gcc dot gnu dot org  2008-04-19 22:31 -------
Subject: Bug 35946

Author: pault
Date: Sat Apr 19 22:30:03 2008
New Revision: 134475

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=134475
Log:
2008-04-19  Paul Thomas  <pault@gcc.gnu.org>

        PR fortran/35944
        PR fortran/35946
        PR fortran/35947
        * trans_array.c (gfc_trans_array_constructor): Temporarily
        realign loop, if loop->from is not zero, before creating
        the temporary array and provide an offset.

        PR fortran/35959
        * trans-decl.c (gfc_init_default_dt): Add gfc_ prefix to name
        and allow for NULL body.  Change all references from
        init_default_dt to gfc_init_default_dt.
        * trans.h : Add prototype for gfc_init_default_dt.
        * trans-array.c (gfc_trans_deferred_vars): After nullification
        call gfc_init_default_dt for derived types with allocatable
        components.

2008-04-19  Paul Thomas  <pault@gcc.gnu.org>

        PR fortran/35944
        PR fortran/35946
        PR fortran/35947
        * gfortran.dg/array_constructor_23.f: New test.

        PR fortran/35959
        * gfortran.dg/alloc_comp_default_init_2.f90: New test.
        * gfortran.dg/alloc_comp_basics_1.f90: Change occurrences of
        "builtin_free" to 27.
        * gfortran.dg/alloc_comp_constructor_1.f90: Change occurrences
        of "builtin_free" to 21.

Added:
   
branches/gcc-4_3-branch/gcc/testsuite/gfortran.dg/alloc_comp_default_init_2.f90
    branches/gcc-4_3-branch/gcc/testsuite/gfortran.dg/array_constructor_23.f
Modified:
    branches/gcc-4_3-branch/gcc/fortran/ChangeLog
    branches/gcc-4_3-branch/gcc/fortran/trans-array.c
    branches/gcc-4_3-branch/gcc/fortran/trans-decl.c
    branches/gcc-4_3-branch/gcc/fortran/trans.h
    branches/gcc-4_3-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_3-branch/gcc/testsuite/gfortran.dg/alloc_comp_basics_1.f90
   
branches/gcc-4_3-branch/gcc/testsuite/gfortran.dg/alloc_comp_constructor_1.f90


-- 


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

[Bug fortran/35946] [4.3, 4.4 regression] wrong result with array constructor as argument to ATAN2

[pault at gcc dot gnu dot org]

------- Comment #9 from pault at gcc dot gnu dot org  2008-04-19 22:32 -------
Fixed on trunk and 4.3.

Thanks for the report.

Paul


-- 

pault at gcc dot gnu dot org changed:

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


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