Bug 28914 - Code inside loop hangs; outside loop runs normally; runs OK on other compilers
Summary: Code inside loop hangs; outside loop runs normally; runs OK on other compilers
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.0.2
: P3 normal
Target Milestone: 4.2.0
Assignee: Jerry DeLisle
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-08-31 01:45 UTC by Ed Kornkven
Modified: 2006-09-10 14:42 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2006-09-10 04:18:51


Attachments
A provisional fix for the problem (615 bytes, patch)
2006-09-01 13:48 UTC, Paul Thomas
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ed Kornkven 2006-08-31 01:45:36 UTC
!
! Un-comment the DO and ENDDO and the compiled program
! hangs after the first output line.
!
! Compiled with: gfortran junk.f90
! where: gfortran --version gives
!   GNU Fortran 95 (GCC 4.0.2 20051125 (Red Hat 4.0.2-8))
! Running on Fedora Core 4, Opteron
!
! E. Kornkven, kornkven@arsc.edu
!
      program junk
      implicit none
      integer n, i
      parameter (n = 1000000)
      double precision a(n), b(n), c(n), summation

      print*, '                perform some computation ... '
      summation = 0.0
      do i = 1, 1
         a = (/ (i, i = 1, n) /)
         a = sqrt(a)
         b = 1.0 / a
         c = b - a
         summation = SUM(a)
      enddo

      a(n/2) = 13.0
      print*, "Is the middle value 13.0? ", a(n/2-1), a(n/2), a(n/2+1)
      print*, "Summation = ", summation

      end program junk
Comment 1 kargls 2006-08-31 02:06:06 UTC
Upgrade gfortran to at least 4.1.1.  The code works
fine with gfortran 4.2.
Comment 2 kargls 2006-08-31 02:47:50 UTC
Well, you do need to upgrade your compiler, but there appears to be
a bug :(

If n <= 65000, the program works fine.  For larger n, the combination
of 
      do i = 1, 1
         a = (/ (i, i = 1, n) /)
      end do
 
i as the do-loop index and i as the implied-do-loop index is
causing problems.  Normally, one cannot alter the do-loop 
index within the body of the loop.  ISTR from the F95 standard,
that the i in the implied-do-loop has the scope of only the
implied-do-loop.  Thus, the above code is legal.

Why there is a change in behavior at i = 65000 (or so), I have
no idea.

Comment 3 Jerry DeLisle 2006-08-31 05:28:45 UTC
Setting the parameter n=65535, the program appears to execute correctly.  However, the pr28914.f90.003t.original file is 706800 bytes long and embedded with a very large static declaration of the array.  As if it has been inlined.

With n=66536, the dump file is 4436 bytes long and has code to initialize the array with a loop.  That looping code appears broken.
Comment 4 Paul Thomas 2006-09-01 13:45:29 UTC
(In reply to comment #3)
> Setting the parameter n=65535, the program appears to execute correctly. 
> However, the pr28914.f90.003t.original file is 706800 bytes long and embedded
> with a very large static declaration of the array.  As if it has been inlined.
> With n=66536, the dump file is 4436 bytes long and has code to initialize the
> array with a loop.  That looping code appears broken.

The looping code, of itself, is not broken.  If one of the 'i's, say in the implied do-loop, is changed to 'j', the code runs to completion.  The problem is that the implied do-loop counter uses a variable declaration and so a symbol.  This clashes with the variable i.

Since do-loops can run with +ve or -ve steps, the end condition is enforced through:

            L.1:;
            D.931 = i == 1;
            i = i + 1;
            if (D.931) goto L.2; else (void) 0;

ie. with an equality.  The implied do-loop sets this larger than 1 so the loop never stops.

For n < 65536, the loop is simplified out of existence and the large static array makes its appearance.  This seems to be an undesirable consequence of treating array initializers and array constructors with the same limit.

I would:

(i) Change the size limit for simplification of array constructors; and
(ii) Store the current value of the loop counter in a temporary and restore it after the array constructor has done its thing (patch follows).

Paul
Comment 5 Paul Thomas 2006-09-01 13:48:57 UTC
Created attachment 12168 [details]
A provisional fix for the problem

This is not regtested.

Also, I know that there is a better way to detect a declared variable; however, I am up to my eyeballs getting TR15541 out of the door... wait a few days, I will sort this one.

Paul
Comment 6 Jerry DeLisle 2006-09-02 19:24:57 UTC
I tested the provisional patch on i686-linux-pc-gnu.  

Had to set tmp_loopvar = NULL when declared to avoid warning message on possibly uninitialized variable.

Fixes the test case in this PR.  Regression testsd fine.
Comment 7 Jerry DeLisle 2006-09-03 03:44:47 UTC
Just an added note.  Compile time for large values of n is very long.  Many seconds.  For n - 20000000

$ time gfc pr28914.f90

real    1m5.009s
user    1m3.896s
sys     0m0.048s

This is on 3.2 gigahertz machine.  The resulting executable is about 10kbytes in size.
Comment 8 Paul Thomas 2006-09-04 12:08:06 UTC
Even simpler is:

trans-array.c(gfc_trans_array_constructor_value)

replace 	  loopvar = se.expr;
by                loopvar = gfc_evaluate_now (se.expr, pblock);

gfc_expand_constructor is called from resolve_expr and from three places in expr.c.  As far as I can tell, non-initialization expressions only try the expansion from resolve_expr.  If I flag the calls to distinguish them and limit the maximum number of expanded elements to 10, say, in gfc_expand_constructor, the code reflects this but the compilation time does not.  Something, somewhere is doing a temporary expansion of the constructor, which is taking all the time.

I have to leave this again but I will get to the bottom of it.

Paul
Comment 9 Jerry DeLisle 2006-09-04 19:33:48 UTC
The proposed change in comment #8 appears to give several testsuite failures.
Comment 10 Paul Thomas 2006-09-05 19:38:14 UTC
(In reply to comment #9)
> The proposed change in comment #8 appears to give several testsuite failures.
> 
Yes, I know; see the email that I sent you today.  The original patch on #5 works fine and is nearly the right solution.

Paul
Comment 11 Jerry DeLisle 2006-09-10 04:18:51 UTC
Apatch for this bug has been submitted to the fortran list for approval.
Comment 12 Jerry DeLisle 2006-09-10 04:53:28 UTC
Subject: Bug 28914

Author: jvdelisle
Date: Sun Sep 10 04:53:18 2006
New Revision: 116808

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=116808
Log:
2006-09-09  Paul Thomas <pault@gcc.gnu.org>

	PR fortran/28914
	* trans-array.c (gfc_trans_array_constructor_value): Create a temporary
	loop variable to hold the current loop variable in case it is modified
	by the array constructor.

Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-array.c

Comment 13 Jerry DeLisle 2006-09-10 04:58:37 UTC
Subject: Bug 28914

Author: jvdelisle
Date: Sun Sep 10 04:58:29 2006
New Revision: 116809

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=116809
Log:
2006-09-09  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR fortran/28914
	* gfortran.dg/actual_array_constructor_3.f90: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/actual_array_constructor_3.f90
Modified:
    trunk/gcc/testsuite/ChangeLog

Comment 14 Jerry DeLisle 2006-09-10 05:07:58 UTC
Fixed on 4.2 only, Follow PR20923 for long compile time issues.