Bug 50288 - FAIL: gfortran.dg/class_45b.f03
Summary: FAIL: gfortran.dg/class_45b.f03
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.7.0
: P3 normal
Target Milestone: ---
Assignee: janus
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-04 18:48 UTC by H.J. Lu
Modified: 2011-09-07 13:33 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2011-09-04 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description H.J. Lu 2011-09-04 18:48:42 UTC
On Linux/x86, revision 178509:

http://gcc.gnu.org/ml/gcc-cvs/2011-09/msg00120.html

gave:

FAIL: gfortran.dg/class_45b.f03  -O1  (test for excess errors)
FAIL: gfortran.dg/class_45b.f03  -O2  (test for excess errors)
FAIL: gfortran.dg/class_45b.f03  -O3 -fomit-frame-pointer  (test for excess errors)
FAIL: gfortran.dg/class_45b.f03  -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions  (test for excess errors)
FAIL: gfortran.dg/class_45b.f03  -O3 -fomit-frame-pointer -funroll-loops  (test for excess errors)
FAIL: gfortran.dg/class_45b.f03  -O3 -g  (test for excess errors)
FAIL: gfortran.dg/class_45b.f03  -Os  (test for excess errors)
Comment 1 Dominique d'Humieres 2011-09-04 19:18:00 UTC
I think I understand what's happening, although I don't know how to fix it:

(1) gfortran.dg/class_45a.f03 is compiled and generate g_nodes.mod,
(2) gfortran.dg/class_45b.f03 is compiled and run with -O0,
(3) g_nodes.mod is cleaned due to "! { dg-final { cleanup-modules "G_Nodes" } }"
(4) gfortran.dg/class_45b.f03 is proceeded with -O1 but g_nodes.mod is no longer there, hence the failure:

Fatal Error: Can't open module file 'g_nodes.mod' for reading at (1): No such file or directory^M
FAIL: gfortran.dg/class_45b.f03  -O1  (test for excess errors)
Excess errors:
/opt/gcc/work/gcc/testsuite/gfortran.dg/class_45b.f03:9:6: Fatal Error: Can't open module file 'g_nodes.mod' for reading at (1): No such file or direc
tory
Comment 2 Dominique d'Humieres 2011-09-04 19:18:48 UTC
Indeed I confirm!-)
Comment 3 Tobias Burnus 2011-09-04 19:54:56 UTC
An evil trick would be the following, which causes a run once:

--- a/gcc/testsuite/gfortran.dg/class_45b.f03
+++ b/gcc/testsuite/gfortran.dg/class_45b.f03
@@ -1 +1 @@
-! { dg-do run }
+! { dg-do  run }
Comment 4 Dominique d'Humieres 2011-09-04 20:35:36 UTC
> An evil trick would be the following, which causes a run once: ...

It works, but I think if this trick is used, it should be documented as in gcc/testsuite/gfortran.dg/cray_pointers_2.f90:

! Using two spaces between dg-do and run is a hack to keep gfortran-dg-runtest
! from cycling through optimization options for this expensive test.

Another working possibility is to remove the line

! { dg-final { cleanup-modules "G_Nodes" } }

from gfortran.dg/class_45b.f03 and add an empty "cleaning" test:

cat ./gcc/testsuite/gfortran.dg/class_45c.f03

! { dg-do run }
!
! PR 50227: [4.7 Regression] [OOP] ICE-on-valid with allocatable class variable
!
! Contributed by Andrew Benson <abenson@caltech.edu>

program Test
end program Test

! { dg-final { cleanup-modules "G_Nodes" } }

Note that it will be impossible to test gfortran.dg/class_45b.f03 alone.
Comment 5 Tobias Burnus 2011-09-04 21:19:31 UTC
I think one problem is that
  ! { dg-additional-sources class_45a.f03 }
compiles the additional source *after* the main file. That's not trivially fixable as the library part of $options needs to come after the files - thus, one would need a new global variable, which is then used before $file in the compiler execution.


(In reply to comment #4)
> > An evil trick would be the following, which causes a run once: ...

A better way is the following patch, which adds "dg-do run-once", which should then also be applied to cray_pointers_2.f90. I think a "dg-do run-once" is also useful to tests of libgfortran for which running a test case multiple times is just a waste for time. One still should add a comment in class_45{a,b}.f90 to denote this nonobvious file dependency.


--- a/gcc/testsuite/lib/gfortran-dg.exp
+++ b/gcc/testsuite/lib/gfortran-dg.exp
@@ -104,7 +104,9 @@ proc gfortran-dg-runtest { testcases default-extra-flags } {
 
        # look if this is dg-do-run test, in which case
        # we cycle through the option list, otherwise we don't
-       if [expr [search_for $test "dg-do run"]] {
+       if [expr [search_for $test "dg-do +run-once"]] {
+           set option_list [list { -O } ]
+       } elseif [expr [search_for $test "dg-do +run"]] {
            set option_list $torture_with_loops
        } else {
            set option_list [list { -O } ]
Comment 6 janus 2011-09-05 12:26:14 UTC
Sorry for the breakage, guys. Of course I *did* check the test case before committing, but for some reason the failures did not occur on my machine (I have no idea why).



(In reply to comment #5)
> A better way is the following patch, which adds "dg-do run-once", which should
> then also be applied to cray_pointers_2.f90.

That sounds reasonable (much better than the evil "double blank" trick). Ok with me.
Comment 7 Tobias Burnus 2011-09-05 12:30:40 UTC
(In reply to comment #6)
> That sounds reasonable (much better than the evil "double blank" trick). Ok
> with me.

Can you package it? (This patch, modification to class_45b.f03 w/ comment, and to cray_pointers_2.f90.) As test-suite patch, it probably should probably be rubber stamped by a test-suite maintained such as Rainer Orth.
Comment 8 janus 2011-09-05 12:49:35 UTC
(In reply to comment #7)
> (In reply to comment #6)
> > That sounds reasonable (much better than the evil "double blank" trick). Ok
> > with me.
> 
> Can you package it? (This patch, modification to class_45b.f03 w/ comment, and
> to cray_pointers_2.f90.) As test-suite patch, it probably should probably be
> rubber stamped by a test-suite maintained such as Rainer Orth.

Sure, I'll take care of it. Thanks for your help in fixing this so quickly.
Comment 9 janus 2011-09-05 13:52:20 UTC
(In reply to comment #5)
> A better way is the following patch, which adds "dg-do run-once", which should
> then also be applied to cray_pointers_2.f90.

I think one should also do this for class_4{a-d}.f03, where Tobias apparently worked around the problem by adding an extra file (just to do the cleanup).

Also, I still don't understand why I did not see the problem. Maybe this could be due to different dejagnu versions (mine is 1.4.4)?
Comment 10 Tobias Burnus 2011-09-05 15:19:17 UTC
(In reply to comment #9)
> I think one should also do this for class_4{a-d}.f03, where Tobias apparently
> worked around the problem by adding an extra file (just to do the cleanup).

Thanks for the honor but that was Paul's patch (cf. PR 40440). The big difference is: That's a "dg-do compile" patch. (I might have added a mod cleanup to one of the files.)

And no, it does not make sense there. The issue only occurs for "dg-do run" - as after each compile/run, the cleanup is done. But with dg-do compile as with dg-do run-once, one has only a single run. Thus, that the .mod file from the previous test is removed, does not matter.

> Also, I still don't understand why I did not see the problem. Maybe this could
> be due to different dejagnu versions (mine is 1.4.4)?

No idea, but I have also 1.4.4 installed - and I do see - as expected - the failure.
Comment 11 janus 2011-09-05 15:59:45 UTC
(In reply to comment #10)
> (In reply to comment #9)
> > I think one should also do this for class_4{a-d}.f03, where Tobias apparently
> > worked around the problem by adding an extra file (just to do the cleanup).
> 
> Thanks for the honor but that was Paul's patch (cf. PR 40440).

I did not check which commit it came from, I just saw your name in the file header ;)


> The big difference is: That's a "dg-do compile" patch.

No. class_4c.f03 clearly has "dg-do run". class_4d.f03 only has "dg-do compile", but as commented, it is only meant to do the cleanup.

I would assume this extra file for the cleanup was added to work around exactly the problem we're discussing here.
Comment 12 janus 2011-09-05 19:33:26 UTC
(In reply to comment #5)
> --- a/gcc/testsuite/lib/gfortran-dg.exp
> +++ b/gcc/testsuite/lib/gfortran-dg.exp
> @@ -104,7 +104,9 @@ proc gfortran-dg-runtest { testcases default-extra-flags }
> {
> 
>         # look if this is dg-do-run test, in which case
>         # we cycle through the option list, otherwise we don't
> -       if [expr [search_for $test "dg-do run"]] {
> +       if [expr [search_for $test "dg-do +run-once"]] {
> +           set option_list [list { -O } ]
> +       } elseif [expr [search_for $test "dg-do +run"]] {
>             set option_list $torture_with_loops
>         } else {
>             set option_list [list { -O } ]


Unfortunately, the patch in comment #5 does not really work for me. When using "dg-do run-once" in class_45b.f03, I get:

ERROR: gfortran.dg/class_45b.f03  -O : 1: syntax error for " dg-do 1 run-once "

Apparently it is not possible to add new 'dg-do' keywords, cf. e.g.:

http://gcc.gnu.org/ml/gcc-patches/2011-01/msg02347.html
Comment 13 janus 2011-09-05 19:52:53 UTC
Ok, since class_45{a,b} is not really a run-time test, I think the best solution would be to just convert it to "dg-do link":


Index: class_45b.f03
===================================================================
--- class_45b.f03       (revision 178551)
+++ class_45b.f03       (working copy)
@@ -1,4 +1,4 @@
-! { dg-do run }
+! { dg-do link }
 ! { dg-additional-sources class_45a.f03 }
 !
 ! PR 50227: [4.7 Regression] [OOP] ICE-on-valid with allocatable class variable


I assume this should get rid of the errors. Can anyone confirm that? (Since I do not see the failures, I can not check it myself.)

One could do the same with class_4{a,b,c}, since this one also does not contain any actual run-time checking (but was committed to check for a compile-time error).

If I get a confirmation the the above patch fixes the errors, I will commit it as obvious.
Comment 14 Tobias Burnus 2011-09-06 06:49:11 UTC
Sorry for missing the issue with "run-once"; I thought I had tested it, but seemingly I haven't done so.

(In reply to comment #13)
> Ok, since class_45{a,b} is not really a run-time test, I think the best
> solution would be to just convert it to "dg-do link":

Good idea; the patch is preapproved, but I have not yet tested it.
Comment 15 janus 2011-09-06 12:19:13 UTC
(In reply to comment #14)
> > Ok, since class_45{a,b} is not really a run-time test, I think the best
> > solution would be to just convert it to "dg-do link":
> 
> Good idea; the patch is preapproved, but I have not yet tested it.

Thanks. Could anyone please test it? Unfortunately I cannot do it myself. One doesn't even have to run the whole testsuite, just class_45 with the patch from comment #13 would be enough.
Comment 16 Tobias Burnus 2011-09-07 07:56:11 UTC
(In reply to comment #13)
> +++ class_45b.f03       (working copy)
> @@ -1,4 +1,4 @@
> -! { dg-do run }
> +! { dg-do link }

With that change, I get:
... gcc-build/gcc> make check-gfortran RUNTESTFLAGS='dg.exp=class_45?.f03'
[...]
                === gfortran Summary ===
# of expected passes            2

Thus, the patch seems to work.
Comment 17 janus 2011-09-07 10:56:49 UTC
(In reply to comment #16)
> Thus, the patch seems to work.

Ok, thanks for checking. I'll commit as obvious the change to "dg-do link" for class_45 and class_4.
Comment 18 janus 2011-09-07 13:31:07 UTC
Author: janus
Date: Wed Sep  7 13:31:04 2011
New Revision: 178635

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=178635
Log:
2011-09-07  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/50288
	* gfortran.dg/class_4c.f03: Modified ("dg-do link" and "dg-final").
	* gfortran.dg/class_4d.f03: Deleted.
	* gfortran.dg/class_45b.f03: Modififed ("dg-do link").

Removed:
    trunk/gcc/testsuite/gfortran.dg/class_4d.f03
Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/class_45b.f03
    trunk/gcc/testsuite/gfortran.dg/class_4c.f03
Comment 19 janus 2011-09-07 13:33:05 UTC
Fixed with r178635. Closing.