Bug 54687 - Use gcc option machinery for gfortran
Summary: Use gcc option machinery for gfortran
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.8.0
: P3 enhancement
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: easyhack
Depends on: 64334
Blocks:
  Show dependency treegraph
 
Reported: 2012-09-24 05:31 UTC by Thomas Koenig
Modified: 2016-02-25 15:33 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2012-09-24 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Koenig 2012-09-24 05:31:41 UTC
It would be nice if gfortran used the normal gcc option
machinery (including the generated variables).

See

http://gcc.gnu.org/ml/fortran/2012-09/msg00090.html

for some details.
Comment 1 Manuel López-Ibáñez 2012-09-24 10:46:41 UTC
This is also a pre-requisite for using LangEnabledBy to encode options dependencies in fortran/lang.opt file.
Comment 2 Manuel López-Ibáñez 2014-04-30 23:43:38 UTC
Note that this is a repetitive but trivial task that consists mostly in deleting code. Just take for example Wcompare-reals:

Index: gfortran.h
===================================================================
--- gfortran.h	(revision 209347)
+++ gfortran.h	(working copy)
@@ -2256,11 +2267,10 @@ typedef struct
   int warn_real_q_constant;
   int warn_unused_dummy_argument;
   int warn_zerotrip;
   int warn_realloc_lhs;
   int warn_realloc_lhs_all;
-  int warn_compare_reals;
   int warn_target_lifetime;
   int max_errors;
 
   int flag_all_intrinsics;
   int flag_default_double;
Index: lang.opt
===================================================================
--- lang.opt	(revision 209347)
+++ lang.opt	(working copy)
@@ -220,11 +220,11 @@ Fortran
 Wcharacter-truncation
 Fortran Warning
 Warn about truncated character expressions
 
 Wcompare-reals
-Fortran Warning
+Fortran Var(warn_compare_reals) Warning LangEnabledBy(Fortran,Wextra)
 Warn about equality comparisons involving REAL or COMPLEX expressions
 
 Wconversion
 Fortran Warning
 ; Documented in C
Index: resolve.c
===================================================================
--- resolve.c	(revision 209347)
+++ resolve.c	(working copy)
@@ -3566,11 +3566,11 @@ resolve_operator (gfc_expr *e)
 	  gfc_type_convert_binary (e, 1);
 
 	  e->ts.type = BT_LOGICAL;
 	  e->ts.kind = gfc_default_logical_kind;
 
-	  if (gfc_option.warn_compare_reals)
+	  if (warn_compare_reals)
 	    {
 	      gfc_intrinsic_op op = e->value.op.op;
 
 	      /* Type conversion has made sure that the types of op1 and op2
 		 agree, so it is only necessary to check the first one.   */
Index: options.c
===================================================================
--- options.c	(revision 209347)
+++ options.c	(working copy)
@@ -110,11 +110,10 @@ gfc_init_options (unsigned int decoded_o
   gfc_option.warn_real_q_constant = 0;
   gfc_option.warn_unused_dummy_argument = 0;
   gfc_option.warn_zerotrip = 0;
   gfc_option.warn_realloc_lhs = 0;
   gfc_option.warn_realloc_lhs_all = 0;
-  gfc_option.warn_compare_reals = 0;
   gfc_option.warn_target_lifetime = 0;
   gfc_option.max_errors = 25;
 
   gfc_option.flag_all_intrinsics = 0;
   gfc_option.flag_default_double = 0;
@@ -470,11 +461,5 @@
-/* Set the options for -Wextra.  */
-
-static void
-set_Wextra (int setting)
-{
-  gfc_option.warn_compare_reals = setting;
 }
 
 static void
 gfc_handle_module_path_options (const char *arg)
 {
@@ -659,31 +632,18 @@ gfc_handle_option (size_t scode, const c
 
     case OPT_Wcharacter_truncation:
       gfc_option.warn_character_truncation = value;
       break;
 
-    case OPT_Wcompare_reals:
-      gfc_option.warn_compare_reals = value;
-      break;
-
     case OPT_Wconversion:
       gfc_option.gfc_warn_conversion = value;
       break;
 
     case OPT_Wconversion_extra:
       gfc_option.warn_conversion_extra = value;
       break;
 
-    case OPT_Wextra:
-      handle_generated_option (&global_options, &global_options_set,
-			       OPT_Wunused_parameter, NULL, value,
-			       gfc_option_lang_mask (), kind, loc,
-			       handlers, global_dc);
-      set_Wextra (value);
-
-      break;
-
     case OPT_Wfunction_elimination:
       gfc_option.warn_function_elimination = value;
       break;
 
     case OPT_Wimplicit_interface:
Comment 3 Manuel López-Ibáñez 2014-10-07 16:13:54 UTC
Author: manu
Date: Tue Oct  7 16:13:22 2014
New Revision: 215974

URL: https://gcc.gnu.org/viewcvs?rev=215974&root=gcc&view=rev
Log:
gcc/fortran/ChangeLog:

2014-10-06  Manuel López-Ibáñez  <manu@gcc.gnu.org>

	PR fortran/44054
	PR fortran/54687
	* gfortran.h (gfc_warning_cmdline): Add overload that takes an
	option.
	(gfc_error_cmdline): Declare.
	* error.c (gfc_warning_cmdline): New overload that takes an option.
	(gfc_error_cmdline): New.
	* lang.opt (Wmissing-include-dirs): New.
	* scanner.c (add_path_to_list): Use the new functions.
	(load_file): Likewise.
	* options.c (gfc_init_options): Wmissing-include-dirs is enabled
	by default in Fortran.
	(gfc_handle_option): Accept automatically handled options.



Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/error.c
    trunk/gcc/fortran/gfortran.h
    trunk/gcc/fortran/lang.opt
    trunk/gcc/fortran/options.c
    trunk/gcc/fortran/scanner.c
Comment 4 Tobias Burnus 2014-12-11 16:25:51 UTC
At least all warnings should be handled now (work done as part of PR44054 and in the separate but related commit r218188).

Is there still something missing?
Comment 5 Manuel López-Ibáñez 2014-12-11 16:58:50 UTC
(In reply to Tobias Burnus from comment #4)
> At least all warnings should be handled now (work done as part of PR44054
> and in the separate but related commit r218188).
> 
> Is there still something missing?

Note that the options machinery has ways to encode String->Enum options, so things like gfc_handle_coarray_option and the handling of OPT_finit_real_ are redundant (and others).

(Personally, I think the way to describe such options in the .opt file is a bit ugly, but incremental improvements like your '||' patch are possible)

The options machinery can also handle Alias options transparently such as OPT_fdump_fortran_original and OPT_fdump_parse_tree.

Also, my intuition tells me that any explicit handling of optimization options that does not use the common machinery such as:

  if (gfc_option.flag_protect_parens == -1)
    gfc_option.flag_protect_parens = !optimize_fast;

  if (gfc_option.flag_stack_arrays == -1)
    gfc_option.flag_stack_arrays = optimize_fast;

will at some moment, if not already, miss any automatic merging of optimization options for LTO or automatic setting via #pragma and attribute "optimize" https://gcc.gnu.org/onlinedocs/gcc/Function-Specific-Option-Pragmas.html (see also generated file options-save.c)

In the future, it would be ideal to have separate option structs for FE-specific options, such that not all FEs have to see all options from all other FEs. Although moving options from gfc_option_t to the globally generated option struct may seem a step backward in that respect, it is actually a step forward, since those separated structs should be generated directly from the *.opt files. 

Finally, most, if not all, of gfc_option_t and gfc_handle_option seems redundant given the features of opt files (and the possibility of improving the generators). There is the chance to remove a lot of code from the Fortran FE, which should be always a good thing, no?

But ultimately, this is up to the Fortran devs. This doesn't block PR44054 anymore since all the warning flags have been moved.
Comment 6 Tobias Burnus 2014-12-16 19:29:52 UTC
Missed the PR number for the commit r218790:

2014-12-16  Tobias Burnus  <burnus@net-b.de>

        PR fortran/54687
        * lang.opt (fsecond-underscore, frecord-marker=8, frecord-marker=4,
        frealloc-lhs, freal-8-real-16, freal-8-real-10, freal-8-real-4,
        freal-4-real-16, freal-4-real-10, freal-4-real-8, fprotect-parens,
        fstack-arrays, fmax-stack-var-size=, fmax-subrecord-length=,
        ffrontend-optimize, ffree-line-length-, ffixed-line-length-,
        finteger-4-integer-8, fdefault-real-8, fdefault-integer-8,
        fdefault-double-8): Add Var() and Init().
        * gfortran.h (gfc_option_t): Remove moved flags.
        * options.c (gfc_init_options, gfc_handle_option): Ditto.
        (gfc_post_options): Update for name change.
        * decl.c (gfc_match_old_kind_spec, gfc_match_kind_spec): Handle
        flag-name change.
        * frontend-passes.c (gfc_run_passes): Ditto.
        * module.c (use_iso_fortran_env_module): Ditto.
        * primary.c (match_integer_constant, match_real_constant): Ditto.
        * resolve.c (resolve_ordinary_assign): Ditto.
        * scanner.c (gfc_next_char_literal, load_line): Ditto.
        * trans-array.c (gfc_trans_allocate_array_storage,
        gfc_conv_resolve_dependencies, gfc_trans_auto_array_allocation,
        gfc_conv_ss_startstride): Ditto.
        * trans-common.c (gfc_sym_mangled_common_id): Ditto.
        * trans-decl.c (gfc_sym_mangled_function_id,
        create_main_function): Ditto.
        * trans-expr.c (gfc_conv_expr_op, gfc_conv_procedure_call,
        arrayfunc_assign_needs_temporary, gfc_trans_arrayfunc_assign,
        gfc_trans_assignment_1): Ditto.
        * trans-stmt.c (gfc_trans_allocate): Ditto.
        * trans-types.c (gfc_init_kinds): Ditto.
Comment 7 Tobias Burnus 2014-12-16 20:45:17 UTC
Author: burnus
Date: Tue Dec 16 20:44:45 2014
New Revision: 218792

URL: https://gcc.gnu.org/viewcvs?rev=218792&root=gcc&view=rev
Log:
2014-12-16  Tobias Burnus  <burnus@net-b.de>

        PR fortran/54687
        * gfortran.h (gfc_option_t): Remove flags which now
        have a Var().
        * lang.opt (flag-aggressive_function_elimination,
        flag-align_commons, flag-all_intrinsics,
        flag-allow_leading_underscore, flag-automatic, flag-backslash,
        flag-backtrace, flag-blas_matmul_limit, flag-cray_pointer,
        flag-dollar_ok, flag-dump_fortran_original,
        flag-dump_fortran_optimized, flag-external_blas, flag-f2c,
        flag-implicit_none, flag-max_array_constructor,
        flag-module_private, flag-pack_derived, flag-range_check,
        flag-recursive, flag-repack_arrays, flag-sign_zero,
        flag-underscoring): Add Var() and, where applicable, Enum().
        * options.c (gfc_init_options, gfc_post_options,
        gfc_handle_option): Update for *.opt changes.
        * arith.c: Update for flag-variable name changes.
        * array.c: Ditto.
        * cpp.c: Ditto.
        * decl.c: Ditto.
        * expr.c: Ditto.
        * f95-lang.c: Ditto.
        * frontend-passes.c: Ditto.
        * intrinsic.c: Ditto.
        * io.c: Ditto.
        * match.c: Ditto.
        * module.c: Ditto.
        * parse.c: Ditto.
        * primary.c: Ditto.
        * resolve.c: Ditto.
        * scanner.c: Ditto.
        * simplify.c: Ditto.
        * symbol.c: Ditto.
        * trans-array.c: Ditto.
        * trans-common.c: Ditto.
        * trans-decl.c: Ditto.
        * trans-expr.c: Ditto.
        * trans-intrinsic.c: Ditto.
        * trans-openmp.c: Ditto.
        * trans-types.c: Ditto.


Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/arith.c
    trunk/gcc/fortran/array.c
    trunk/gcc/fortran/cpp.c
    trunk/gcc/fortran/decl.c
    trunk/gcc/fortran/expr.c
    trunk/gcc/fortran/f95-lang.c
    trunk/gcc/fortran/frontend-passes.c
    trunk/gcc/fortran/gfortran.h
    trunk/gcc/fortran/intrinsic.c
    trunk/gcc/fortran/io.c
    trunk/gcc/fortran/lang.opt
    trunk/gcc/fortran/match.c
    trunk/gcc/fortran/module.c
    trunk/gcc/fortran/options.c
    trunk/gcc/fortran/parse.c
    trunk/gcc/fortran/primary.c
    trunk/gcc/fortran/resolve.c
    trunk/gcc/fortran/scanner.c
    trunk/gcc/fortran/simplify.c
    trunk/gcc/fortran/symbol.c
    trunk/gcc/fortran/trans-array.c
    trunk/gcc/fortran/trans-common.c
    trunk/gcc/fortran/trans-decl.c
    trunk/gcc/fortran/trans-expr.c
    trunk/gcc/fortran/trans-intrinsic.c
    trunk/gcc/fortran/trans-openmp.c
    trunk/gcc/fortran/trans-types.c
Comment 8 Tobias Burnus 2014-12-16 22:25:00 UTC
(In reply to Manuel López-Ibáñez from comment #5)
> > Is there still something missing?
> Note that the options machinery has ways to encode String->Enum options, so
> things like gfc_handle_coarray_option and the handling of OPT_finit_real_
> are redundant (and others).

Pending patch (awaiting ME review): https://gcc.gnu.org/ml/gcc-patches/2014-12/msg01377.html


> Also, my intuition tells me that any explicit handling of optimization
> options that does not use the common machinery such as:
>   if (gfc_option.flag_protect_parens == -1)
>   if (gfc_option.flag_stack_arrays == -1)

Those should be fine as they are completely handled in the FE: The protect_parens translates into PAREN_EXPR and stack_arrays is also a choice done by the FE.


> In the future, it would be ideal to have separate option structs for
> FE-specific options, such that not all FEs have to see all options from all
> other FEs. Although moving options from gfc_option_t to the globally
> generated option struct may seem a step backward in that respect, it is
> actually a step forward, since those separated structs should be generated
> directly from the *.opt files.

I concur. I think it also would be useful to support combined options like Fortran's -ffpe-trap= and -fcheck= or common's -fsanitize=.
Comment 9 Manuel López-Ibáñez 2014-12-16 22:40:22 UTC
(In reply to Tobias Burnus from comment #8)
> I concur. I think it also would be useful to support combined options like
> Fortran's -ffpe-trap= and -fcheck= or common's -fsanitize=.

Total agreement! 

In my wildest dreams most of invoke.texi would be auto-generated from the opt files.

Please could you open PRs for such missing features of the common machinery?
Comment 10 Tobias Burnus 2014-12-17 06:30:02 UTC
Author: burnus
Date: Wed Dec 17 06:29:30 2014
New Revision: 218808

URL: https://gcc.gnu.org/viewcvs?rev=218808&root=gcc&view=rev
Log:
2014-12-17  Tobias Burnus  <burnus@net-b.de>

        PR fortran/54687
gcc/
        * flag-types.h (gfc_init_local_real, gfc_fcoarray,
        gfc_convert): New enums; moved from fortran/.

gcc/fortran/
        * gfortran.h (gfc_option_t): Remove flags which now
        have a Var().
        (init_local_real, gfc_fcoarray): Moved to ../flag-types.h.
        * libgfortran.h (unit_convert): Add comment.
        * lang.opt (flag-convert, flag-init_real, flag-coarray):
        Add Var() and Enum().
        * options.c (gfc_handle_coarray_option): Remove.
        (gfc_init_options, gfc_post_options, gfc_handle_option):
        Update for *.opt changes.
        * array.c: Update for flag-variable name changes.
        * check.c: Ditto.
        * match.c: Ditto.
        * resolve.c: Ditto.
        * simplify.c: Ditto.
        * trans-array.c: Ditto.
        * trans-decl.c: Ditto.
        * trans-expr.c: Ditto.
        * trans-intrinsic.c: Ditto.
        * trans-stmt.c: Ditto.
        * trans-types.c: Ditto.
        * trans.c: Ditto.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/flag-types.h
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/array.c
    trunk/gcc/fortran/check.c
    trunk/gcc/fortran/gfortran.h
    trunk/gcc/fortran/lang.opt
    trunk/gcc/fortran/libgfortran.h
    trunk/gcc/fortran/match.c
    trunk/gcc/fortran/options.c
    trunk/gcc/fortran/resolve.c
    trunk/gcc/fortran/simplify.c
    trunk/gcc/fortran/trans-array.c
    trunk/gcc/fortran/trans-decl.c
    trunk/gcc/fortran/trans-expr.c
    trunk/gcc/fortran/trans-intrinsic.c
    trunk/gcc/fortran/trans-stmt.c
    trunk/gcc/fortran/trans-types.c
    trunk/gcc/fortran/trans.c
Comment 11 Dominique d'Humieres 2016-02-25 14:10:33 UTC
What is left in this PR before closing it as FIXED?
Comment 12 Manuel López-Ibáñez 2016-02-25 15:33:10 UTC
(In reply to Dominique d'Humieres from comment #11)
> What is left in this PR before closing it as FIXED?

My understanding is that almost all entries in gfc_option_t are duplicated in the common machinery (and if not, that would be quite strange and possibly buggy, or just they are not command-line options, so why they reside in gfc_option_t?).

My intuition is that almost all the code in gfc_handle_option can be generated automatically by using the appropriate flags in fortran/lang.opt (https://gcc.gnu.org/onlinedocs/gccint/Option-properties.html). But that could  be done incrementally apart from this bug.

There are some things that both the common machinery and Fortran do manually (PR64334). Factoring out that code (possibly generating it automatically from the .opt files) would be nice, but not essential (so PR64334 does not actually block this).