diff mbox

[Fortran] PRs 52751/40973 - don't set TREE_PUBLIC for PRIVATE module procs/vars

Message ID 4F7B6924.7010003@net-b.de
State New
Headers show

Commit Message

Tobias Burnus April 3, 2012, 9:18 p.m. UTC 
Dear all,

the attached patch only sets TREE_PUBLIC for module variables and module 
procedures which have neither the PRIVATE attribute nor a C-binding 
label. Seemingly, only NAG f95 does this for module variables and none 
of my compilers does this for module procedures.

The main effect of this patch is a code size reduction as the compiler 
might (even without -fwhole-program -flto) optimize unused 
variables/procedures away. Additionally, the compiler might inline code 
which it otherwise wouldn't do (due to the code size increase) or do 
optimizations based on the value of the module variables (though, GCC 
has room for improvement for optimizing static variables.)

Note: For C-binding variables without binding label ("bind(C, 
name='')"), I don't use DECL_COMMON. That should be okay as DECL_COMMON 
is only used to make sure that a variable can be initialized from either 
C or Fortran. But without binding name, that's not possible from C, 
hence, that's fine.

Build and regtested on x86-64-linux. (And currently regtesting again - 
after a minor modification.)
OK for the trunk?

Tobias

Comments

Paul Richard Thomas April 4, 2012, 9:31 a.m. UTC | #1 
Dear Tobias,

This is OK for trunk - thanks for the patch.

Cheers

Paul

On Tue, Apr 3, 2012 at 11:18 PM, Tobias Burnus <burnus@net-b.de> wrote:
> Dear all,
>
> the attached patch only sets TREE_PUBLIC for module variables and module
> procedures which have neither the PRIVATE attribute nor a C-binding label.
> Seemingly, only NAG f95 does this for module variables and none of my
> compilers does this for module procedures.
>
> The main effect of this patch is a code size reduction as the compiler might
> (even without -fwhole-program -flto) optimize unused variables/procedures
> away. Additionally, the compiler might inline code which it otherwise
> wouldn't do (due to the code size increase) or do optimizations based on the
> value of the module variables (though, GCC has room for improvement for
> optimizing static variables.)
>
> Note: For C-binding variables without binding label ("bind(C, name='')"), I
> don't use DECL_COMMON. That should be okay as DECL_COMMON is only used to
> make sure that a variable can be initialized from either C or Fortran. But
> without binding name, that's not possible from C, hence, that's fine.
>
> Build and regtested on x86-64-linux. (And currently regtesting again - after
> a minor modification.)
> OK for the trunk?
>
> Tobias
>
diff mbox

Patch

2012-04-03  Tobias Burnus  <burnus@net-b.de>

	PR fortran/52751
	* trans-decl.c (gfc_finish_var_decl): Don't set TREE_PUBLIC
	for PRIVATE module variables without C-binding label.

	PR fortran/40973
	* trans-decl.c (build_function_decl): Ditto for procedures.

2012-04-03  Tobias Burnus  <burnus@net-b.de>

	PR fortran/40973
	PR fortran/52751
	* gfortran.dg/public_private_module_2.f90: New.

diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index 8a1dd2e..23cf052 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -539,7 +539,7 @@  gfc_finish_var_decl (tree decl, gfc_symbol * sym)
   if (sym->attr.cray_pointee)
     return;
 
-  if(sym->attr.is_bind_c == 1)
+  if(sym->attr.is_bind_c == 1 && sym->binding_label)
     {
       /* We need to put variables that are bind(c) into the common
 	 segment of the object file, because this is what C would do.
@@ -565,7 +565,8 @@  gfc_finish_var_decl (tree decl, gfc_symbol * sym)
       /* TODO: Don't set sym->module for result or dummy variables.  */
       gcc_assert (current_function_decl == NULL_TREE || sym->result == sym);
       /* This is the declaration of a module variable.  */
-      TREE_PUBLIC (decl) = 1;
+      if (sym->attr.access != ACCESS_PRIVATE)
+	TREE_PUBLIC (decl) = 1;
       TREE_STATIC (decl) = 1;
     }
 
@@ -1842,7 +1857,8 @@  build_function_decl (gfc_symbol * sym, bool global)
   DECL_EXTERNAL (fndecl) = 0;
 
   if (!current_function_decl
-      && !sym->attr.entry_master && !sym->attr.is_main_program)
+      && !sym->attr.entry_master && !sym->attr.is_main_program
+      && (sym->attr.access != ACCESS_PRIVATE || sym->binding_label))
     TREE_PUBLIC (fndecl) = 1;
 
   attributes = add_attributes_to_decl (attr, NULL_TREE);

--- /dev/null	2012-03-22 21:06:43.387787737 +0100
+++ gcc/gcc/testsuite/gfortran.dg/public_private_module_2.f90	2012-04-03 23:02:59.000000000 +0200
@@ -0,0 +1,70 @@ 
+! { dg-do compile }
+! { dg-options "-O2" }
+!
+! PR fortran/52751 (top, "module mod")
+! PR fortran/40973 (bottom, "module m"
+!
+! Ensure that (only) those module variables and procedures which are PRIVATE
+! and have no C-binding label are optimized away.
+!
+      module mod
+        integer :: aa
+        integer, private :: iii
+        integer, private, bind(C) :: jj             ! { dg-warning "PRIVATE but has been given the binding label" }
+        integer, private, bind(C,name='lll') :: kk  ! { dg-warning "PRIVATE but has been given the binding label" }
+        integer, private, bind(C,name='') :: mmmm
+        integer, bind(C) :: nnn
+        integer, bind(C,name='oo') :: pp
+        integer, bind(C,name='') :: qq
+      end module mod
+
+      ! { dg-final { scan-assembler "__mod_MOD_aa" } }
+      ! { dg-final { scan-assembler-not "iii" } }
+      ! { dg-final { scan-assembler "jj" } }
+      ! { dg-final { scan-assembler "lll" } }
+      ! { dg-final { scan-assembler-not "kk" } }
+      ! { dg-final { scan-assembler-not "mmmm" } }
+      ! { dg-final { scan-assembler "nnn" } }
+      ! { dg-final { scan-assembler "oo" } }
+      ! { dg-final { scan-assembler "__mod_MOD_qq" } }
+
+MODULE M
+  PRIVATE :: two, three, four, six
+  PUBLIC :: one, seven, eight, ten
+CONTAINS
+  SUBROUTINE one(a)
+    integer :: a
+    a = two()
+  END SUBROUTINE one
+  integer FUNCTION two()
+     two = 42
+  END FUNCTION two
+  integer FUNCTION three() bind(C) ! { dg-warning "PRIVATE but has been given the binding label" }
+     three = 43
+  END FUNCTION three
+  integer FUNCTION four() bind(C, name='five') ! { dg-warning "PRIVATE but has been given the binding label" }
+     four = 44
+  END FUNCTION four
+  integer FUNCTION six() bind(C, name='')
+     six = 46
+  END FUNCTION six
+  integer FUNCTION seven() bind(C)
+     seven = 46
+  END FUNCTION seven
+  integer FUNCTION eight() bind(C, name='nine')
+     eight = 48
+  END FUNCTION eight
+  integer FUNCTION ten() bind(C, name='')
+     ten = 48
+  END FUNCTION ten
+END MODULE
+
+! { dg-final { scan-assembler "__m_MOD_one" } }
+! { dg-final { scan-assembler-not "two" } }
+! { dg-final { scan-assembler "three" } }
+! { dg-final { scan-assembler-not "four" } }
+! { dg-final { scan-assembler "five" } }
+! { dg-final { scan-assembler-not "six" } }
+! { dg-final { scan-assembler "seven" } }
+! { dg-final { scan-assembler "nine" } }
+! { dg-final { scan-assembler "__m_MOD_ten" } }