* [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-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
1 sibling, 0 replies; 6+ messages in thread
From: Tobias Burnus @ 2013-01-07 14:14 UTC (permalink / raw)
To: gcc patches, gfortran
Early * ping * the patch below, i.e.
http://gcc.gnu.org/ml/fortran/2013-01/msg00033.html
Other pending patches by me:
http://gcc.gnu.org/ml/fortran/2013-01/msg00049.html
http://gcc.gnu.org/ml/fortran/2013-01/msg00025.html
Other pending patches by â¦
Thomas: http://gcc.gnu.org/ml/fortran/2013-01/msg00000.html
Janne: http://gcc.gnu.org/ml/fortran/2013-01/msg00037.html
Tobias Burnus wrote:
> 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).
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch, Fortran] Remove the Fortran-only flag -fno-whole-file
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
1 sibling, 1 reply; 6+ messages in thread
From: Tobias Burnus @ 2013-02-13 18:20 UTC (permalink / raw)
To: gcc patches, gfortran
*** PING ***
I think it is now a bit late for 4.8. Thus, I change my request to: OK
for the 4.9 trunk?
Tobias
On January 5, 2013 00:31, Tobias Burnus wrote:
> 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).
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch, Fortran] Remove the Fortran-only flag -fno-whole-file
2013-02-13 18:20 ` Tobias Burnus
@ 2013-02-13 19:29 ` Steve Kargl
0 siblings, 0 replies; 6+ messages in thread
From: Steve Kargl @ 2013-02-13 19:29 UTC (permalink / raw)
To: Tobias Burnus; +Cc: gcc patches, gfortran
On Wed, Feb 13, 2013 at 07:20:10PM +0100, Tobias Burnus wrote:
> *** PING ***
>
> I think it is now a bit late for 4.8. Thus, I change my request to: OK
> for the 4.9 trunk?
>
IMHO, yes. Don't know if others have an opinion, but
waiting any longer would seem to be counter productive.
--
Steve
^ 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, 0 replies; 6+ messages in thread
From: Tobias Burnus @ 2013-01-07 18:23 UTC (permalink / raw)
To: Dominique Dhumieres; +Cc: fortran, gcc-patches, pault
Dominique Dhumieres wrote:
> (1) Is there any hint of -fno-whole-file misbehavior (as suggested by the
> second point of the "Reasoning")?
The middle end is simply not prepared for having multiple declaration
for a single file ("translation unit"), which leads to wrong
optimizations and thus to wrong code.
I think the bug which triggeded my decision to fix all remaining known
-fno-whole-file issues and then to enable it by default was PR 44945.
See also http://gcc.gnu.org/ml/fortran/2010-07/msg00321.html
But there are surely more. Using multiple declarations was simply a
design bug in the original implementation of gfortran, which was only
realized rather late - and then deferred. Paul was that brave to fix
this by implementing -fwhole-file.
> As said in my comment below, if the answer is yes, then IMO this should be
> investigated and fixed before removal.
We did the investigation when switching to -fwhole-file by default. And
no, I do not want to investigate why the middle end "misbehaves" with
multiple declarations in a single translation unit - nor do I want to
fix it. I am content if everything works with -fwhole-file. There is
really not any advantage of having multiple declarations in a single TU.
> (2) Is the -fwhole-file status ZKB (zero known bug)?
> I am not 100% confident that this the case (see at least PR 45128).
I am not aware of any bug since 4.6. I am rather certain that we fixed
all known bugs in this area. Neither having remaining mult-decls nor
issues which wouldn't occur with -fno-whole-file. By now it simply makes
more sense to fix a bug, ignoring whether it worked by chance with
-fno-whole-file or not. All recent regressions seem to be unrelated.
Thus, for users it makes more sense to use an older version or wait for
the patch than to try -fno-while-file. Given the potential to generate
wrong-code bugs, I wouldn't trust -fno-whole-file for any code which
goes beyond a hello-world example.
PR45128 requires the new array descriptor; we have a kludge which allows
sub-type pointer in some cases. Seemingly, that kludge works in one case
with -fno-whole-file where it fails with -fwhole-file. But I will
actively ignore all subpointer PRs until the new array descriptor is
ready, which will be the right fix.
> (3) What is the confidence that there is no rampant bug in -fwhole-file?
Well, even if there is a bug which doesn't affect -fno-whole-file, using
-fno-whole-file is no solution, given that -fno-whole-file has many
known bugs.
And while I am willing to fix -fwhole-file bugs, I will ignore any
-fno-whole-file ones.
> (4) Is there an easy replacement for -fno-whole-file?
Yes: -fwhole-file :-) Really, you shouldn't think of -fno-whole-file
anymore.
> Here the answer is clearly yes: put the different TUs in different files.
Or doing that ;-)
> 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.
Well, the main point of -fno-realloc-lhs is to disable the performance
penalty of RHS. That is also helped to find some bugs was a side effect.
For -fno-frontend-optimize I agree that its purpose was indeed to toggle
the optimization separately from only using -O0 vs -O1.
But there is a difference between -frealloc-lhs, -ffrontend-optimize and
-fwhole-file: The former two enable a specific feature or optimization
and valid programs can (should) be produced with either version. By
contrast, in terms of middle-end assumptions -fno-whole-file is always
wrong.
I think (ii) does not really apply. We never changed the defaults when a
serious bug popped up, we always fixed it. [Though we might have
suggested a command-line option as workaround, cf. (i).] For
-fno-whole-file, it also didn't help much with localizing the bug. For
-f(no-)frontend-optimize I agree that it helped several times to isolate
the bug.
>> * 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.
You have to search for mult-decl issues. -fwhole-file was the cure of
that problem; as it had a couple of issues before July 2010 and wasn't
enabled by default, almost no one used it. Hence, you don't find it in
the log. And after -fwhole-file became the default, no one cared anymore
about issues which occurred only with -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
But that's not how the middle end works. It expects that in a single
file (translation unit), no multiple declarations for the same object exist.
We do not want to change the middle end. Besides, having a single decl
per object allows for inlining and similar optimization. There is simply
no reason for -fno-whole-file except that gfortran until 4.5 had it by
default.
In principle, we could have done the switch without adding
-f(no-)whole-file; having the flag was only useful because: (i) It
allowed Paul to sensible put the patch into the trunk even though it was
not yet completely working. (ii) It allowed to find out that PR44945 was
only failing with -fno-whole-file. (iii) Mainly in 2010-07, it made it
easier to find regressions in -fwhole-file. After 2010-07, most issues
were solved, rendering -f(no-)whole-file less useful.
>> * 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.
No but with the addition of -fwhole-file. Without, inlining was
essentially not possible.
> AFAICT most of the speeddup is achieved with -fwhole-program.
But that only works with a single file/TU, unless you use LTO.
Additionally, -flto and -fwhole-program both enable -fwhole-file. Due to
the multiple declarations issue, -fno-whole-file -fwhole-program would
optimize many procedures away, which were actually used - leading to
linking errors.
> OK, but no time table has been released so far.
In terms of the bugs, I was even considering to remove it for 4.6. I
think by 2010-08 we had fixed nearly all -fwhole-file specific bugs -
and no one wanted to go back to -fno-whole-file.
> 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
As said above: I will actively ignore subpointer bugs! Actually, before
I thought that -fno-whole-file worked for that file, but as the linker
error indicates, -fno-whole-file also doesn't work!
Tobias
^ 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).