[PATCH] Fix gfortran.dg/forall_1.f90 (take 2)

[Jakub Jelinek <jakub at redhat dot com>]

On Tue, Jun 07, 2005 at 10:08:01PM +0200, Tobias Schl?ter wrote:
> Jakub Jelinek wrote:
> > Of course we only want to increment it in the innermost loop which
> > contains the load of the mask.  Here is a fix, tested with no check-f95
> > failures on x86_64-linux.  Ok for HEAD and 4.0.2?
> > 
> > Alternatively, the increment could be moved to gfc_trans_nested_forall_loop
> > and body passed as stmtblock_t instead of tree.  This change is shorter
> > though...
> 
> The patch is ok for both provided you add more tests, the testcase I added is
> not really sufficient, because it doesn't have multi-dimensional masks or
> non-trivial dependencies between the mask and the assignments.

Gotta stop somewhere.  Clearly there are more bugs in the forall/where
handling lurking.

The problem with the (earlier) forall_7.f90 testcase was that if
LHS in the nested forall did not depend on RHS, only one of the 2
maskindexes was cleared between the setup of the mask temp arrays
and their use.  There are 10 calls to gfc_trans_nested_forall_loop
in trans-stmt.c which set mask_flag to 1, 8 of them were clearing
all maskindexes for nested forall statements, but the remaining
2 were just clearing the innermost one.
Fixed by moving the clearing into gfc_trans_nested_forall_loop,
so that it is not duplicated 10 times and done right in all cases.
I then enhanced the forall_7.f90 testcase and further bug showed up
- it is not enough to clear the maskindexes before the loops,
we need to clear it before each set of loops corresponding to
one nested FORALL stmt, otherwise we are walking over uninitialized
memory again.  E.g. in forall_2.f90 testcase we got around this by pure
luck (the outer mask was .true. only in one case).

The patch below fixes the forall_7.f90 testcase included in the patch
(and incorporates the patch from yesterday as well).
Ok for HEAD/4.0?

Now, forall_8.f90 and forall_9.f90 tests attached separately still fail,
but I must work on other stuff as well now, so if anyone wants to
continue...  forall_9.f90 is forall_7.f90, just that it uses any ()
in the mask expr and causes ICE.  forall_8.f90 compiles, but aborts
at runtime.  From quick skimming of the gimple dump the bug seems to be
that unlike forall_7.f90 (and all other current testcases), some inner
masks in forall_8.f90 are dependent on the control variables of (some of
the) outer forall statements.  But gfortran does not take that into account
and precomputes temp array of size just 5, using whatever value the
control variables ended up having after the loop.

So for:
  forall (i = 1:5, j = 1:5, a (i, j, 1, 1) .lt. 8)
    forall (k = 1:5, ((a (i, j, k, i) .gt. 6) .or. (a (i, j, k, j) .gt. 6)))
      forall (l = 1:5, a (1, 2, 3, l) .lt. 2)
        a (i, j, k, l) = i - j + k - l + 0.5
      end forall
    end forall
  end forall

gfortran essentially compiles:
  forall (i = 1:5, j = 1:5, a (i, j, 1, 1) .lt. 8)
    forall (k = 1:5, ((a (6, 6, k, 6) .gt. 6) .or. (a (6, 6, k, 6) .gt. 6)))
      forall (l = 1:5, a (1, 2, 3, l) .lt. 2)
        a (i, j, k, l) = i - j + k - l + 0.5
      end forall
    end forall
  end forall

(6 is the value both i and j variables have after computing the 5x5 temp
mask array for the outermost forall loop).

Assuming the above is valid fortran code, I'm afraid gfc_forall_1 needs
to test dependence of the mask expression on the control variables of
outer FORALL statements and if there is any, increase the dimensions
of the temp mask array and iterate over the loops.

	Jakub

Attachment: gcc41-fortran-forall.patch
Description: Text document

Attachment: forall_8.f90
Description: Text document

Attachment: forall_9.f90
Description: Text document