Bug 47051 - [4.6 Regression] Wrong reallocate
Summary: [4.6 Regression] Wrong reallocate
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.6.0
: P4 normal
Target Milestone: 4.6.0
Assignee: Paul Thomas
URL:
Keywords: wrong-code
Depends on:
Blocks: 32834
  Show dependency treegraph
 
Reported: 2010-12-23 13:27 UTC by Joost VandeVondele
Modified: 2011-01-11 05:20 UTC (History)
7 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2010-12-30 16:53:55


Attachments
This patch fixes the PR (2.25 KB, patch)
2011-01-09 21:02 UTC, Paul Thomas
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joost VandeVondele 2010-12-23 13:27:43 UTC
In the following a reallocation seems to happen, changing the bounds of a. However the size of a and b are the same, so I would not expect this.

 INTEGER, DIMENSION(:), ALLOCATABLE :: a,b
 ALLOCATE(a(-1:1),b(1:3))
 b=0
 a=b
 write(6,*) LBOUND(a)
 IF(LBOUND(a,1).NE.-1) STOP "BUG"
END
Comment 1 Dominique d'Humieres 2010-12-23 13:36:40 UTC
> ... so I would not expect this.

Why?
Comment 2 Joost VandeVondele 2010-12-23 13:46:50 UTC
(In reply to comment #1)
> > ... so I would not expect this.
> 
> Why?

that would imply that F95 code and F2003 code are not compatible ? Or was this not allowed in F95 (certainly that was in CP2K, and no compiler ever gave a bounds check error)
Comment 3 Joost VandeVondele 2010-12-23 13:56:29 UTC
OK, more checking. F2003 specifies that the lhs should only be deallocated if it differs in shape. a and b have the same shape here.
Comment 4 Dominique d'Humieres 2010-12-23 15:13:46 UTC
I have raised a similar question in

http://gcc.gnu.org/ml/fortran/2010-11/msg00004.html answered in the following posts in this thread.

The relevant part of the standard (from f2008 draft) is

7.2.1.3 Interpretation of intrinsic assignments

...

3 If the variable is an unallocated allocatable array, expr shall have the same rank. If the variable is an allocated allocatable variable, it is deallocated if expr is an array of different shape, any of the corresponding length type parameter values of the variable and expr differ, or the variable is polymorphic and the dynamic type of the variable and expr differ. If the variable is or becomes an unallocated allocatable variable, it is then allocated with
 if the variable is polymorphic, the same dynamic type as expr,
 each deferred type parameter equal to the corresponding type parameter of expr,
 if the variable is an array and expr is scalar, the same bounds as before, and
 if expr is an array, the shape of expr with each lower bound equal to the corresponding element of
LBOUND (expr).
NOTE 7.36
For example, given the declaration
CHARACTER(:),ALLOCATABLE :: NAME
then after the assignment statement
NAME = 'Dr. '//FIRST_NAME//' '//SURNAME
NAME will have the length LEN(FIRST NAME)+LEN(SURNAME)+5, even if it had previously been
unallocated, or allocated with a dierent length. However, for the assignment statement
NAME(:) = 'Dr. '//FIRST_NAME//' '//SURNAME
NAME must already be allocated at the time of the assignment; the assigned value is truncated or blank
padded to the previously allocated length of NAME.

For which a strict reading of " If the variable is or becomes an unallocated allocatable variable, ... the shape of expr with each lower bound equal to the corresponding element of
LBOUND (expr)." support this PR since a is not deallocated. If this reading is right it will be a very strong restriction for "reallocate on assignement":

 INTEGER, DIMENSION(:), ALLOCATABLE :: a,b,c,d
 ALLOCATE(a(-1:1),b(1:3),c(2:3),d(3:6))
 b=0
 c=0
 d=0
 a=b
 write(6,*) LBOUND(a), size(a)
 a=c
 write(6,*) LBOUND(a), size(a)
 a=d
 write(6,*) LBOUND(a), size(a)
END

will give

          -1           3
           2           2
           3           4

Note that there are two work-around:
(1) compile with -fno-realloc-lhs,
(2) use a(:).
Comment 5 Joost VandeVondele 2010-12-23 15:32:14 UTC
(In reply to comment #4)
Hi Dominique,

I have read exactly this:
 
> 3 If the variable is an unallocated allocatable array, expr shall have the same
> rank. If the variable is an allocated allocatable variable, it is deallocated
> if expr is an array of different shape, any of the corresponding length type
> parameter values of the variable and expr differ, or the variable is
> polymorphic and the dynamic type of the variable and expr differ. 

this section explicitly says it is deallocated only if the shape is different. In my example, the shape is the same (even if the bounds are different). 

There is thus no need to deallocate a, and reallocate afterwards with the bounds of b.

Joost
Comment 6 Paul Thomas 2010-12-23 17:51:00 UTC
(In reply to comment #4)

>  INTEGER, DIMENSION(:), ALLOCATABLE :: a,b,c,d
>  ALLOCATE(a(-1:1),b(1:3),c(2:3),d(3:6))
>  b=0
>  c=0
>  d=0
>  a=b
>  write(6,*) LBOUND(a), size(a)
>  a=c
>  write(6,*) LBOUND(a), size(a)
>  a=d
>  write(6,*) LBOUND(a), size(a)
> END
> 
> will give
> 
>           -1           3
>            2           2
>            3           4
> 
> Note that there are two work-around:
> (1) compile with -fno-realloc-lhs,
> (2) use a(:).

extending this example slightly:

 INTEGER, DIMENSION(:), ALLOCATABLE :: a,b,c,d
 ALLOCATE(a(-1:1),b(1:3),c(2:3),d(3:6))
 b=0
 c=0
 d=0
 write(6,*) LBOUND(a), size(a), loc(a(lbound(a)))
 a=b
 write(6,*) LBOUND(a), size(a), loc(a(lbound(a)))
 a=c
 write(6,*) LBOUND(a), size(a), loc(a(lbound(a)))
 a=d
 write(6,*) LBOUND(a), size(a), loc(a(lbound(a)))
END

gives

[prt@localhost pr47051]# ifort --version
ifort (IFORT) 11.1 20090630
Copyright (C) 1985-2009 Intel Corporation.  All rights reserved.

[prt@localhost pr47051]# ifort -assume realloc-lhs pr47051a.f90
[prt@localhost pr47051]# ./a.out
          -1           3              17346660
           1           3              17346652
           2           2              17346648
           3           4              17346644

and for gfortran:
          -1           3      140736566840176
           1           3      140736566840128
           2           2      140736566840080
           3           4      140736566840032

Thus the bounds are the same and it was this comparison that made me stop where I did.  Worryingly, though, the array is being reallocated at each assignment.  If you examine the code, the address should be the same for the first two lines, since the array sizes are the same.

In summary, the reallocation should NOT occur and the setting of the bounds should be sorted out.

Cheers

Paul
Comment 7 Jerry DeLisle 2010-12-30 03:19:55 UTC
Whys is this a regression? When did it work last?
Comment 8 Mikael Morin 2010-12-30 14:43:46 UTC
(In reply to comment #7)
> When did it work last?

When automatic/implicit reallocation on assignment was not supported. ;-)
Comment 9 Thomas Koenig 2010-12-30 16:53:55 UTC
Not a regression, adjusting subject, confirming.
Comment 10 Joost VandeVondele 2011-01-02 11:54:52 UTC
The testcase in comment #0 is just Fortran95 (i.e. is well defined without reallocation), and works fine with all versions of gfortran prior to 4.6. It is thus a regression, and a quite serious one IMO since it can affect any code using array assignment. It was found with CP2K, if that matters.
Comment 11 Thomas Koenig 2011-01-02 22:17:32 UTC
Joost, you're right.
Comment 12 Paul Thomas 2011-01-04 19:24:18 UTC
Obviously, I should take this one.  As soon as I have done with PR46896, I will turn to this one.

Paul
Comment 13 Paul Thomas 2011-01-09 21:02:50 UTC
Created attachment 22936 [details]
This patch fixes the PR

I need to go through this with a fine toothed comb.  It regtests OK and represents a big simplification of the code.

I'll submit tomorrow.

Paul
Comment 14 Paul Thomas 2011-01-10 07:31:42 UTC
(In reply to comment #13)

> I'll submit tomorrow.

Done http://gcc.gnu.org/ml/fortran/2011-01/msg00075.html

Paul
Comment 15 Paul Thomas 2011-01-11 05:19:24 UTC
Author: pault
Date: Tue Jan 11 05:19:20 2011
New Revision: 168650

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=168650
Log:
2011-01-11  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/47051
	* trans-array.c (gfc_alloc_allocatable_for_assignment): Change
	to be standard compliant by testing for shape rather than size
	before skipping reallocation. Improve comments.

2011-01-11  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/47051
	* gfortran.dg/realloc_on_assign_2.f03 : Modify 'test1' to be
	standard compliant and comment.

Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-array.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/realloc_on_assign_2.f03
Comment 16 Paul Thomas 2011-01-11 05:20:48 UTC
Fixed.  Thanks for the report Joost!

Paul