PR fortran/87919 patch for -fno-dec-structure

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

PR fortran/87919 patch for -fno-dec-structure

Mark Eggleston
Please find attached the patch and a ChangeLog entry. This is my first
patch, apologies for any mistakes in the submission process.

This patch is the simple removal of an OPT_dec_structure case from a
switch statement, it was noticeable as there was no corresponding code
for the other dec specific options. I spotted this before the creation
of PR fortran/87919 so I now know that this fixes the broken behaviour
of -fno-dec-structure.

The patch contains a change to gcc/fortran/options.c and four testcases
pr87919-dec-structure-*.f where * is one of 1, 2, 3 and 4.

The testcases are specified to compile with no specific options, -fdec,
-fdec-structure and both -fdec and -fno-dec-structure.

After building the compiler on x86_64 I got the following results
aggregated from make -j 5 check-fortran:

         === gfortran Summary ===

# of expected passes        48184
# of expected failures        103
# of unsupported tests        79


--
https://www.codethink.co.uk/privacy.html


ChangeLogEntry.txt (186 bytes) Download Attachment
pr87919-dec-structure.patch (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: PR fortran/87919 patch for -fno-dec-structure

Jakub Jelinek
On Wed, Nov 07, 2018 at 03:07:04PM +0000, Mark Eggleston wrote:

> PR fortran/87919
> * options.c (gfc_handle_option): Removed case OPT_fdec_structure
> as it breaks the handling of -fno-dec-structure.

No entries for the tests, i.e.
        * gfortran.dg/pr87919-dec-structure-1.f: New test.
        * gfortran.dg/pr87919-dec-structure-2.f: New test.
        * gfortran.dg/pr87919-dec-structure-3.f: New test.
        * gfortran.dg/pr87919-dec-structure-4.f: New test.

> diff --git a/gcc/fortran/options.c b/gcc/fortran/options.c
> index 73f5389361d9..3b7c2d40fe8a 100644
> --- a/gcc/fortran/options.c
> +++ b/gcc/fortran/options.c
> @@ -761,10 +761,6 @@ gfc_handle_option (size_t scode, const char *arg, HOST_WIDE_INT value,
>        /* Enable all DEC extensions.  */
>        set_dec_flags (1);
>        break;
> -
> -    case OPT_fdec_structure:
> -      flag_dec_structure = 1;
> -      break;
>      }
>  
>    Fortran_handle_option_auto (&global_options, &global_options_set,

LGTM, but I'll defer the final review to Fortran maintainers.

> diff --git a/gcc/testsuite/gfortran.dg/pr87919-dec-structure-1.f b/gcc/testsuite/gfortran.dg/pr87919-dec-structure-1.f
> new file mode 100644
> index 000000000000..4dd34082b97a
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/pr87919-dec-structure-1.f
> @@ -0,0 +1,21 @@
> +! { dg-do compile }
> +!
> +! PR/fortran/87919

Without the first /, i.e.
! PR fortran/87919
(in all tests).

> +++ b/gcc/testsuite/gfortran.dg/pr87919-dec-structure-2.f
> @@ -0,0 +1,22 @@
> +! { dg-do run }
> +! { dg-options "-fdec" }
> +!
> +! PR/fortran/87919
> +!
> +! Should compile anf run with the -fdec option

s/anf/and/ (in several tests).

> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/pr87919-dec-structure-3.f
> @@ -0,0 +1,22 @@
> +! { dg-do run }
> +! { dg-options "-fdec-structure" }
> +!
> +! PR/fortran/87919
> +!
> +! Should compile anf run with the -fdec option

s/-fdec/-fdec-structure/ in this case.

> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/pr87919-dec-structure-4.f
> @@ -0,0 +1,22 @@
> +! { dg-do compile }
> +! { dg-options "-fdec -fno-dec-structure" }
> +!
> +! PR/fortran/87919
> +!
> +! Should fail to compile with the -fdec and -fno-dec-structure option

s/option/options/

I'd suggest to add another test, with
! { dg-options "-fdec-structure -fno-dec-structure" }
where the options cancel each other and the result is no DEC structure
support.

        Jakub
Reply | Threaded
Open this post in threaded view
|

Re: PR fortran/87919 patch for -fno-dec-structure

Fritz Reese
On 11/7/18, Jakub Jelinek <[hidden email]> wrote:

> On Wed, Nov 07, 2018 at 03:07:04PM +0000, Mark Eggleston wrote:
>
>>      PR fortran/87919
>>      * options.c (gfc_handle_option): Removed case OPT_fdec_structure
>>      as it breaks the handling of -fno-dec-structure.
>
> No entries for the tests, i.e.
>       * gfortran.dg/pr87919-dec-structure-1.f: New test.
>       * gfortran.dg/pr87919-dec-structure-2.f: New test.
>       * gfortran.dg/pr87919-dec-structure-3.f: New test.
>       * gfortran.dg/pr87919-dec-structure-4.f: New test.
>
>> diff --git a/gcc/fortran/options.c b/gcc/fortran/options.c
>> index 73f5389361d9..3b7c2d40fe8a 100644
>> --- a/gcc/fortran/options.c
>> +++ b/gcc/fortran/options.c
>> @@ -761,10 +761,6 @@ gfc_handle_option (size_t scode, const char *arg,
>> HOST_WIDE_INT value,
>>        /* Enable all DEC extensions.  */
>>        set_dec_flags (1);
>>        break;
>> -
>> -    case OPT_fdec_structure:
>> -      flag_dec_structure = 1;
>> -      break;
>>      }
>>
>>    Fortran_handle_option_auto (&global_options, &global_options_set,
>
> LGTM, but I'll defer the final review to Fortran maintainers.
Thanks for the patch Mark, I concur with Jakub that it is correct for
what it does. However, I have a few comments in addition to the fixes
recommended by Jakub regarding the test cases.

First, I would prefer to name these test cases as "dec_structure_*.f"
to align with the other (23) -fdec-structure test cases. Second, the
third case (*dec-structure-3.f) is unnecessary because it is identical
in function to dec_structure_1.f90. I concur with the remaining test
cases, as well as Jakub's suggestion to cover "-fdec-structure
-fno-dec-structure" with an additional test. I would name the final
four (= 4 - 1 + 1) tests as "dec_structure_[24-27].f".


I have taken the liberty of extending this patch to cover the
remainder of PR 87919. That is, to fix -fno-* for -fno-dec,
-fno-check-array-temporaries and -fno-init-local-zero. In the extended
patch, the 'value' set for the aforementioned options is no longer
ignored, so that value=1 truly means set and value=0 truly means
"unset". Previously, the aforementioned flags effectively ignored the
value=0 condition. Similarly to the tests Mark provided with
-fdec-structure, I've provided new tests for the various facets of
-fno-dec, -fno-check-array-temporaries, and -fno-init-local-zero.

Below is the changelog. Bootstraps and regtests fine for me on
x86_64-redhat-linux. If it looks OK I'll commit to trunk (and probably
backport to 8-branch and 7-branch since the affected code appears to
be the same for those branches).


From 2d9e39bbf4a179ae433f33f4e7039b85078ba72f Mon Sep 17 00:00:00 2001
From: Fritz Reese <[hidden email]>
Date: Wed, 7 Nov 2018 15:13:50 -0500
Subject: [PATCH] PR fortran/87919

Fix handling -fno-* prefix for init-local-zero, check-array-temporaries and dec.

gcc/fortran/
        * options.c (SET_FLAG, SET_BITFLAG): New macros.
        (set_dec_flags): Unset DEC flags with value==0.
        (set_init_local_zero): New helper for -finit-local-zero flag group.
        (gfc_init_options): Fix disabling of init flags, array temporaries
        check, and dec flags when value is zero (from -fno-*).

gcc/testsuiste/
        * gfortran.dg/array_temporaries_5.f90: New test.
        * gfortran.dg/dec_bitwise_ops_3.f90: Ditto.
        * gfortran.dg/dec_d_lines_3.f: Ditto.
        * gfortran.dg/dec_exp_4.f90: Ditto.
        * gfortran.dg/dec_exp_5.f90: Ditto.
        * gfortran.dg/dec_io_7.f90: Ditto.
        * gfortran.dg/dec_structure_24.f: Ditto.
        * gfortran.dg/dec_structure_25.f: Ditto.
        * gfortran.dg/dec_structure_26.f: Ditto.
        * gfortran.dg/dec_structure_27.f: Ditto.
        * gfortran.dg/dec_type_print_3.f90: Ditto.
        * gfortran.dg/init_flag_20.f90: Ditto.
---
 gcc/fortran/options.c                             | 70 +++++++++++++++--------
 gcc/testsuite/gfortran.dg/array_temporaries_5.f90 | 20 +++++++
 gcc/testsuite/gfortran.dg/dec_bitwise_ops_3.f90   | 19 ++++++
 gcc/testsuite/gfortran.dg/dec_d_lines_3.f         | 10 ++++
 gcc/testsuite/gfortran.dg/dec_exp_4.f90           | 13 +++++
 gcc/testsuite/gfortran.dg/dec_exp_5.f90           | 15 +++++
 gcc/testsuite/gfortran.dg/dec_io_7.f90            | 22 +++++++
 gcc/testsuite/gfortran.dg/dec_structure_24.f      | 21 +++++++
 gcc/testsuite/gfortran.dg/dec_structure_25.f      | 22 +++++++
 gcc/testsuite/gfortran.dg/dec_structure_26.f      | 22 +++++++
 gcc/testsuite/gfortran.dg/dec_structure_27.f      | 20 +++++++
 gcc/testsuite/gfortran.dg/dec_type_print_3.f90    | 29 ++++++++++
 gcc/testsuite/gfortran.dg/init_flag_20.f90        | 62 ++++++++++++++++++++
 13 files changed, 320 insertions(+), 25 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/array_temporaries_5.f90
 create mode 100644 gcc/testsuite/gfortran.dg/dec_bitwise_ops_3.f90
 create mode 100644 gcc/testsuite/gfortran.dg/dec_d_lines_3.f
 create mode 100644 gcc/testsuite/gfortran.dg/dec_exp_4.f90
 create mode 100644 gcc/testsuite/gfortran.dg/dec_exp_5.f90
 create mode 100644 gcc/testsuite/gfortran.dg/dec_io_7.f90
 create mode 100644 gcc/testsuite/gfortran.dg/dec_structure_24.f
 create mode 100644 gcc/testsuite/gfortran.dg/dec_structure_25.f
 create mode 100644 gcc/testsuite/gfortran.dg/dec_structure_26.f
 create mode 100644 gcc/testsuite/gfortran.dg/dec_structure_27.f
 create mode 100644 gcc/testsuite/gfortran.dg/dec_type_print_3.f90
 create mode 100644 gcc/testsuite/gfortran.dg/init_flag_20.f90

0001-PR-fortran-87919.patch (24K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: PR fortran/87919 patch for -fno-dec-structure

Fritz Reese
On Wed, Nov 7, 2018 at 5:32 PM Jakub Jelinek <[hidden email]> wrote:

>
> On Wed, Nov 07, 2018 at 05:05:13PM -0500, Fritz Reese wrote:
>
> --- a/gcc/fortran/options.c
> +++ b/gcc/fortran/options.c
> @@ -32,6 +32,20 @@ along with GCC; see the file COPYING3.  If not see
>
>  gfc_option_t gfc_option;
>
> +#define _expand(m) m
>
> I think it would be better to avoid names like _expand, too generic
> name and starts with underscore, name it e.g. SET_BITFLAG_1 or something
> similar.  And it isn't mentioned in the ChangeLog.
I find "expand" a more helpful name than "set_bitflag_1" since it
describes what the macro does. However, I don't think it makes too
much of a difference so I'll follow your preference (but I'll use
SET_BITFLAG2 since then the definition line fits in 80 characters).

>
> @@ -62,14 +75,30 @@ set_dec_flags (int value)
>      }
>
> What about the
>       /* Allow legacy code without warnings.  */
>       gfc_option.allow_std |= GFC_STD_F95_OBS | GFC_STD_F95_DEL
>         | GFC_STD_GNU | GFC_STD_LEGACY;
>       gfc_option.warn_std &= ~(GFC_STD_LEGACY | GFC_STD_F95_DEL);
> that is done for value, shouldn't set_dec_flags remove those
> flags again?  Maybe not the allow_std ones, because those are set already by
> default, perhaps just the warn_std flags?
>
Sure. I wasn't convinced about this and how it might interplay with
-std= so I left it alone, but I suppose it makes sense to unsuppress
the warnings when disabling -fdec.

> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/array_temporaries_5.f90
> @@ -0,0 +1,20 @@
> +! { dg-do run }
> +! { dg-options "-fcheck-array-temporaries -fno-check-array-temporaries" }
> +!
> +! PR fortran/87919
> +!
> +! Ensure -fno-check-array-temporaries disables array temporary checking.
> +! Copied from array_temporaries_2.f90.
>
> For tests where you expect no errors and that are just copies of other
> testcases, perhaps
> include 'array_temporaries_2.f90'
> or similar instead?
>
Strictly speaking it's not an exact copy because it omits the final {
dg-output } check for the runtime warning, since the warning is
supposed to occur in array_temporaries_2.f90 but not in the new case
array_temporaries_5.f90. I assumed include would propagate the {
dg-output } check -- upon actually testing this, it appears that is
not the case. I find this misleading at a glance, but it works, so I
don't mind this with an extra comment line.

Attached is a new patch which incorporates your recommendations.
Here's the diff between the two, followed by the new changelog:

diff --git a/gcc/fortran/options.c b/gcc/fortran/options.c
index e703cad96fd..167621905bc 100644
--- a/gcc/fortran/options.c
+++ b/gcc/fortran/options.c
@@ -32,8 +32,6 @@ along with GCC; see the file COPYING3.  If not see

 gfc_option_t gfc_option;

-#define _expand(m) m
-
 #define SET_FLAG(flag, condition, on_value, off_value) \
   do \
     { \
@@ -43,8 +41,10 @@ gfc_option_t gfc_option;
        flag = (off_value); \
     } while (0)

+#define SET_BITFLAG2(m) m
+
 #define SET_BITFLAG(flag, condition, value) \
-  _expand (SET_FLAG (flag, condition, (flag | (value)), (flag & ~(value))))
+  SET_BITFLAG2 (SET_FLAG (flag, condition, (flag | (value)), (flag &
~(value))))


 /* Set flags that control warnings and errors for different
@@ -66,15 +66,17 @@ set_default_std_flags (void)
 static void
 set_dec_flags (int value)
 {
+  /* Allow legacy code without warnings.
+     Nb. We do not unset the allowed standards with value == 0 because
+     they are set by default in set_default_std_flags.  */
   if (value)
-    {
-      /* Allow legacy code without warnings.  */
-      gfc_option.allow_std |= GFC_STD_F95_OBS | GFC_STD_F95_DEL
-        | GFC_STD_GNU | GFC_STD_LEGACY;
-      gfc_option.warn_std &= ~(GFC_STD_LEGACY | GFC_STD_F95_DEL);
-    }
+    gfc_option.allow_std |= GFC_STD_F95_OBS | GFC_STD_F95_DEL
+      | GFC_STD_GNU | GFC_STD_LEGACY;
+
+  SET_BITFLAG (gfc_option.warn_std, !value, GFC_STD_LEGACY);
+  SET_BITFLAG (gfc_option.warn_std, !value, GFC_STD_F95_DEL);

-  /* Set other DEC compatibility extensions.  */
+  /* Set (or unset) other DEC compatibility extensions.  */
   SET_BITFLAG (flag_dollar_ok, value, value);
   SET_BITFLAG (flag_cray_pointer, value, value);
   SET_BITFLAG (flag_dec_structure, value, value);
@@ -855,3 +871,7 @@ gfc_get_option_string (void)
 }

 #undef SET_BITFLAG
+#undef SET_BITFLAG2
 #undef SET_FLAG
-#undef _expand

diff --git a/gcc/testsuite/gfortran.dg/array_temporaries_5.f90
b/gcc/testsuite/gfortran.dg/array_temporaries_5.f90
index 0070a935933..dd147ba38ed 100644
--- a/gcc/testsuite/gfortran.dg/array_temporaries_5.f90
+++ b/gcc/testsuite/gfortran.dg/array_temporaries_5.f90
@@ -4,17 +4,7 @@
 ! PR fortran/87919
 !
 ! Ensure -fno-check-array-temporaries disables array temporary checking.
-! Copied from array_temporaries_2.f90.
 !

-program test
-  implicit none
-  integer :: a(3,3)
-  call foo(a(:,1))  ! OK, no temporary created
-  call foo(a(1,:))  ! BAD, temporary var created
-contains
-  subroutine foo(x)
-    integer :: x(3)
-    x = 5
-  end subroutine foo
-end program test
+! Note that 'include' drops the dg-output check from the original test case.
+include 'array_temporaries_2.f90'



>>>>>> ChangeLog <<<<<<
PR fortran/87919

Fix handling -fno-* prefix for init-local-zero, check-array-temporaries and dec.

gcc/fortran/
        * options.c (SET_FLAG, SET_BITFLAG, SET_BITFLAG2): New macros.
        (set_dec_flags): Set/unset DEC and std flags according to value.
        (set_init_local_zero): New helper for -finit-local-zero flag group.
        (gfc_init_options): Fix disabling of init flags, array temporaries
        check, and dec flags when value is zero (from -fno-*).

gcc/testsuiste/
        * gfortran.dg/array_temporaries_5.f90: New test.
        * gfortran.dg/dec_bitwise_ops_3.f90: Ditto.
        * gfortran.dg/dec_d_lines_3.f: Ditto.
        * gfortran.dg/dec_exp_4.f90: Ditto.
        * gfortran.dg/dec_exp_5.f90: Ditto.
        * gfortran.dg/dec_io_7.f90: Ditto.
        * gfortran.dg/dec_structure_24.f: Ditto.
        * gfortran.dg/dec_structure_25.f: Ditto.
        * gfortran.dg/dec_structure_26.f: Ditto.
        * gfortran.dg/dec_structure_27.f: Ditto.
        * gfortran.dg/dec_type_print_3.f90: Ditto.
        * gfortran.dg/init_flag_20.f90: Ditto.

0002-PR-fortran-87919.patch (24K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: PR fortran/87919 patch for -fno-dec-structure

Fritz Reese
On Thu, Nov 8, 2018 at 12:54 PM Jakub Jelinek <[hidden email]> wrote:

>
> On Thu, Nov 08, 2018 at 12:09:33PM -0500, Fritz Reese wrote:
> > > What about the
> > >       /* Allow legacy code without warnings.  */
> > >       gfc_option.allow_std |= GFC_STD_F95_OBS | GFC_STD_F95_DEL
> > >         | GFC_STD_GNU | GFC_STD_LEGACY;
> > >       gfc_option.warn_std &= ~(GFC_STD_LEGACY | GFC_STD_F95_DEL);
> > > that is done for value, shouldn't set_dec_flags remove those
> > > flags again?  Maybe not the allow_std ones, because those are set already by
> > > default, perhaps just the warn_std flags?
> > >
> >
> > Sure. I wasn't convinced about this and how it might interplay with
> > -std= so I left it alone, but I suppose it makes sense to unsuppress
> > the warnings when disabling -fdec.
>
> Perhaps it might be better not to change the allow_std/warn_std flags
> during the option parsing, instead set or clear say flag_dec and
> only when option processing is being finalized (gfc_post_options)
> check if flag_dec is set and set those.  It would change behavior of
> -fdec -std=f2018 and similar though.  Not sure what users expect.
>
Actually, the gcc frontend appears to move -std= before the
language-specific options before f951 is even executed regardless of
its location compared to the -fdec flags. I don't know if this is a
bug or if it is by design -- the feeling I get is that the gcc
frontend processes it first since it is recognized before the flang
specific options. Therefore, greedily setting the standard options the
first time flag_dec appears means the standard information is lost and
I believe your suggestion is correct: the standard flags must be set
only once in gfc_post_options.

In fact the new testcase dec_bitwise_ops_3.f90 is a good test of this:
it uses -fdec -fno-dec -std=legacy to avoid warnings for XOR. With the
version posted previously, the -std=legacy is overwritten by -fno-dec
and warnings still appear. Here's what I'd change from the previous
patch to support this:

diff --git a/gcc/fortran/options.c b/gcc/fortran/options.c
index af89a5d2faf..b7f7360215c 100644
--- a/gcc/fortran/options.c
+++ b/gcc/fortran/options.c
@@ -66,16 +66,6 @@ set_default_std_flags (void)
 static void
 set_dec_flags (int value)
 {
-  /* Allow legacy code without warnings.
-     Nb. We do not unset the allowed standards with value == 0 because
-     they are set by default in set_default_std_flags.  */
-  if (value)
-    gfc_option.allow_std |= GFC_STD_F95_OBS | GFC_STD_F95_DEL
-      | GFC_STD_GNU | GFC_STD_LEGACY;
-
-  SET_BITFLAG (gfc_option.warn_std, !value, GFC_STD_LEGACY);
-  SET_BITFLAG (gfc_option.warn_std, !value, GFC_STD_F95_DEL);
-
   /* Set (or unset) other DEC compatibility extensions.  */
   SET_BITFLAG (flag_dollar_ok, value, value);
   SET_BITFLAG (flag_cray_pointer, value, value);
@@ -85,6 +75,24 @@ set_dec_flags (int value)
   SET_BITFLAG (flag_dec_math, value, value);
 }

+/* Finalize DEC flags.  */
+
+static void
+post_dec_flags (int value)
+{
+  /* Don't warn for legacy code if -fdec is given; however, setting -fno-dec
+     does not force these warnings.  We make one final determination on this
+     at the end because -std= is always set first; thus, we can avoid
+     clobbering the user's desired standard settings in gfc_handle_option
+     e.g. when -fdec and -fno-dec are both given.  */
+  if (value)
+    {
+      gfc_option.allow_std |= GFC_STD_F95_OBS | GFC_STD_F95_DEL
+       | GFC_STD_GNU | GFC_STD_LEGACY;
+      gfc_option.warn_std &= ~(GFC_STD_LEGACY | GFC_STD_F95_DEL);
+    }
+}
+
 /* Enable (or disable) -finit-local-zero.  */

 static void
@@ -248,6 +256,9 @@ gfc_post_options (const char **pfilename)
   char *source_path;
   int i;

+  /* Finalize DEC flags.  */
+  post_dec_flags (flag_dec);
+
   /* Excess precision other than "fast" requires front-end
      support.  */
   if (flag_excess_precision_cmdline == EXCESS_PRECISION_STANDARD)
@@

> Directives are only processed in the current file, so it doesn't really
> matter what the included file has as directives.  One could even have the
> included one be with expected dg-error lines and then include it in
> the ones that don't expect any.

Good to know, thanks! In that case, I like your suggestion of reducing
the test cases to includes. See new the newly attached patch for
updated cases.

> Anyway, that is all from me, I still don't want to stomp on Fortran
> maintainer's review (use my global reviewer's rights for that) and
> thus I'm deferring the review to them.  When committing, please make sure
> to include Mark's email in the ChangeLog next to yours to credit him.

Thanks for your comments. I think nobody will feel stomped on since
maintainers are sparse and busy. I will certainly make note of Mark's
contributions when committing.

Attached is the latest version, which builds and regtests cleanly on
x86_64-redhat-linux. OK for trunk, 7-branch, and 8-branch?

Fritz

From 1cae11a88b29fe521e0e6c6c7c1796a7adb34cad Mon Sep 17 00:00:00 2001
From: Fritz Reese <[hidden email]>
Date: Mon, 12 Nov 2018 13:57:25 -0500
Subject: [PATCH] PR fortran/87919

Fix handling -fno-* prefix for init-local-zero, check-array-temporaries and dec.

gcc/fortran/
    * options.c (SET_FLAG, SET_BITFLAG, SET_BITFLAG2): New macros.
    (set_dec_flags): Set/unset DEC and std flags according to value.
    (set_init_local_zero): New helper for -finit-local-zero flag group.
    (gfc_init_options): Fix disabling of init flags, array temporaries
    check, and dec flags when value is zero (from -fno-*).

gcc/testsuiste/
    * gfortran.dg/array_temporaries_5.f90: New test.
    * gfortran.dg/dec_bitwise_ops_3.f90: Ditto.
    * gfortran.dg/dec_d_lines_3.f: Ditto.
    * gfortran.dg/dec_exp_4.f90: Ditto.
    * gfortran.dg/dec_exp_5.f90: Ditto.
    * gfortran.dg/dec_io_7.f90: Ditto.
    * gfortran.dg/dec_structure_24.f90: Ditto.
    * gfortran.dg/dec_structure_25.f90: Ditto.
    * gfortran.dg/dec_structure_26.f90: Ditto.
    * gfortran.dg/dec_structure_27.f90: Ditto.
    * gfortran.dg/dec_type_print_3.f90: Ditto.
    * gfortran.dg/init_flag_20.f90: Ditto.
---
 gcc/fortran/options.c                             | 93 +++++++++++++++--------
 gcc/testsuite/gfortran.dg/array_temporaries_5.f90 | 10 +++
 gcc/testsuite/gfortran.dg/dec_bitwise_ops_3.f90   | 29 +++++++
 gcc/testsuite/gfortran.dg/dec_d_lines_3.f         | 14 ++++
 gcc/testsuite/gfortran.dg/dec_exp_4.f90           | 12 +++
 gcc/testsuite/gfortran.dg/dec_exp_5.f90           | 11 +++
 gcc/testsuite/gfortran.dg/dec_io_7.f90            | 20 +++++
 gcc/testsuite/gfortran.dg/dec_structure_24.f90    | 32 ++++++++
 gcc/testsuite/gfortran.dg/dec_structure_25.f90    | 11 +++
 gcc/testsuite/gfortran.dg/dec_structure_26.f90    | 34 +++++++++
 gcc/testsuite/gfortran.dg/dec_structure_27.f90    | 34 +++++++++
 gcc/testsuite/gfortran.dg/dec_type_print_3.f90    | 21 +++++
 gcc/testsuite/gfortran.dg/init_flag_20.f90        | 15 ++++
 13 files changed, 306 insertions(+), 30 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/array_temporaries_5.f90
 create mode 100644 gcc/testsuite/gfortran.dg/dec_bitwise_ops_3.f90
 create mode 100644 gcc/testsuite/gfortran.dg/dec_d_lines_3.f
 create mode 100644 gcc/testsuite/gfortran.dg/dec_exp_4.f90
 create mode 100644 gcc/testsuite/gfortran.dg/dec_exp_5.f90
 create mode 100644 gcc/testsuite/gfortran.dg/dec_io_7.f90
 create mode 100644 gcc/testsuite/gfortran.dg/dec_structure_24.f90
 create mode 100644 gcc/testsuite/gfortran.dg/dec_structure_25.f90
 create mode 100644 gcc/testsuite/gfortran.dg/dec_structure_26.f90
 create mode 100644 gcc/testsuite/gfortran.dg/dec_structure_27.f90
 create mode 100644 gcc/testsuite/gfortran.dg/dec_type_print_3.f90
 create mode 100644 gcc/testsuite/gfortran.dg/init_flag_20.f90

0003-PR-fortran-87919.patch (28K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: PR fortran/87919 patch for -fno-dec-structure

Jakub Jelinek
On Mon, Nov 12, 2018 at 03:28:47PM -0500, Fritz Reese wrote:
> Actually, the gcc frontend appears to move -std= before the
> language-specific options before f951 is even executed regardless of
> its location compared to the -fdec flags. I don't know if this is a

That is because:
#define F951_OPTIONS        "%(cc1_options) %{J*} \
                             %{!nostdinc:-fintrinsic-modules-path finclude%s}\
                             %{!fsyntax-only:%(invoke_as)}"
and
static const char *cc1_options =
"%{pg:%{fomit-frame-pointer:%e-pg and -fomit-frame-pointer are incompatible}}\
 %{!iplugindir*:%{fplugin*:%:find-plugindir()}}\
 %1 %{!Q:-quiet} %{!dumpbase:-dumpbase %B} %{d*} %{m*} %{aux-info*}\
 %{fcompare-debug-second:%:compare-debug-auxbase-opt(%b)} \
 %{!fcompare-debug-second:%{c|S:%{o*:-auxbase-strip %*}%{!o*:-auxbase %b}}}%{!c:%{!S:-auxbase %b}} \
 %{g*} %{O*} %{W*&pedantic*} %{w} %{std*&ansi&trigraphs}\
 %{v:-version} %{pg:-p} %{p} %{f*} %{undef}\

where %{std*&ansi&trigraphs} comes before %{f*}.
I guess let's not change that behavior.

> bug or if it is by design -- the feeling I get is that the gcc
> frontend processes it first since it is recognized before the flang
> specific options. Therefore, greedily setting the standard options the
> first time flag_dec appears means the standard information is lost and
> I believe your suggestion is correct: the standard flags must be set
> only once in gfc_post_options.
>
> In fact the new testcase dec_bitwise_ops_3.f90 is a good test of this:
> it uses -fdec -fno-dec -std=legacy to avoid warnings for XOR. With the
> version posted previously, the -std=legacy is overwritten by -fno-dec
> and warnings still appear. Here's what I'd change from the previous
> patch to support this:

LGTM.

> > Anyway, that is all from me, I still don't want to stomp on Fortran
> > maintainer's review (use my global reviewer's rights for that) and
> > thus I'm deferring the review to them.  When committing, please make sure
> > to include Mark's email in the ChangeLog next to yours to credit him.
>
> Thanks for your comments. I think nobody will feel stomped on since
> maintainers are sparse and busy. I will certainly make note of Mark's
> contributions when committing.

Ok, so I'll ack it for trunk now, but please give the other Fortran
maintainers one day to disagree before committing.
For the release branches, I'd wait two weeks or so before backporting it.

Thanks.

        Jakub