https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107424

--- Comment #4 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Created attachment 54265
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54265&action=edit
WIP patch (with one WIP testcase)

This patch should address the issues of this PR reported so far.

But there two issues: (A) I am not sure whether the implicit 'lastprivate'
should be added for SIMD or not. (B) there is an odd issue with lastprivate
after gimplify, before it looks fine.

(A) 'lastprivate' or not (refers to the patch itself):
The SIMD/!simple change is because of:
  If there is no unit stride, 'count' is created (+ 'lastprivate(count)').
  For collapse == 1, 'linear' is added and later due to the 'dovar_found = 3',
    OMP_CLAUSE_LINEAR_STMT (c) = ...
  For collapse > 1, nothing was added – such that the code trying to add
  a ..._STMT failed to find a clause + and the 'c != NULL' assert then failed.

  The current patch adds 'lastprivate' in this case – an alternative would be
  to move the 'dovar_found = 3' into the 'if'.
  (The lastprivate(count) would still be added.)

  It is not quite clear to me what OpenMP requires - lastprivate or nothing
  (TODO: check).


(B) The following code goes wrong; I converted it as closely as possible to C
and the C program works fine.

  integer :: n,m,p, i,j,k
  n = 11; m = 23; p = 27
  !$omp simd collapse(3) lastprivate(k)
  do i = 1, n
    do j = 1, m
      do k = j - 41, p
        if (k < 1 - 41 .or. k > p) error stop
      end do
    end do
  end do
  end

First, without the patch, the 'j - 41' is evaluated before the loop, which is
obviously wrong:
    D.4268 = j + -41;
    #pragma omp simd lastprivate(k) collapse(3)
      ...
        for (k = D.4268; k <= p; k = k + 1)

But with the patch, it seems to be fine:

  #pragma omp simd lastprivate(j) lastprivate(i) lastprivate(k) collapse(3)
  for (i = 1; i <= n; i = i + 1)
    for (j = 1; j <= m; j = j + 1)
      for (k = j + -41; k <= p; k = k + 1)
            if (k < -40 || k > p)

However, the gimple dump has the following - note the 'k = D.4287' where the
RHS is uninitialized!

      n = 11;
      m = 23;
      p = 27;
      #pragma omp simd lastprivate(j) lastprivate(i) lastprivate(k) collapse(3)
private(k.9) private(j.6) private(i.3) private(dt_parm.0)
      for (i.3 = 1; i.3 <= n; i.3 = i.3 + 1)
        for (j.6 = 1; j.6 <= m; j.6 = j.6 + 1)
          for (k.9 = D.4287; k.9 <= p; k.9 = k.9 + 1)


BTW: The attached patch contains a testcase from which (B) has been extracted
as reduced fail.

TODO: Besides fixing issue (B) and checking (A), some more testcases are needed
+ fixing issues popping up when doing so.

(PS: Currently, the start/end/step expressions are put into the loop even if
they do not depend on another loop variable - this could be optimized, but my
feeling is that this is not worthwhile.)

Reply via email to