| Message ID | aa2fb52b-124a-2bcb-9559-f53c15f9184e@codethink.co.uk |
|---|---|
| State | New |
| Headers | show |
| Series | [Fortran] Fix Automatics in equivalence test cases was Re: Automatics in Equivalences failures | expand |
On Wed, Oct 02, 2019 at 02:36:55PM +0100, Mark Eggleston wrote: > Please find attached a patch to replace the test cases for "Automatic in > equivalence". > > ChangeLog: > > Mark Eggleston <mark.eggleston@codethink.com> > > * gfortran.dg/auto_in_equiv_1.f90: Deleted. > * gfortran.dg/auto_in_equiv_2.f90: Deleted. > * gfortran.dg/auto_in_equiv_3.f90: Deleted. > * gfortran.dg/automatics_in_equivalence_1.f90: New test. > * gfortran.dg/automatics_in_equivalence_2.f90: New test. > > OK to commit? > yes
On Wed, Oct 02, 2019 at 02:36:55PM +0100, Mark Eggleston wrote: > ChangeLog: > > Mark Eggleston <mark.eggleston@codethink.com> > > * gfortran.dg/auto_in_equiv_1.f90: Deleted. > * gfortran.dg/auto_in_equiv_2.f90: Deleted. > * gfortran.dg/auto_in_equiv_3.f90: Deleted. > * gfortran.dg/automatics_in_equivalence_1.f90: New test. > * gfortran.dg/automatics_in_equivalence_2.f90: New test. Why the so long testcase names? Just replacing the first old test with the new one would be IMHO better. > --- /dev/null > +++ b/gcc/testsuite/gfortran.dg/automatics_in_equivalence_2.f90 > @@ -0,0 +1,37 @@ > +! { dg-do run } > +! { dg-options "-fdec-static -frecursive -fno-automatic" } ... 1) if the test is the same, the only difference is in dg- directives, it is easier to just include the other test. 2) if -frecursive -fno-automatic is the same as -fno-automatic, then the test is not valid unless you use explicit recursive keyword, because then you recurse on something that shouldn't be called recursively, right? > +! { dg-warning "Flag '-fno-automatic' overwrites '-frecursive'" "warning" { target *-*-* } 0 } I think you want one runtime test (e.g. the one you wrote in automatics_in_equivalence_1.f90) and the rest just dg-do compile tests that will check the original or gimple dumps to verify what happened in addition to checking diagnostics (none) from the compilation, one testing the default, another -fno-automatic, but in both cases without -frecursive. Jakub
On 04/10/2019 11:39, Jakub Jelinek wrote: > On Wed, Oct 02, 2019 at 03:31:53PM +0100, Mark Eggleston wrote: >> It was there because the code base has -fno-automatic for procedures are >> that aren't recursive and the -frecursive was used because the recursive >> routines don't have the recursive keyword. (Legacy issues...). It is to >> ensure that the use of -fno-automatic does not affect local variables in >> recursive routine. > Ah, seems while -frecursive -fno-automatic emits a warning that can be read > that as if -frecursive is ignored, it just forces all (unless explicitly automatic) > variables to be saved, but still allows recursion. One just has to manually > arrange for the variables that shouldn't be saved to be automatic. > So, I'm not against that test. >> If a procedure is marked with the keyword recursive all its local variables >> are always automatic by default. If a procedure is not marked with the >> keyword recursive its variables are not automatic when -fno-automatic is >> used unless they have the automatic attribute specified directly or acquired >> via the equivalence statement, recursion can still be effected using >> -frecursive provided all the variables used by the recursion are automatic. >> >>>> +! { dg-warning "Flag '-fno-automatic' overwrites '-frecursive'" "warning" { target *-*-* } 0 } >>> I think you want one runtime test (e.g. the one you wrote in >>> automatics_in_equivalence_1.f90) >> The errors checked there only check that automatic can be used that's >> already covered by dec_static_3.f90 and automatic can't be used in >> equivalence with the use of -fdec-static. >>> and the rest just dg-do compile tests that >>> will check the original or gimple dumps to verify what happened in addition >>> to checking diagnostics (none) from the compilation, one testing the >>> default, another -fno-automatic, but in both cases without -frecursive. >> Not sure what you mean, here. Are these already handled by the tests for >> -fdec-static? > I meant add a couple of new tests. One like: > ! { dg-do compile } > ! { dg-options "-fdump-tree-original" } > ! { dg-final { scan-tree-dump "static union" "original" } } > > subroutine foo > integer, save :: a, b > equivalence (a, b) > a = 5 > if (b.ne.5) stop 1 > end subroutine > > which verifies that the equivalence is saved in that case. Another one > like: > ! { dg-do compile } > ! { dg-options "-fdec-static -fdump-tree-original -fno-automatic" } > ! { dg-final { scan-tree-dump-not "static union" "original" } } > > subroutine foo > integer, automatic :: a > integer :: b > equivalence (a, b) > a = 5 > if (b.ne.5) stop 1 > end subroutine > > another one perhaps with swapped ", automatic" between a and b. > Another two without the -fno-automatic? > > Jakub Please find attached patch for additional test cases. Are these sufficient? If so, OK to commit? Change log: Mark Eggleston <mark.eggleston@codethink.com> * gfortran.dg/auto_in_equiv_3.f90: New test. * gfortran.dg/auto_in_equiv_4.f90: New test. * gfortran.dg/auto_in_equiv_5.f90: New test. * gfortran.dg/auto_in_equiv_6.f90: New test. * gfortran.dg/auto_in_equiv_7.f90: New test.
On Thu, Oct 17, 2019 at 08:43:29AM +0100, Mark Eggleston wrote: > Please find attached patch for additional test cases. Are these sufficient? > If so, OK to commit? > > Change log: > > Mark Eggleston <mark.eggleston@codethink.com> > > * gfortran.dg/auto_in_equiv_3.f90: New test. > * gfortran.dg/auto_in_equiv_4.f90: New test. > * gfortran.dg/auto_in_equiv_5.f90: New test. > * gfortran.dg/auto_in_equiv_6.f90: New test. > * gfortran.dg/auto_in_equiv_7.f90: New test. Ok, with small nits: > --- /dev/null > +++ b/gcc/testsuite/gfortran.dg/auto_in_equiv_4.f90 > @@ -0,0 +1,18 @@ > +! { dg-do compile } > +! { dg-options "-fdec-static -fno-automatic -fdump-tree-original" } > +! > +! Neither of the local variable have the auotmatic attribute so they s/auotmatic/automatic/ > --- /dev/null > +++ b/gcc/testsuite/gfortran.dg/auto_in_equiv_5.f90 > @@ -0,0 +1,18 @@ > +! { dg-do compile } > +! { dg-options "-fdump-tree-original" } > +! > +! Neither of the local variable have the auotmatic attribute so they > +! not be allocated on the stack. Likewise. > --- /dev/null > +++ b/gcc/testsuite/gfortran.dg/auto_in_equiv_6.f90 > @@ -0,0 +1,18 @@ > +! { dg-do compile } > +! { dg-options "-fdec-static -fdump-tree-original" } > +! > +! Neither of the local variable have the auotmatic attribute so they > +! not be allocated on the stack. Likewise. Jakub
From e461900f602b48e8d13402fca46d34506308785f Mon Sep 17 00:00:00 2001 From: Mark Eggleston <markeggleston@codethink.com> Date: Wed, 2 Oct 2019 10:23:49 +0100 Subject: [PATCH] Replace test cases for Automatics in equivalence --- gcc/testsuite/gfortran.dg/auto_in_equiv_1.f90 | 36 ------------- gcc/testsuite/gfortran.dg/auto_in_equiv_2.f90 | 38 ------------- gcc/testsuite/gfortran.dg/auto_in_equiv_3.f90 | 63 ---------------------- .../gfortran.dg/automatics_in_equivalence_1.f90 | 35 ++++++++++++ .../gfortran.dg/automatics_in_equivalence_2.f90 | 37 +++++++++++++ 5 files changed, 72 insertions(+), 137 deletions(-) delete mode 100644 gcc/testsuite/gfortran.dg/auto_in_equiv_1.f90 delete mode 100644 gcc/testsuite/gfortran.dg/auto_in_equiv_2.f90 delete mode 100644 gcc/testsuite/gfortran.dg/auto_in_equiv_3.f90 create mode 100644 gcc/testsuite/gfortran.dg/automatics_in_equivalence_1.f90 create mode 100644 gcc/testsuite/gfortran.dg/automatics_in_equivalence_2.f90 diff --git a/gcc/testsuite/gfortran.dg/auto_in_equiv_1.f90 b/gcc/testsuite/gfortran.dg/auto_in_equiv_1.f90 deleted file mode 100644 index bf6e0c68a57..00000000000 --- a/gcc/testsuite/gfortran.dg/auto_in_equiv_1.f90 +++ /dev/null @@ -1,36 +0,0 @@ -! { dg-do compile } - -! Contributed by Mark Eggleston <mark.eggleston@codethink.com> -program test - call suba(0) - call subb(0) - call suba(1) - -contains - subroutine suba(option) - integer, intent(in) :: option - integer, automatic :: a ! { dg-error "AUTOMATIC at \\(1\\) is a DEC extension" } - integer :: b - integer :: c - equivalence (a, b) - if (option.eq.0) then - ! initialise a and c - a = 9 - c = 99 - if (a.ne.b) stop 1 - if (loc(a).ne.loc(b)) stop 2 - else - ! a should've been overwritten - if (a.eq.9) stop 3 - end if - end subroutine suba - - subroutine subb(dummy) - integer, intent(in) :: dummy - integer, automatic :: x ! { dg-error "AUTOMATIC at \\(1\\) is a DEC extension" } - integer :: y - x = 77 - y = 7 - end subroutine subb - -end program test diff --git a/gcc/testsuite/gfortran.dg/auto_in_equiv_2.f90 b/gcc/testsuite/gfortran.dg/auto_in_equiv_2.f90 deleted file mode 100644 index e40c0f15f3e..00000000000 --- a/gcc/testsuite/gfortran.dg/auto_in_equiv_2.f90 +++ /dev/null @@ -1,38 +0,0 @@ -! { dg-do run } -! { dg-options "-fdec-static" } - -! Contributed by Mark Eggleston <mark.eggleston@codethink.com> - -program test - call suba(0) - call subb(0) - call suba(1) - -contains - subroutine suba(option) - integer, intent(in) :: option - integer, automatic :: a - integer :: b - integer :: c - equivalence (a, b) - if (option.eq.0) then - ! initialise a and c - a = 9 - c = 99 - if (a.ne.b) stop 1 - if (loc(a).ne.loc(b)) stop 2 - else - ! a should've been overwritten - if (a.eq.9) stop 3 - end if - end subroutine suba - - subroutine subb(dummy) - integer, intent(in) :: dummy - integer, automatic :: x - integer :: y - x = 77 - y = 7 - end subroutine subb - -end program test diff --git a/gcc/testsuite/gfortran.dg/auto_in_equiv_3.f90 b/gcc/testsuite/gfortran.dg/auto_in_equiv_3.f90 deleted file mode 100644 index 57c384d1772..00000000000 --- a/gcc/testsuite/gfortran.dg/auto_in_equiv_3.f90 +++ /dev/null @@ -1,63 +0,0 @@ -! { dg-do run } -! { dg-options "-fdec-static -fno-automatic" } - -! Contributed by Mark Eggleston <mark.eggleston@codethink.com> - -! Storage is NOT on the static unless explicitly specified using the -! DEC extension "automatic". The address of the first local variable -! is used to determine that storage for the automatic local variable -! is different to that of a local variable with no attributes. The -! contents of the local variable in suba should be overwritten by the -! call to subb. -! -program test - integer :: dummy - integer, parameter :: address = kind(loc(dummy)) - integer(address) :: ad1 - integer(address) :: ad2 - integer(address) :: ad3 - logical :: ok - - call suba(0, ad1) - call subb(0, ad2) - call suba(1, ad1) - call subc(0, ad3) - ok = (ad1.eq.ad3).and.(ad1.ne.ad2) - if (.not.ok) stop 4 - -contains - subroutine suba(option, addr) - integer, intent(in) :: option - integer(address), intent(out) :: addr - integer, automatic :: a - integer :: b - equivalence (a, b) - addr = loc(a) - if (option.eq.0) then - ! initialise a and c - a = 9 - if (a.ne.b) stop 1 - if (loc(a).ne.loc(b)) stop 2 - else - ! a should've been overwritten - if (a.eq.9) stop 3 - end if - end subroutine suba - - subroutine subb(dummy, addr) - integer, intent(in) :: dummy - integer(address), intent(out) :: addr - integer :: x - addr = loc(x) - x = 77 - end subroutine subb - - subroutine subc(dummy, addr) - integer, intent(in) :: dummy - integer(address), intent(out) :: addr - integer, automatic :: y - addr = loc(y) - y = 77 - end subroutine subc - -end program test diff --git a/gcc/testsuite/gfortran.dg/automatics_in_equivalence_1.f90 b/gcc/testsuite/gfortran.dg/automatics_in_equivalence_1.f90 new file mode 100644 index 00000000000..2791675af1b --- /dev/null +++ b/gcc/testsuite/gfortran.dg/automatics_in_equivalence_1.f90 @@ -0,0 +1,35 @@ +! { dg-do run } +! { dg-options "-fdec-static -frecursive" } + +! Contributed by Mark Eggleston <mark.eggleston@codethink.com> +! +! Check automatic variables can be used in equivalence statements. +! Any other variables that do not explicitly have the automatic +! attribute will be given the automatic attribute. +! +! Check that variables are on the stack by incorporating the +! equivalence in a recursive function. +! +program test + integer :: f + + f = factorial(5) + if (f.ne.120) stop 2 + +contains + function factorial(n) result(f) + integer :: f + integer, intent(in) :: n + integer, automatic :: a + integer :: b + equivalence (a,b) + + if (loc(a).ne.loc(b)) stop 1 + b = n + if (a.eq.1) then + f = 1 + else + f = a * factorial(b-1) + end if + end function +end program test diff --git a/gcc/testsuite/gfortran.dg/automatics_in_equivalence_2.f90 b/gcc/testsuite/gfortran.dg/automatics_in_equivalence_2.f90 new file mode 100644 index 00000000000..626db5835b8 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/automatics_in_equivalence_2.f90 @@ -0,0 +1,37 @@ +! { dg-do run } +! { dg-options "-fdec-static -frecursive -fno-automatic" } + +! Contributed by Mark Eggleston <mark.eggleston@codethink.com> +! +! Check automatic variables can be used in equivalence statements. +! Any other variables that do not explicitly have the automatic +! attribute will be given the automatic attribute. +! +! Check that variables are on the stack by incorporating the +! equivalence in a recursive function. +! +program test + integer :: f + + f = factorial(5) + if (f.ne.120) stop 2 + +contains + function factorial(n) result(f) + integer :: f + integer, intent(in) :: n + integer, automatic :: a + integer :: b + equivalence (a,b) + + if (loc(a).ne.loc(b)) stop 1 + b = n + if (a.eq.1) then + f = 1 + else + f = a * factorial(b-1) + end if + end function +end program test + +! { dg-warning "Flag '-fno-automatic' overwrites '-frecursive'" "warning" { target *-*-* } 0 } -- 2.11.0