User account creation filtered due to spam.

Bug 34540 - cshift, eoshift, kind=1 and kind=2 arguments...
Summary: cshift, eoshift, kind=1 and kind=2 arguments...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libfortran (show other bugs)
Version: 4.3.0
: P3 normal
Target Milestone: ---
Assignee: Jerry DeLisle
URL:
Keywords: wrong-code
Depends on: 33317
Blocks: 32834
  Show dependency treegraph
 
Reported: 2007-12-20 20:40 UTC by Thomas Koenig
Modified: 2008-01-06 18:40 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2008-01-05 21:00:04


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Koenig 2007-12-20 20:40:16 UTC
I really thought we had these figured out...

$ cat shift-variations.f90
program main
  integer(kind=1) :: d1
  integer(kind=2) :: d2
  integer(kind=4) :: d4
  integer(kind=8) :: d8
  integer(kind=1), dimension(2) :: s1
  integer(kind=2), dimension(2) :: s2
  integer(kind=4), dimension(2) :: s4
  integer(kind=8), dimension(2) :: s8
  real, dimension(2,2) :: r
  data r /1.0, 2.0, 3.0, 4.0/
  s1 = (/1, 1/)
  s2 = (/1, 1/)
  s4 = (/1, 1/)
  s8 = (/1, 1/)
  d1 = 1
  d2 = 1
  d4 = 1
  d8 = 1
  print *,eoshift(r,shift=s1,dim=d1)
  print *,eoshift(r,shift=s2,dim=d2)
  print *,eoshift(r,shift=s4,dim=d4)
  print *,eoshift(r,shift=s8,dim=d8)
  print *,cshift(r,shift=s1,dim=d1)
  print *,cshift(r,shift=s2,dim=d2)
  print *,cshift(r,shift=s4,dim=d4)
  print *,cshift(r,shift=s8,dim=d8)
end program main
$ gfortran shift-variations.f90
$ ./a.out
   0.0000000      3.98764005E-34  1.26116862E-44   0.0000000    
 -2.73992191E-05 -2.85282731E-05   0.0000000      4.08616923E-38
   2.0000000       0.0000000       4.0000000       0.0000000    
   2.0000000       0.0000000       4.0000000       0.0000000    
Fortran runtime error: Argument 'DIM' is out of range in call to 'CSHIFT'
$ gfortran -v
Using built-in specs.
Target: i686-pc-linux-gnu
Configured with: ../../gcc/trunk/configure --prefix=/home/ig25 --enable-languages=c,fortran
Thread model: posix
gcc version 4.3.0 20071216 (experimental) (GCC)
Comment 1 Jerry DeLisle 2007-12-21 03:13:20 UTC
This is a regression.  The test case is OK with 4.2
Comment 2 Thomas Koenig 2007-12-21 19:49:44 UTC
The problem is with dim.
Comment 3 Thomas Koenig 2007-12-21 20:56:49 UTC
The problem is with the lines

      if (dim->expr_type != EXPR_CONSTANT)
        {
          /* Mark this for later setting the type in gfc_conv_missing_dummy.  */
          dim->representation.length = shift->ts.kind;
        }
      else
        {
          gfc_resolve_dim_arg (dim);
          /* Convert dim to shift's kind to reduce variations.  */
          if (dim->ts.kind != shift->ts.kind)
            gfc_convert_type_warn (dim, &shift->ts, 2, 0);
        }

For the test case, we take the first branch, which means we never
adjust dim's kind.

Jerry, you did some work in this area.  I'mm CC: ing you on
this in case this rings a bell.
Comment 4 Jerry DeLisle 2007-12-21 21:07:37 UTC
Yes, this is my doing in fixing PR33317.  I will look at it tonight.
Comment 5 Jerry DeLisle 2007-12-21 22:18:05 UTC
Patch is testing.
Comment 6 Jerry DeLisle 2007-12-22 01:57:22 UTC
Subject: Bug 34540

Author: jvdelisle
Date: Sat Dec 22 01:57:07 2007
New Revision: 131133

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=131133
Log:
2007-12-21  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR fortran/34540
	* iresolve.c (gfc_resolve_cshift): Take optional dim path
	only if the argument is an optional itself.
	* iresolve.c (gfc_resolve_eoshift): Same.

Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/iresolve.c

Comment 7 Jerry DeLisle 2007-12-22 02:00:18 UTC
Subject: Bug 34540

Author: jvdelisle
Date: Sat Dec 22 01:59:56 2007
New Revision: 131134

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=131134
Log:
2007-12-21  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR fortran/34540
	* gfortran.dg/shift-kind_2.f90: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/shift-kind_2.f90
Modified:
    trunk/gcc/testsuite/ChangeLog

Comment 8 Jerry DeLisle 2007-12-22 02:00:55 UTC
Fixed on trunk.
Comment 9 Thomas Koenig 2007-12-22 09:32:18 UTC
Hi Jerry,

thanks for the quick fix.

I don't have time for testing right now, but maybe you
can also check that

module tst_foo
  implicit none
contains
  subroutine tst_optional(a,n1,n2)
    real, dimension(:,:) :: a
    integer(kind=1), intent(in), optional:: n1
    integer(kind=2), intent(in), optional:: n2
    integer(kind=1), dimension(2) :: s1
    s1 = (/1, 1/)
    print *,cshift(a,shift=s1,dim=n1)
    print *,cshift(a,shift=s1,dim=n2)
    print *,eoshift(a,shift=s1,dim=n1)
    print *,eoshift(a,shift=s1,dim=n2)
  end subroutine tst_optional
end module tst_foo

program main
  use tst_foo
  implicit none
  real, dimension(2,2) :: r
  integer(kind=1) :: d1
  integer(kind=2) :: d2
  data r /1.0, 2.0, 3.0, 4.0/
  d1 = 1_1
  d2 = 1_2
  call tst_optional(r, d1,d2)
end program main

works?
Comment 10 Dominique d'Humieres 2007-12-22 10:57:45 UTC
The test case in comment #9 fails with:

Fortran runtime error: Argument 'DIM' is out of range in call to 'CSHIFT'

gfortran: gcc version 4.3.0 20071222, revision 131134 + patches for PR34421,  PR34514, PR34533, and PR34536 (they all did what they were supposed to do without regression at revision 131125).


Comment 11 Jerry DeLisle 2007-12-22 15:00:35 UTC
I get a segfault.
Comment 12 Jerry DeLisle 2007-12-22 16:04:13 UTC
This latest issue is not a regression so I changed the summary.
Comment 13 Jerry DeLisle 2007-12-22 16:28:54 UTC
This is surprising!  If I get rid of the hack that was needed before for optional_dim_2.f90 :

Index: iresolve.c
===================================================================
--- iresolve.c  (revision 131133)
+++ iresolve.c  (working copy)
@@ -590,12 +590,6 @@ gfc_resolve_cshift (gfc_expr *f, gfc_exp
  
   if (dim != NULL)
     {
-      if (dim->expr_type != EXPR_CONSTANT && dim->symtree->n.sym->attr.optional)
-       {
-         /* Mark this for later setting the type in gfc_conv_missing_dummy.  */
-         dim->representation.length = shift->ts.kind;
-       }
-      else
        {
          gfc_resolve_dim_arg (dim);
          /* Convert dim to shift's kind to reduce variations.  */

The new case passes (for cshift and I would presume eoshift) and the optional_dim_2.f90 passes and I see no apparent regressions.  Still testing.

This means something got fixed somewhere else. I also checked this on ppc64-linux with the same result.
Comment 14 Jerry DeLisle 2007-12-22 16:39:11 UTC
No, I checked the original case for pr33317 and it segfaults on compilation with the change in comment #13.  This means the testcase optional_dim_2.f90 is not sufficient.  I will fix that along the way here.
Comment 15 Jerry DeLisle 2007-12-22 19:22:08 UTC
Thomas, hope you don't mind me taking this over.  This last bug is a different problem all together.  I am hoping we don't have to redesign the implementation.
Comment 16 Jerry DeLisle 2007-12-23 16:43:43 UTC
The test case in comment #9 passes if I revert the code path required by the original test case in pr33317.  Using gdb, I see no difference in the incoming dim expression for either case.  This implies we need to approach this problem differently.

The root of the problem is saving on the number of variations of the runtime cshift and eoshift functions, gfortran converts the dim expression to a convert_* function with dim as an argument.  The dim argument is optional and when it is not present a null is provided.  The convert functions do not handle a null and gfortran ends up segfaulting on memory access at run time.

Attempting to resolve the dim argument also results in a convert function rather than a NULL constant.

I am unassigning myself for a while.
Comment 17 Jerry DeLisle 2008-01-05 21:00:04 UTC
I think I have a patch for this. Testing
Comment 18 Tobias Burnus 2008-01-06 09:40:07 UTC
Posted patch for this PR and PR 34387.
http://gcc.gnu.org/ml/gcc-patches/2008-01/msg00192.html
Comment 19 Jerry DeLisle 2008-01-06 18:35:39 UTC
Subject: Bug 34540

Author: jvdelisle
Date: Sun Jan  6 18:34:14 2008
New Revision: 131357

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=131357
Log:
2008-01-06  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR libfortran/34540
	* gfortran.dg/optional_dim_3.f90: New test.

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

Comment 20 Jerry DeLisle 2008-01-06 18:40:51 UTC
Fixed with patch to 34387.