User account creation filtered due to spam.

Bug 55207 - [F08] Variables declared in the main program should implicitly get the SAVE attribute
Summary: [F08] Variables declared in the main program should implicitly get the SAVE a...
Status: REOPENED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.8.0
: P3 normal
Target Milestone: ---
Assignee: janus
URL:
Keywords: wrong-code
: 55887 (view as bug list)
Depends on: 78392
Blocks: 37336 78122
  Show dependency treegraph
 
Reported: 2012-11-04 18:26 UTC by janus
Modified: 2016-11-17 11:57 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2013-07-31 00:00:00


Attachments
patch (1.64 KB, patch)
2012-11-05 17:45 UTC, janus
Details | Diff
new patch (1.55 KB, patch)
2013-08-01 15:02 UTC, janus
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description janus 2012-11-04 18:26:50 UTC
Although F95/F03/F08 only require automatic deallocation of local unsaved variables upon termination of a procedure, gfortran currently also does it upon termination of the main program.

Simple test case (for both scalars and arrays):

program main
  integer, allocatable :: i
  integer, allocatable, dimension(:) :: j
  allocate(i)
  allocate(j(1:5))
end program


The dump shows a block which automatically deallocates the variables at the end of the main program:

  finally
    {
      if (j.data != 0B)
        {
          __builtin_free ((void *) j.data);
        }
      j.data = 0B;
      if (i != 0B)
        {
          __builtin_free ((void *) i);
        }
    }


From F08:6.7.3.2:

"When the execution of a procedure is terminated by execution of a RETURN or END statement, an unsaved allocatable local variable of the procedure retains its allocation and definition status if it is a function result variable or a subobject thereof; otherwise, it is deallocated."

This mentions only *procedures*, not the main program. Moreover, variables declared in the main program are implicitly SAVEd in F08, cf. chapter 5.3.16:

"A variable, common block, or procedure pointer declared in the scoping unit of a main program, module, or submodule implicitly has the SAVE attribute, which may be confirmed by explicit specification. If a common block has the SAVE attribute in any other kind of scoping unit, it shall have the SAVE attribute in every scoping unit that is not of a main program, module, or submodule."

Currently we miss to flag this via attr.save = SAVE_IMPLICIT.

In F95 it is basically impossible to detect whether a compiler does end-of-program auto-dealloc. However, in F03 it can be detected by using a FINAL procedure, since deallocation (automatic or explicit) also implies finalization (for finalizable variables).
Comment 1 janus 2012-11-04 18:32:29 UTC
Patch:

Index: gcc/fortran/trans-decl.c
===================================================================
--- gcc/fortran/trans-decl.c	(revision 193135)
+++ gcc/fortran/trans-decl.c	(working copy)
@@ -3771,9 +3771,10 @@ gfc_trans_deferred_vars (gfc_symbol * proc_sym, gf
 	      else
 		gfc_restore_backend_locus (&loc);
 
-	      /* Deallocate when leaving the scope. Nullifying is not
-		 needed.  */
-	      if (!sym->attr.result && !sym->attr.dummy)
+	      /* Automatic deallocation when leaving the scope.
+		 Nullifying is not needed.  */
+	      if (!proc_sym->attr.is_main_program
+		  && !sym->attr.result && !sym->attr.dummy)
 		{
 		  if (sym->ts.type == BT_CLASS
 		      && CLASS_DATA (sym)->attr.codimension)



This regresses on:

FAIL: gfortran.dg/allocatable_scalar_9.f90  -O0   scan-tree-dump-times original "__builtin_free" 32
FAIL: gfortran.dg/coarray_lib_alloc_2.f90  -O   scan-tree-dump-times original "_gfortran_caf_deregister .&yy._data.token, 0B, 0B, 0.;" 1
FAIL: gfortran.dg/move_alloc_4.f90  -O0   scan-tree-dump-times original "__builtin_free" 9
Comment 2 janus 2012-11-04 22:26:44 UTC
(In reply to comment #1)
> Patch:

Note: The patch in comment 1 only fixes the auto-deallocation for scalars.
Comment 3 janus 2012-11-04 22:48:37 UTC
The following patch applies the implicit SAVE attribute to variables declared in the main program:

Index: gcc/fortran/decl.c
===================================================================
--- gcc/fortran/decl.c	(revision 193137)
+++ gcc/fortran/decl.c	(working copy)
@@ -3812,9 +3812,11 @@ match_attr_spec (void)
 	}
     }
 
-  /* Since Fortran 2008 module variables implicitly have the SAVE attribute.  */
-  if (gfc_current_state () == COMP_MODULE && !current_attr.save
-      && (gfc_option.allow_std & GFC_STD_F2008) != 0)
+  /* Since Fortran 2008, variables declared in a MODULE or PROGRAM
+     implicitly have the SAVE attribute.  */
+  if ((gfc_current_state () == COMP_MODULE
+       || gfc_current_state () == COMP_PROGRAM)
+      && !current_attr.save && (gfc_option.allow_std & GFC_STD_F2008) != 0)
     current_attr.save = SAVE_IMPLICIT;
 
   colon_seen = 1;


This also removes the automatic allocation of both variables in the test case in comment 0. Therefore it has the same testsuite failures as the patch in comment 1 (possibly more?).
Comment 4 janus 2012-11-05 09:07:23 UTC
(In reply to comment #3)
> Therefore it has the same testsuite failures as the patch in
> comment 1 (possibly more?).

Indeed it has a few more ...

FAIL: gfortran.dg/alloc_comp_basics_1.f90  -O0   scan-tree-dump-times original "builtin_free" 18
FAIL: gfortran.dg/alloc_comp_constructor_1.f90  -O0   scan-tree-dump-times original "builtin_free" 19
FAIL: gfortran.dg/allocatable_scalar_9.f90  -O0   scan-tree-dump-times original "__builtin_free" 32
FAIL: gfortran.dg/auto_dealloc_2.f90  -O   scan-tree-dump-times original "__builtin_free" 3
FAIL: gfortran.dg/coarray_lib_alloc_2.f90  -O   scan-tree-dump-times original "_gfortran_caf_deregister .&yy._data.token, 0B, 0B, 0.;" 1
FAIL: gfortran.dg/intent_optimize_1.f90  -O   scan-tree-dump-times optimized "does_not_exist" 0
FAIL: gfortran.dg/storage_size_3.f08  -O0  execution test
FAIL: gfortran.dg/extends_14.f03  -O   scan-tree-dump-times original "__builtin_free" 3
FAIL: gfortran.dg/volatile4.f90  -O   scan-tree-dump-not optimized "NonVolatileNotOptimizedAway"
FAIL: gfortran.dg/volatile6.f90  -O   scan-tree-dump-not optimized "NonVolatileNotOptimizedAway1"
FAIL: gfortran.dg/move_alloc_4.f90  -O0   scan-tree-dump-times original "__builtin_free" 9
FAIL: gfortran.dg/typebound_proc_27.f03  -O0   scan-tree-dump-times original "__builtin_free" 7


Most of them seem to be scan-tree-dump failures, except for:

FAIL: gfortran.dg/storage_size_3.f08  -O0  execution test

which apparently segfaults at runtime (invalid memory reference).
Comment 5 janus 2012-11-05 09:37:20 UTC
(In reply to comment #4)
> Most of them seem to be scan-tree-dump failures, except for:
> 
> FAIL: gfortran.dg/storage_size_3.f08  -O0  execution test
> 
> which apparently segfaults at runtime (invalid memory reference).


This is due to missing vptr initialization of the variable 'y':

class(t), allocatable :: y
Comment 6 janus 2012-11-05 17:45:23 UTC
Created attachment 28620 [details]
patch

Here is an extended patch, based on comment 3, which fixes the storage_size_3 wrong-code problem (cf. gfc_trans_deferred_vars) and corrects the __builtin_free counts in the test cases.

It still fails on the following:

FAIL: gfortran.dg/c_ptr_tests_16.f90  -O   scan-tree-dump-times optimized "i_do_not_exist" 0
FAIL: gfortran.dg/coarray_lib_alloc_2.f90  -O   scan-tree-dump-times original "_gfortran_caf_deregister .&xx._data.token, 0B, 0B, 0.;" 1
FAIL: gfortran.dg/intent_optimize_1.f90  -O   scan-tree-dump-times optimized "does_not_exist" 0
FAIL: gfortran.dg/volatile4.f90  -O   scan-tree-dump-not optimized "NonVolatileNotOptimizedAway"
FAIL: gfortran.dg/volatile6.f90  -O   scan-tree-dump-not optimized "NonVolatileNotOptimizedAway1"

Except for one coarray issue, those seem to be problems with optimizing away stuff in certain situations (I haven't checked any details yet).
Comment 7 janus 2013-07-22 10:59:41 UTC
I think this PR has been fixed by r199643.
Comment 8 janus 2013-07-31 20:43:08 UTC
(In reply to janus from comment #7)
> I think this PR has been fixed by r199643.

Reopening. Auto-dealloc has indeed been fixed by the above commit. However, variables in the main program are still not SAVE_IMPLICIT, which makes problems e.g. in PR 57306.

I think we need the patch in comment 6 after all. But how do we get rid of the remaining regressions?
Comment 9 janus 2013-08-01 14:58:24 UTC
(In reply to janus from comment #8)
> I think we need the patch in comment 6 after all. But how do we get rid of
> the remaining regressions?

Simplest solution: Move the code in these test cases from the main program into a subroutine (or similar). Then the variables will not be SAVEd and all optimizations can be applied as before. (However, it's a bit unfortunate that we lose the possibility to do these optimizations in the main program.)
Comment 10 janus 2013-08-01 15:02:14 UTC
Created attachment 30584 [details]
new patch

Here is a new version of the patch which regtests cleanly.

The trans-decl.c parts are not strictly necessary. They just do some clean-up.
Comment 11 Tobias Burnus 2013-08-01 21:26:43 UTC
(In reply to janus from comment #10)
> Created attachment 30584 [details]
> new patch

+  if ((gfc_current_state () == COMP_MODULE
+       || gfc_current_state () == COMP_PROGRAM)

I haven't tried the patch, but does it work correctly with BLOCK? (It might well be valid.) For instance, "i" in the following code shouldn't acquire the SAVE attribute:

  program main
    block
      integer :: i
    end block
  end program main
Comment 12 janus 2013-08-01 21:40:36 UTC
(In reply to Tobias Burnus from comment #11)
> 
> +  if ((gfc_current_state () == COMP_MODULE
> +       || gfc_current_state () == COMP_PROGRAM)
> 
> I haven't tried the patch, but does it work correctly with BLOCK? (It might
> well be valid.)

Without actually trying it: I would say it should work, since a BLOCK construct would imply COMP_BLOCK here, so that the save attribute is not set ...
Comment 13 janus 2013-08-06 08:29:10 UTC
Test case from PR 57306 comment 7 (see also http://gcc.gnu.org/ml/fortran/2013-07/msg00103.html):


  type :: c
  end type c

  type(c), target :: x
  class(c), pointer :: px => x

  if (.not. associated(px)) call abort()
end
Comment 14 janus 2014-03-15 10:53:37 UTC
Author: janus
Date: Sat Mar 15 10:53:04 2014
New Revision: 208590

URL: http://gcc.gnu.org/viewcvs?rev=208590&root=gcc&view=rev
Log:
2014-03-15  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/55207
	* decl.c (match_attr_spec): Variables in the main program implicitly
	get the SAVE attribute in Fortran 2008.


2014-03-15  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/55207
	* gfortran.dg/assumed_rank_7.f90: Explicitly deallocate variables.
	* gfortran.dg/c_ptr_tests_16.f90: Put into subroutine.
	* gfortran.dg/inline_sum_bounds_check_1.f90: Add
	-Wno-aggressive-loop-optimizations and remove an unused variable.
	* gfortran.dg/intent_optimize_1.f90: Put into subroutine.
	* gfortran.dg/pointer_init_9.f90: New.
	* gfortran.dg/volatile4.f90: Put into subroutine.
	* gfortran.dg/volatile6.f90: Ditto.

Added:
    trunk/gcc/testsuite/gfortran.dg/pointer_init_9.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/decl.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/assumed_rank_7.f90
    trunk/gcc/testsuite/gfortran.dg/c_ptr_tests_16.f90
    trunk/gcc/testsuite/gfortran.dg/inline_sum_bounds_check_1.f90
    trunk/gcc/testsuite/gfortran.dg/intent_optimize_1.f90
    trunk/gcc/testsuite/gfortran.dg/volatile4.f90
    trunk/gcc/testsuite/gfortran.dg/volatile6.f90
Comment 15 Dominique d'Humieres 2014-03-15 11:12:13 UTC
*** Bug 55887 has been marked as a duplicate of this bug. ***
Comment 16 janus 2014-03-15 12:14:20 UTC
Fixed with r208590. Closing.
Comment 17 janus 2014-03-18 22:15:41 UTC
Author: janus
Date: Tue Mar 18 22:15:10 2014
New Revision: 208668

URL: http://gcc.gnu.org/viewcvs?rev=208668&root=gcc&view=rev
Log:
2014-03-18  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/55207
	PR fortran/60549
	* decl.c (match_attr_spec): Revert r208590.

2014-03-18  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/55207
	PR fortran/60549
	* gfortran.dg/assumed_rank_7.f90: Revert r208590.
	* gfortran.dg/c_ptr_tests_16.f90: Ditto.
	* gfortran.dg/inline_sum_bounds_check_1.f90: Ditto.
	* gfortran.dg/intent_optimize_1.f90: Ditto.
	* gfortran.dg/pointer_init_9.f90: Ditto.
	* gfortran.dg/volatile4.f90: Ditto.
	* gfortran.dg/volatile6.f90: Ditto.

Removed:
    trunk/gcc/testsuite/gfortran.dg/pointer_init_9.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/decl.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/assumed_rank_7.f90
    trunk/gcc/testsuite/gfortran.dg/c_ptr_tests_16.f90
    trunk/gcc/testsuite/gfortran.dg/inline_sum_bounds_check_1.f90
    trunk/gcc/testsuite/gfortran.dg/intent_optimize_1.f90
    trunk/gcc/testsuite/gfortran.dg/volatile4.f90
    trunk/gcc/testsuite/gfortran.dg/volatile6.f90
Comment 18 janus 2014-03-18 22:23:12 UTC
Reopening. The patch has been reverted due to the problems mentioned in

http://gcc.gnu.org/ml/fortran/2014-03/msg00108.html
Comment 19 Janne Blomqvist 2015-10-21 10:31:56 UTC
As was mentioned recently on comp.lang.fortran, this affects OpenMP. Consider

program teste_paralelo
  use omp_lib
  real :: v1(12,60,60,60,2)
  !$omp parallel do
  do kk=2,60
     v1(5,2,2,kk,1)=0
  enddo
  !$omp end parallel do
  !$omp parallel
  print *, 'Hello World!'
  !$omp end parallel
end program teste_paralelo

Compiling without OpenMP and checking the binary with size, the array v1 is allocated statically in the bss section:

$ gfortran -g teste_paralelo.f90 -o test-serial
$ size test-serial 
   text	   data	    bss	    dec	    hex	filename
   2147	    608	20736032	20738787	13c72e3	test-serial

Enabling openmp, the array suddenly goes on the stack:

$ gfortran -g -fopenmp teste_paralelo.f90 -o test
$ size test
   text	   data	    bss	    dec	    hex	filename
   2822	    656	      8	   3486	    d9e	test


If one manually makes v1 saved, it naturally goes into the bss with openmp too:

$ gfortran -g -fopenmp teste_paralelo.f90 -o test-save 
$ size test-save 
   text	   data	    bss	    dec	    hex	filename
   2790	    656	20736032	20739478	13c7596	test-save