public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [Patch, Fortran] Remove the Fortran-only flag -fno-whole-file
@ 2013-01-04 23:31 Tobias Burnus
  2013-01-07 14:14 ` Tobias Burnus
  2013-02-13 18:20 ` Tobias Burnus
  0 siblings, 2 replies; 6+ messages in thread
From: Tobias Burnus @ 2013-01-04 23:31 UTC (permalink / raw)
  To: gcc patches, gfortran

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

This patch "removes" -fno-whole-file. (Actually, it turns it into "Ignore".)


Reasoning:

* -fwhole-file/-fno-whole-file was added in 4.5 to make the transition 
easier; -fwhole-file is the default since 4.6.

* There are many wrong-code issues and probably also some ICEs with 
-fno-whole file.

* The generated code of -fwhole-file is faster as it allows for inlining.

* -fno-whole-file has been deprecated since 4.6 and announced for removal.

* Code cleanup is always nice (diff -w): 17 insertions(+), 80 deletions(-)


Build and regtested on x86-64-gnu-linux.
OK for the trunk?


Tobias

PS: Dominique pointed out that PR 45128 is a -fwhole-file "regression". 
However, it mainly shows that gfortran needs the new array descriptor to 
fix such subpointer issues (and other PRs).

[-- Attachment #2: remove-whole-file.diff --]
[-- Type: text/x-patch, Size: 12581 bytes --]

2013-01-05  Tobias Burnus  <burnus@net-b.de>

	* gfortran.h (gfc_option_t): Remove flag_whole_file.
	* invoke.texi (-fno-whole-file): Remove.
	* lang.opt (fwhole-file): Change to Ignore.
	* options.c (gfc_init_options, gfc_post_options,
	gfc_handle_option): Remove !flag_whole_file handling
	* parse.c (resolve_all_program_units, translate_all_program_units,
	gfc_parse_file): Ditto.
	* resolve.c (resolve_global_procedure): Ditto.
	* trans-decl.c (gfc_get_symbol_decl, gfc_get_extern_function_decl,
	gfc_create_module_variable): Ditto.
	* trans-types.c (gfc_get_derived_type): Ditto.

diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 027cab6..b87bf64 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -2287,7 +2287,6 @@ typedef struct
   int flag_init_character;
   char flag_init_character_value;
   int flag_align_commons;
-  int flag_whole_file;
   int flag_protect_parens;
   int flag_realloc_lhs;
   int flag_aggressive_function_elimination;
diff --git a/gcc/fortran/invoke.texi b/gcc/fortran/invoke.texi
index d7c3219..4ba94e5 100644
--- a/gcc/fortran/invoke.texi
+++ b/gcc/fortran/invoke.texi
@@ -182,7 +182,7 @@ and warnings}.
 -finit-real=@var{<zero|inf|-inf|nan|snan>} @gol
 -fmax-array-constructor=@var{n} -fmax-stack-var-size=@var{n}
 -fno-align-commons @gol
--fno-automatic -fno-protect-parens -fno-underscoring -fno-whole-file @gol
+-fno-automatic -fno-protect-parens -fno-underscoring @gol
 -fsecond-underscore -fpack-derived -frealloc-lhs -frecursive @gol
 -frepack-arrays -fshort-enums -fstack-arrays
 }
@@ -1293,22 +1293,6 @@ in the source, even if the names as seen by the linker are mangled to
 prevent accidental linking between procedures with incompatible
 interfaces.
 
-@item -fno-whole-file
-@opindex @code{fno-whole-file}
-This flag causes the compiler to resolve and translate each procedure in
-a file separately. 
-
-By default, the whole file is parsed and placed in a single front-end tree.
-During resolution, in addition to all the usual checks and fixups, references
-to external procedures that are in the same file effect resolution of
-that procedure, if not already done, and a check of the interfaces.  The
-dependences are resolved by changing the order in which the file is
-translated into the backend tree.  Thus, a procedure that is referenced
-is translated before the reference and the duplication of backend tree
-declarations eliminated.
-
-The @option{-fno-whole-file} option is deprecated and may lead to wrong code.
-
 @item -fsecond-underscore
 @opindex @code{fsecond-underscore}
 @cindex underscore
diff --git a/gcc/fortran/lang.opt b/gcc/fortran/lang.opt
index 1535187..c885ff3 100644
--- a/gcc/fortran/lang.opt
+++ b/gcc/fortran/lang.opt
@@ -591,8 +591,8 @@ Fortran
 Append underscores to externally visible names
 
 fwhole-file
-Fortran
-Compile all program units at once and check all interfaces
+Fortran Ignore
+Does nothing.  Preserved for backward compatibility.
 
 fworking-directory
 Fortran
diff --git a/gcc/fortran/options.c b/gcc/fortran/options.c
index e05b935..1b2373b 100644
--- a/gcc/fortran/options.c
+++ b/gcc/fortran/options.c
@@ -126,7 +126,6 @@ gfc_init_options (unsigned int decoded_options_count,
   gfc_option.flag_real8_kind = 0;
   gfc_option.flag_dollar_ok = 0;
   gfc_option.flag_underscoring = 1;
-  gfc_option.flag_whole_file = 1;
   gfc_option.flag_f2c = 0;
   gfc_option.flag_second_underscore = -1;
   gfc_option.flag_implicit_none = 0;
@@ -266,14 +265,6 @@ gfc_post_options (const char **pfilename)
     sorry ("-fexcess-precision=standard for Fortran");
   flag_excess_precision_cmdline = EXCESS_PRECISION_FAST;
 
-  /* Whole program needs whole file mode.  */
-  if (flag_whole_program)
-    gfc_option.flag_whole_file = 1;
-
-  /* Enable whole-file mode if LTO is in effect.  */
-  if (flag_lto)
-    gfc_option.flag_whole_file = 1;
-
   /* Fortran allows associative math - but we cannot reassociate if
      we want traps or signed zeros. Cf. also flag_protect_parens.  */
   if (flag_associative_math == -1)
@@ -432,9 +423,6 @@ gfc_post_options (const char **pfilename)
       gfc_option.warn_tabs = 0;
     }
 
-  if (pedantic && gfc_option.flag_whole_file)
-    gfc_option.flag_whole_file = 2;
-
   /* Optimization implies front end optimization, unless the user
      specified it directly.  */
 
@@ -825,10 +813,6 @@ gfc_handle_option (size_t scode, const char *arg, int value,
       gfc_option.flag_underscoring = value;
       break;
 
-    case OPT_fwhole_file:
-      gfc_option.flag_whole_file = value;
-      break;
-
     case OPT_fsecond_underscore:
       gfc_option.flag_second_underscore = value;
       break;
diff --git a/gcc/fortran/parse.c b/gcc/fortran/parse.c
index 659e9fc..5f2c33b 100644
--- a/gcc/fortran/parse.c
+++ b/gcc/fortran/parse.c
@@ -4379,8 +4379,7 @@ add_global_program (void)
 }
 
 
-/* Resolve all the program units when whole file scope option
-   is active. */
+/* Resolve all the program units. */
 static void
 resolve_all_program_units (gfc_namespace *gfc_global_ns_list)
 {
@@ -4421,9 +4420,8 @@ clean_up_modules (gfc_gsymbol *gsym)
 }
 
 
-/* Translate all the program units when whole file scope option
-   is active. This could be in a different order to resolution if
-   there are forward references in the file.  */
+/* Translate all the program units. This could be in a different order
+   to resolution if there are forward references in the file.  */
 static void
 translate_all_program_units (gfc_namespace *gfc_global_ns_list,
 			     bool main_in_tu)
@@ -4548,8 +4546,7 @@ loop:
       accept_statement (st);
       add_global_program ();
       parse_progunit (ST_NONE);
-      if (gfc_option.flag_whole_file)
-	goto prog_units;
+      goto prog_units;
       break;
 
     case ST_SUBROUTINE:
@@ -4557,8 +4554,7 @@ loop:
       push_state (&s, COMP_SUBROUTINE, gfc_new_block);
       accept_statement (st);
       parse_progunit (ST_NONE);
-      if (gfc_option.flag_whole_file)
-	goto prog_units;
+      goto prog_units;
       break;
 
     case ST_FUNCTION:
@@ -4566,8 +4562,7 @@ loop:
       push_state (&s, COMP_FUNCTION, gfc_new_block);
       accept_statement (st);
       parse_progunit (ST_NONE);
-      if (gfc_option.flag_whole_file)
-	goto prog_units;
+      goto prog_units;
       break;
 
     case ST_BLOCK_DATA:
@@ -4594,8 +4589,7 @@ loop:
       push_state (&s, COMP_PROGRAM, gfc_new_block);
       main_program_symbol (gfc_current_ns, "MAIN__");
       parse_progunit (st);
-      if (gfc_option.flag_whole_file)
-	goto prog_units;
+      goto prog_units;
       break;
     }
 
@@ -4612,19 +4606,9 @@ loop:
   if (s.state == COMP_MODULE)
     {
       gfc_dump_module (s.sym->name, errors_before == errors);
-      if (!gfc_option.flag_whole_file)
-	{
-	  if (errors == 0)
-	    gfc_generate_module_code (gfc_current_ns);
-	  pop_state ();
-	  gfc_done_2 ();
-	}
-      else
-	{
-	  gfc_current_ns->derived_types = gfc_derived_types;
-	  gfc_derived_types = NULL;
-	  goto prog_units;
-	}
+      gfc_current_ns->derived_types = gfc_derived_types;
+      gfc_derived_types = NULL;
+      goto prog_units;
     }
   else
     {
@@ -4657,9 +4641,6 @@ prog_units:
 
   done:
 
-  if (!gfc_option.flag_whole_file)
-    goto termination;
-
   /* Do the resolution.  */
   resolve_all_program_units (gfc_global_ns_list);
 
@@ -4678,8 +4659,6 @@ prog_units:
   /* Do the translation.  */
   translate_all_program_units (gfc_global_ns_list, seen_program);
 
-termination:
-
   gfc_end_source_files ();
   return SUCCESS;
 
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 54ac3c6..72e0b0a 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -2153,15 +2153,14 @@ resolve_global_procedure (gfc_symbol *sym, locus *where,
   if ((gsym->type != GSYM_UNKNOWN && gsym->type != type))
     gfc_global_used (gsym, where);
 
-  if (gfc_option.flag_whole_file
-	&& (sym->attr.if_source == IFSRC_UNKNOWN
-	    || sym->attr.if_source == IFSRC_IFBODY)
-	&& gsym->type != GSYM_UNKNOWN
-	&& gsym->ns
-	&& gsym->ns->resolved != -1
-	&& gsym->ns->proc_name
-	&& not_in_recursive (sym, gsym->ns)
-	&& not_entry_self_reference (sym, gsym->ns))
+  if ((sym->attr.if_source == IFSRC_UNKNOWN
+       || sym->attr.if_source == IFSRC_IFBODY)
+      && gsym->type != GSYM_UNKNOWN
+      && gsym->ns
+      && gsym->ns->resolved != -1
+      && gsym->ns->proc_name
+      && not_in_recursive (sym, gsym->ns)
+      && not_entry_self_reference (sym, gsym->ns))
     {
       gfc_symbol *def_sym;
 
@@ -2372,7 +2371,7 @@ resolve_global_procedure (gfc_symbol *sym, locus *where,
 		     "an explicit interface", sym->name, &sym->declared_at);
 	}
 
-      if (gfc_option.flag_whole_file == 1
+      if (!pedantic
 	  || ((gfc_option.warn_std & GFC_STD_LEGACY)
 	      && !(gfc_option.warn_std & GFC_STD_GNU)))
 	gfc_errors_to_warnings (1);
diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index 88f9c56..36b3a29 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -1340,15 +1340,14 @@ gfc_get_symbol_decl (gfc_symbol * sym)
       && sym->attr.flavor == FL_PARAMETER)
     intrinsic_array_parameter = true;
 
-  /* If use associated and whole file compilation, use the module
+  /* If use associated compilation, use the module
      declaration.  */
-  if (gfc_option.flag_whole_file
-	&& (sym->attr.flavor == FL_VARIABLE
-	    || sym->attr.flavor == FL_PARAMETER)
-	&& sym->attr.use_assoc
-	&& !intrinsic_array_parameter
-	&& sym->module
-	&& gfc_get_module_backend_decl (sym))
+  if ((sym->attr.flavor == FL_VARIABLE
+       || sym->attr.flavor == FL_PARAMETER)
+      && sym->attr.use_assoc
+      && !intrinsic_array_parameter
+      && sym->module
+      && gfc_get_module_backend_decl (sym))
     {
       if (sym->ts.type == BT_CLASS && sym->backend_decl)
 	GFC_DECL_CLASS(sym->backend_decl) = 1;
@@ -1638,12 +1637,11 @@ gfc_get_extern_function_decl (gfc_symbol * sym)
      return the backend_decl.  */
   gsym =  gfc_find_gsymbol (gfc_gsym_root, sym->name);
 
-  if (gfc_option.flag_whole_file
-	&& (!sym->attr.use_assoc || sym->attr.if_source != IFSRC_DECL)
-	&& !sym->backend_decl
-	&& gsym && gsym->ns
-	&& ((gsym->type == GSYM_SUBROUTINE) || (gsym->type == GSYM_FUNCTION))
-	&& (gsym->ns->proc_name->backend_decl || !sym->attr.intrinsic))
+  if ((!sym->attr.use_assoc || sym->attr.if_source != IFSRC_DECL)
+      && !sym->backend_decl
+      && gsym && gsym->ns
+      && ((gsym->type == GSYM_SUBROUTINE) || (gsym->type == GSYM_FUNCTION))
+      && (gsym->ns->proc_name->backend_decl || !sym->attr.intrinsic))
     {
       if (!gsym->ns->proc_name->backend_decl)
 	{
@@ -1695,9 +1693,7 @@ gfc_get_extern_function_decl (gfc_symbol * sym)
   if (sym->module)
     gsym =  gfc_find_gsymbol (gfc_gsym_root, sym->module);
 
-  if (gfc_option.flag_whole_file
-	&& gsym && gsym->ns
-	&& gsym->type == GSYM_MODULE)
+  if (gsym && gsym->ns && gsym->type == GSYM_MODULE)
     {
       gfc_symbol *s;
 
@@ -4035,8 +4031,7 @@ gfc_create_module_variable (gfc_symbol * sym)
       decl = sym->backend_decl;
       gcc_assert (sym->ns->proc_name->attr.flavor == FL_MODULE);
 
-      /* -fwhole-file mixes up the contexts so these asserts are unnecessary.  */
-      if (!(gfc_option.flag_whole_file && sym->attr.use_assoc))
+      if (!sym->attr.use_assoc)
 	{
 	  gcc_assert (TYPE_CONTEXT (decl) == NULL_TREE
 		      || TYPE_CONTEXT (decl) == sym->ns->proc_name->backend_decl);
diff --git a/gcc/fortran/trans-types.c b/gcc/fortran/trans-types.c
index 8394bf9..52ca23e 100644
--- a/gcc/fortran/trans-types.c
+++ b/gcc/fortran/trans-types.c
@@ -2372,19 +2372,16 @@ gfc_get_derived_type (gfc_symbol * derived)
     }
 
   /* If use associated, use the module type for this one.  */
-  if (gfc_option.flag_whole_file
-	&& derived->backend_decl == NULL
-	&& derived->attr.use_assoc
-	&& derived->module
-	&& gfc_get_module_backend_decl (derived))
+  if (derived->backend_decl == NULL
+      && derived->attr.use_assoc
+      && derived->module
+      && gfc_get_module_backend_decl (derived))
     goto copy_derived_types;
 
-  /* If a whole file compilation, the derived types from an earlier
-     namespace can be used as the canonical type.  */
-  if (gfc_option.flag_whole_file
-	&& derived->backend_decl == NULL
-	&& !derived->attr.use_assoc
-	&& gfc_global_ns_list)
+  /* The derived types from an earlier namespace can be used as the
+     canonical type.  */
+  if (derived->backend_decl == NULL && !derived->attr.use_assoc
+      && gfc_global_ns_list)
     {
       for (ns = gfc_global_ns_list;
 	   ns->translated && !got_canonical;

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [Patch, Fortran] Remove the Fortran-only flag -fno-whole-file
@ 2013-01-07 16:22 Dominique Dhumieres
  2013-01-07 18:23 ` Tobias Burnus
  0 siblings, 1 reply; 6+ messages in thread
From: Dominique Dhumieres @ 2013-01-07 16:22 UTC (permalink / raw)
  To: fortran; +Cc: gcc-patches, pault, burnus

Sorry for the late and lengthy answer.  To make the story short, I think
the decision to remove -fno-whole-file should be based on the answer to the
following questions:

(1) Is there any hint of -fno-whole-file misbehavior (as suggested by the 
second point of the "Reasoning")?

As said in my comment below, if the answer is yes, then IMO this should be 
investigated and fixed before removal.

(2) Is the -fwhole-file status ZKB (zero known bug)?

I am not 100% confident that this the case (see at least PR 45128).

(3) What is the confidence that there is no rampant bug in -fwhole-file?

While I cannot remember any recent PR in this area (thanks Paul), at the
same time I did not see recent reports (in buzilla or in comp.lang.fortran)
about the common errors which have plagued the f77 to f90 transition.

(4) Is there an easy replacement for -fno-whole-file?

Here the answer is clearly yes: put the different TUs in different files.

So to summarize, I think the key point is the answer to the first 
question. I think the answer is no, but this was not what I understood 
from the original post.

> This patch "removes" -fno-whole-file.  (Actually, it turns it into
> "Ignore".)
>
> Reasoning:
> 
> * -fwhole-file/-fno-whole-file was added in 4.5 to make the transition
> easier; -fwhole-file is the default since 4.6.

My understanding of -fno-whole-file as well as -fno-realloc-lhs, 
-fno-frontend-optimize, or any of the -fno-* options is two fold:

(i) to provide a workaround to user hit by a bug in the corresponding area,
(ii) to help the maintainers to locate the bug or to change the default in 
case of a too severe bug.

So the decision to remove an option should be based on the answer to the 
questions (2) and (3) above.

> * There are many wrong-code issues and probably also some ICEs with
> -fno-whole file.

I think the wording is misleading. I have checked bugzilla for open PR 
containing -fno-whole-file and I have found only 2 (48939 and 52621).
None of them are related to wrong-code issue due to -fno-whole-file.

My understanding of the -fno-whole-file option is that it is (should be) 
strictly equivalent to put all the TUs in separate files. If it exists an 
example for which this is not true, I think it would be a bug, a PR should 
be filled for it, and indeed, the removal of -fno-whole-file should be 
delayed after it the bug(s) is (are) fixed.

If the meaning of the quoted sentence is that -fwhole-file gives errors 
for invalid code (missing interface, argument mismatches, ...), hence 
prevents "wrong-code" issues, this is true. However such errors can only 
be catched if all the involved TUs are in the same file, e.g. I won't get 
any error if I mess up the arguments of a subroutine in the lapack library 
unless I add it (and its dependencies) to my source.

> * The generated code of -fwhole-file is faster as it allows for inlining.

This has nothing to do with the removal of -fno-whole-file.
AFAICT most of the speeddup is achieved with -fwhole-program.
In addition the code has to be in a single file. On platforms with the 
right linker and the right plugin (i.e. not Darwin) the speedup for split 
files can be obtained by compiling the different files with -flto.

> * -fno-whole-file has been deprecated since 4.6 and announced for
> removal.
>

OK, but no time table has been released so far.

> * Code cleanup is always nice (diff -w): 17 insertions(+), 80
> deletions(-)

OK
 
> Build and regtested on x86-64-gnu-linux.  
>
> OK for the trunk?
>
> PS: Dominique pointed out that PR 45128 is a -fwhole-file "regression".
> However, it mainly shows that gfortran needs the new array descriptor to
> fix such subpointer issues (and other PRs).

Compiling PR 45128 with -fwhole-file gives an ICE: Segmentation fault, 
while with -fno-whole-file it gives a linking error

Undefined symbols:
  "_span.0", referenced from:
...

as it does if the two TUs are put is different files.

Dominique

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

end of thread, other threads:[~2013-02-13 19:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-04 23:31 [Patch, Fortran] Remove the Fortran-only flag -fno-whole-file Tobias Burnus
2013-01-07 14:14 ` Tobias Burnus
2013-02-13 18:20 ` Tobias Burnus
2013-02-13 19:29   ` Steve Kargl
2013-01-07 16:22 Dominique Dhumieres
2013-01-07 18:23 ` Tobias Burnus

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).