public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Re: [PATCH, Fortran] -fno-automatic with -finit-local prevents initialization of automatics in recursive functions
       [not found] <CAE4aFAkSNjGYmpxUZtM8wSDYKvHaBxkpq2hX6sq8aqWfKndT+w@mail.gmail.com>
@ 2014-08-29 20:13 ` Fritz Reese
  2014-08-31 22:28   ` Tobias Burnus
  0 siblings, 1 reply; 3+ messages in thread
From: Fritz Reese @ 2014-08-29 20:13 UTC (permalink / raw)
  To: fortran, gcc-patches

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62309

It seems with gcc-4.8.3 -fno-automatic prevents initializers from
being applied to automatic variables. The following does not behave as
I would expect it to with gcc-4.8.3 on linux-x86-64:

      function f (x)
        implicit none
        integer f, x
        integer a ! should be SAVEd from -fno-automatic
        a = a + x ! should increment by y every time
        f = a
        return
      endfunction

      recursive function g (x)
        implicit none
        integer g, x
        integer b ! should be automatic from recursive
        b = b + x ! should be set to y every time
        g = b
        return
      endfunction

      implicit none
      integer f, g

      ! Should return static value of a; accumulates x
      print *, f(3) ! -> 3, ok
      print *, f(4) ! -> 7, ok
      print *, f(2) ! -> 2, ok

      ! Should return automatic value of c; equal to y each time
      print *, g(3) ! -> garbage, expected 3
      print *, g(4) ! -> garbage, expected 4
      print *, g(2) ! -> garbage, expected 2

      end

$ gfortran -fno-automatic -finit-local-zero auto_test.f
$ ./a.out
           3
           7
           9
       32770
       32771
       32769
$

According to gfortran's manual page, -fno-automatic should "Treat each
program unit (except those marked as RECURSIVE) as if the "SAVE"
statement were specified for every local variable [...]". As far as I
can tell, -finit-local-zero should still initialize automatic
variables in RECURSIVE functions.

I believe this is a simple fix; to actually follow the specification
set forth in the man page, don't treat symbols in a RECURSIVE
namespace as if they are saved in resolve.c
(apply_default_init_local):


2014-08-29  Fritz Reese  <Reese-Fritz@zai.com>

    * resolve.c (apply_default_init_local): Don't treat variables in
    RECURSIVE units as saved.


diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 43eb240..a428633 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -11104,6 +11104,7 @@ apply_default_init_local (gfc_symbol *sym)
      result variable, which are also nonstatic.  */
   if (sym->attr.save || sym->ns->save_all
       || (gfc_option.flag_max_stack_var_size == 0 && !sym->attr.result
+          && !sym->ns->proc_name->attr.recursive
          && (!sym->attr.dimension || !is_non_constant_shape_array (sym))))
     {
       /* Don't clobber an existing initializer!  */
diff --git a/gcc/testsuite/gfortran.dg/auto_save_2.f90 b/gcc/testsuite/gfortran.
new file mode 100644
index 0000000..0d39d48
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/auto_save_2.f90
@@ -0,0 +1,52 @@
+! { dg-do run }
+! { dg-options "-fno-automatic -finit-local-zero" }
+!
+! Make sure variables are saved with -fno-automatic except in
+! functions marked RECURSIVE, and that they are still initialized with
+! -finit-local-zero.
+!
+
+function f (x)
+implicit none
+  integer f, x
+  integer a ! should be SAVEd
+  a = a + x ! should increment by y every time
+  f = a
+  return
+endfunction
+
+recursive function g (x)
+implicit none
+  integer g, x
+  integer b ! should be automatic
+  b = b + x ! should be set to y every time
+  g = b
+  return
+endfunction
+
+implicit none
+integer f, g
+
+! Should return static value of a; accumulates y
+if ( f(3) .ne. 3 ) then
+  call abort ()
+endif
+if ( f(4) .ne. 7 ) then
+  call abort ()
+endif
+if ( f(2) .ne. 9 ) then
+  call abort ()
+endif
+
+! Should return automatic value of a; equal to y each time
+if ( g(3) .ne. 3 ) then
+  call abort ()
+endif
+if ( g(4) .ne. 4 ) then
+  call abort ()
+endif
+if ( g(2) .ne. 2 ) then
+  call abort ()
+endif
+
+end


Fritz Reese

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH, Fortran] -fno-automatic with -finit-local prevents initialization of automatics in recursive functions
  2014-08-29 20:13 ` [PATCH, Fortran] -fno-automatic with -finit-local prevents initialization of automatics in recursive functions Fritz Reese
@ 2014-08-31 22:28   ` Tobias Burnus
  2014-08-31 22:43     ` Steve Kargl
  0 siblings, 1 reply; 3+ messages in thread
From: Tobias Burnus @ 2014-08-31 22:28 UTC (permalink / raw)
  To: Fritz Reese, fortran, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1574 bytes --]

Fritz Reese wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62309
>
> It seems with gcc-4.8.3 -fno-automatic prevents initializers from
> being applied to automatic variables.
[...]
> According to gfortran's manual page, -fno-automatic should "Treat each
> program unit (except those marked as RECURSIVE) as if the "SAVE"
> statement were specified for every local variable [...]". As far as I
> can tell, -finit-local-zero should still initialize automatic
> variables in RECURSIVE functions.
>
> I believe this is a simple fix; to actually follow the specification
> set forth in the man page, don't treat symbols in a RECURSIVE
> namespace as if they are saved in resolve.c
> (apply_default_init_local):

Thanks for the patch. I think it qualifies as obvious. But note for 
potential future contributions that for nontrivial contributions a 
copyright assignment with the FSF is required: 
https://gcc.gnu.org/contribute.html

(Tiny nit: 8 spaces are replaced by a tab.)

I initially thought that one had to be more careful because of BLOCK, 
but the RECURSIVE is passed on those namespaces, hence, the patch works 
fine. I have extended the test case to cover that part as well.

I have now committed the attached version of the patch to the trunk (GCC 
5) as Rev. 214771.

Tobias

(Side remark: I think it is bad style to rely on -finit-local with new 
code - and RECURSIVE is new since Fortran 90) code.)

> 2014-08-29  Fritz Reese  <Reese-Fritz@zai.com>
>
>      * resolve.c (apply_default_init_local): Don't treat variables in
>      RECURSIVE units as saved.

[-- Attachment #2: committed.diff --]
[-- Type: text/x-patch, Size: 3692 bytes --]

Index: gcc/fortran/ChangeLog
===================================================================
--- gcc/fortran/ChangeLog	(Revision 214770)
+++ gcc/fortran/ChangeLog	(Arbeitskopie)
@@ -1,3 +1,9 @@
+2014-08-31  Fritz Reese  <Reese-Fritz@zai.com>
+
+	PR fortran/62309
+	* resolve.c (apply_default_init_local): Don't treat variables
+	in RECURSIVE procedures as saved.
+
 2014-08-31  Tobias Burnus  <burnus@net-b.de>
 
 	* trans-decl.c (gfc_build_builtin_function_decls): Add
Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c	(Revision 214770)
+++ gcc/fortran/resolve.c	(Arbeitskopie)
@@ -10780,6 +10780,7 @@ apply_default_init_local (gfc_symbol *sym)
      result variable, which are also nonstatic.  */
   if (sym->attr.save || sym->ns->save_all
       || (gfc_option.flag_max_stack_var_size == 0 && !sym->attr.result
+	  && !sym->ns->proc_name->attr.recursive
 	  && (!sym->attr.dimension || !is_non_constant_shape_array (sym))))
     {
       /* Don't clobber an existing initializer!  */
Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog	(Revision 214770)
+++ gcc/testsuite/ChangeLog	(Arbeitskopie)
@@ -1,3 +1,9 @@
+2014-08-31  Fritz Reese  <Reese-Fritz@zai.com>
+	    Tobias Burnus  <burnus@net-b.de>
+
+	PR fortran/62309
+	* gcc/testsuite/gfortran.dg/auto_save_2.f90: New.
+
 2014-08-31  Tobias Burnus  <burnus@net-b.de>
 
 	* gfortran.dg/coarray_lib_comm_1.f90: New.
Index: gcc/testsuite/gfortran.dg/auto_save_2.f90
===================================================================
--- gcc/testsuite/gfortran.dg/auto_save_2.f90	(Revision 0)
+++ gcc/testsuite/gfortran.dg/auto_save_2.f90	(Arbeitskopie)
@@ -0,0 +1,84 @@
+! { dg-do run }
+! { dg-options "-fno-automatic -finit-local-zero -fdump-tree-original" }
+!
+! PR fortran/62309
+!
+! Make sure variables are saved with -fno-automatic except in
+! functions marked RECURSIVE, and that they are still initialized with
+! -finit-local-zero.
+!
+
+function f (x)
+implicit none
+  integer f, x
+  integer a ! should be SAVEd
+  a = a + x ! should increment by y every time
+  f = a
+  return
+endfunction
+
+function f2 (x)
+implicit none
+  integer f2, x
+  block
+   named: block
+    block
+    integer a ! should be SAVEd
+    a = a + x ! should increment by y every time
+    f2 = a
+    end block
+   end block named
+  end block
+  return
+endfunction
+
+recursive function g (x)
+implicit none
+  integer g, x
+  integer b ! should be automatic
+  b = b + x ! should be set to y every time
+  g = b
+  return
+endfunction
+
+recursive function g2 (x)
+implicit none
+  integer g2, x
+  block
+   named: block
+    block
+    integer b ! should be automatic
+    b = b + x ! should be set to y every time
+    g2 = b
+    end block
+   end block named
+  end block
+  return
+endfunction
+
+implicit none
+integer f, f2, g, g2
+
+! Should return static value of a; accumulates y
+if ( f(3) .ne. 3 ) call abort ()
+if ( f(4) .ne. 7 ) call abort ()
+if ( f(2) .ne. 9 ) call abort ()
+
+if ( f2(3) .ne. 3 ) call abort ()
+if ( f2(4) .ne. 7 ) call abort ()
+if ( f2(2) .ne. 9 ) call abort ()
+
+! Should return automatic value of a; equal to y each time
+if ( g(3) .ne. 3 ) call abort ()
+if ( g(4) .ne. 4 ) call abort ()
+if ( g(2) .ne. 2 ) call abort ()
+
+if ( g2(3) .ne. 3 ) call abort ()
+if ( g2(4) .ne. 4 ) call abort ()
+if ( g2(2) .ne. 2 ) call abort ()
+
+end
+
+! { dg-final { scan-tree-dump-times "  static integer\\\(kind=4\\\) a = 0;" 2 "original" } }
+! { dg-final { scan-tree-dump-times "  b = 0;" 2 "original" } }
+! { dg-final { cleanup-tree-dump "original" } }

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH, Fortran] -fno-automatic with -finit-local prevents initialization of automatics in recursive functions
  2014-08-31 22:28   ` Tobias Burnus
@ 2014-08-31 22:43     ` Steve Kargl
  0 siblings, 0 replies; 3+ messages in thread
From: Steve Kargl @ 2014-08-31 22:43 UTC (permalink / raw)
  To: Tobias Burnus; +Cc: Fritz Reese, fortran, gcc-patches

On Mon, Sep 01, 2014 at 12:28:39AM +0200, Tobias Burnus wrote:
> Fritz Reese wrote:
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62309
> >
> > It seems with gcc-4.8.3 -fno-automatic prevents initializers from
> > being applied to automatic variables.
> [...]
> > According to gfortran's manual page, -fno-automatic should "Treat each
> > program unit (except those marked as RECURSIVE) as if the "SAVE"
> > statement were specified for every local variable [...]". As far as I
> > can tell, -finit-local-zero should still initialize automatic
> > variables in RECURSIVE functions.
> >
> > I believe this is a simple fix; to actually follow the specification
> > set forth in the man page, don't treat symbols in a RECURSIVE
> > namespace as if they are saved in resolve.c
> > (apply_default_init_local):
> 

(large snip)

> +implicit none
> +  integer f, x
> +  integer a ! should be SAVEd
> +  a = a + x ! should increment by y every time

a is undefined on the RHS.  Relying a compiler option to
make the code conforming is rather dubious.  I'd rather
see an error here; even if the user uses a option because
the user is too lazy to write conforming code.

> +  f = a
> +  return
> +endfunction
> +
> +function f2 (x)
> +implicit none
> +  integer f2, x
> +  block
> +   named: block
> +    block
> +    integer a ! should be SAVEd
> +    a = a + x ! should increment by y every time
> +    f2 = a

a is undefined on the RHS and this should elicit an error.

PS: the comment is wrong as there is no y.

> +    end block
> +   end block named
> +  end block
> +  return
> +endfunction
> +
> +recursive function g (x)
> +implicit none
> +  integer g, x
> +  integer b ! should be automatic
> +  b = b + x ! should be set to y every time

b is undefined on the RHS and this should elicit an error.

PS: the comment is wrong as there is no y.

The -finit-* option were implemented for compatibility with
g77 and to allow the compilation of dusty deck code.  New
code should be written to conform to the standard.

-- 
Steve

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2014-08-31 22:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CAE4aFAkSNjGYmpxUZtM8wSDYKvHaBxkpq2hX6sq8aqWfKndT+w@mail.gmail.com>
2014-08-29 20:13 ` [PATCH, Fortran] -fno-automatic with -finit-local prevents initialization of automatics in recursive functions Fritz Reese
2014-08-31 22:28   ` Tobias Burnus
2014-08-31 22:43     ` Steve Kargl

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).